Prevent race condition between SavedQuery and Query Loader
SearchFragment and SavedQueryLoader share a loader manager
and had been using the same loader id for different
loaders.
Occaisionally (in monkey tests) two loaders with the same
IDs would be started and crash when they finished.
The loaders now have different IDs.
Change-Id: I11e9b7365605fbcef44cf7d2323183415422f5c8
Fixes: 64756515
Test: robotests
diff --git a/src/com/android/settings/search/SavedQueryController.java b/src/com/android/settings/search/SavedQueryController.java
index 545b415..eaba3d6 100644
--- a/src/com/android/settings/search/SavedQueryController.java
+++ b/src/com/android/settings/search/SavedQueryController.java
@@ -34,10 +34,6 @@
MenuItem.OnMenuItemClickListener {
// TODO: make a generic background task manager to handle one-off tasks like this one.
-
- private static final int LOADER_ID_SAVE_QUERY_TASK = 0;
- private static final int LOADER_ID_REMOVE_QUERY_TASK = 1;
- private static final int LOADER_ID_SAVED_QUERIES = 2;
private static final String ARG_QUERY = "remove_query";
private static final String TAG = "SearchSavedQueryCtrl";
@@ -60,11 +56,11 @@
@Override
public Loader onCreateLoader(int id, Bundle args) {
switch (id) {
- case LOADER_ID_SAVE_QUERY_TASK:
+ case SearchFragment.SearchLoaderId.SAVE_QUERY_TASK:
return new SavedQueryRecorder(mContext, args.getString(ARG_QUERY));
- case LOADER_ID_REMOVE_QUERY_TASK:
+ case SearchFragment.SearchLoaderId.REMOVE_QUERY_TASK:
return new SavedQueryRemover(mContext);
- case LOADER_ID_SAVED_QUERIES:
+ case SearchFragment.SearchLoaderId.SAVED_QUERIES:
return mSearchFeatureProvider.getSavedQueryLoader(mContext);
}
return null;
@@ -73,10 +69,11 @@
@Override
public void onLoadFinished(Loader loader, Object data) {
switch (loader.getId()) {
- case LOADER_ID_REMOVE_QUERY_TASK:
- mLoaderManager.restartLoader(LOADER_ID_SAVED_QUERIES, null, this);
+ case SearchFragment.SearchLoaderId.REMOVE_QUERY_TASK:
+ mLoaderManager.restartLoader(SearchFragment.SearchLoaderId.SAVED_QUERIES,
+ null /* args */, this /* callback */);
break;
- case LOADER_ID_SAVED_QUERIES:
+ case SearchFragment.SearchLoaderId.SAVED_QUERIES:
if (SettingsSearchIndexablesProvider.DEBUG) {
Log.d(TAG, "Saved queries loaded");
}
@@ -107,7 +104,8 @@
public void saveQuery(String query) {
final Bundle args = new Bundle();
args.putString(ARG_QUERY, query);
- mLoaderManager.restartLoader(LOADER_ID_SAVE_QUERY_TASK, args, this);
+ mLoaderManager.restartLoader(SearchFragment.SearchLoaderId.SAVE_QUERY_TASK, args,
+ this /* callback */);
}
/**
@@ -115,13 +113,15 @@
*/
public void removeQueries() {
final Bundle args = new Bundle();
- mLoaderManager.restartLoader(LOADER_ID_REMOVE_QUERY_TASK, args, this);
+ mLoaderManager.restartLoader(SearchFragment.SearchLoaderId.REMOVE_QUERY_TASK, args,
+ this /* callback */);
}
public void loadSavedQueries() {
if (SettingsSearchIndexablesProvider.DEBUG) {
Log.d(TAG, "loading saved queries");
}
- mLoaderManager.restartLoader(LOADER_ID_SAVED_QUERIES, null, this);
+ mLoaderManager.restartLoader(SearchFragment.SearchLoaderId.SAVED_QUERIES, null /* args */,
+ this /* callback */);
}
}
diff --git a/src/com/android/settings/search/SearchFragment.java b/src/com/android/settings/search/SearchFragment.java
index ccc4d61..092404c 100644
--- a/src/com/android/settings/search/SearchFragment.java
+++ b/src/com/android/settings/search/SearchFragment.java
@@ -77,15 +77,19 @@
private static final String STATE_NEVER_ENTERED_QUERY = "state_never_entered_query";
private static final String STATE_RESULT_CLICK_COUNT = "state_result_click_count";
- // Loader IDs
- @VisibleForTesting
- static final int LOADER_ID_DATABASE = 1;
- @VisibleForTesting
- static final int LOADER_ID_INSTALLED_APPS = 2;
- @VisibleForTesting
- static final int LOADER_ID_ACCESSIBILITY_SERVICES = 3;
- @VisibleForTesting
- static final int LOADER_ID_INPUT_DEVICES = 4;
+ static final class SearchLoaderId {
+ // Search Query IDs
+ public static final int DATABASE = 1;
+ public static final int INSTALLED_APPS = 2;
+ public static final int ACCESSIBILITY_SERVICES = 3;
+ public static final int INPUT_DEVICES = 4;
+
+ // Saved Query IDs
+ public static final int SAVE_QUERY_TASK = 5;
+ public static final int REMOVE_QUERY_TASK = 6;
+ public static final int SAVED_QUERIES = 7;
+ }
+
private static final int NUM_QUERY_LOADERS = 4;
@@ -283,10 +287,10 @@
if (isEmptyQuery) {
final LoaderManager loaderManager = getLoaderManager();
- loaderManager.destroyLoader(LOADER_ID_DATABASE);
- loaderManager.destroyLoader(LOADER_ID_INSTALLED_APPS);
- loaderManager.destroyLoader(LOADER_ID_ACCESSIBILITY_SERVICES);
- loaderManager.destroyLoader(LOADER_ID_INPUT_DEVICES);
+ loaderManager.destroyLoader(SearchLoaderId.DATABASE);
+ loaderManager.destroyLoader(SearchLoaderId.INSTALLED_APPS);
+ loaderManager.destroyLoader(SearchLoaderId.ACCESSIBILITY_SERVICES);
+ loaderManager.destroyLoader(SearchLoaderId.INPUT_DEVICES);
mShowingSavedQuery = true;
mSavedQueryController.loadSavedQueries();
mSearchFeatureProvider.hideFeedbackButton();
@@ -311,13 +315,13 @@
final Activity activity = getActivity();
switch (id) {
- case LOADER_ID_DATABASE:
+ case SearchLoaderId.DATABASE:
return mSearchFeatureProvider.getDatabaseSearchLoader(activity, mQuery);
- case LOADER_ID_INSTALLED_APPS:
+ case SearchLoaderId.INSTALLED_APPS:
return mSearchFeatureProvider.getInstalledAppSearchLoader(activity, mQuery);
- case LOADER_ID_ACCESSIBILITY_SERVICES:
+ case SearchLoaderId.ACCESSIBILITY_SERVICES:
return mSearchFeatureProvider.getAccessibilityServiceResultLoader(activity, mQuery);
- case LOADER_ID_INPUT_DEVICES:
+ case SearchLoaderId.INPUT_DEVICES:
return mSearchFeatureProvider.getInputDeviceResultLoader(activity, mQuery);
default:
return null;
@@ -351,13 +355,13 @@
mSavedQueryController.loadSavedQueries();
} else {
final LoaderManager loaderManager = getLoaderManager();
- loaderManager.initLoader(LOADER_ID_DATABASE, null /* args */, this /* callback */);
+ loaderManager.initLoader(SearchLoaderId.DATABASE, null /* args */, this /* callback */);
loaderManager.initLoader(
- LOADER_ID_INSTALLED_APPS, null /* args */, this /* callback */);
+ SearchLoaderId.INSTALLED_APPS, null /* args */, this /* callback */);
loaderManager.initLoader(
- LOADER_ID_ACCESSIBILITY_SERVICES, null /* args */, this /* callback */);
+ SearchLoaderId.ACCESSIBILITY_SERVICES, null /* args */, this /* callback */);
loaderManager.initLoader(
- LOADER_ID_INPUT_DEVICES, null /* args */, this /* callback */);
+ SearchLoaderId.INPUT_DEVICES, null /* args */, this /* callback */);
}
requery();
@@ -395,12 +399,14 @@
mShowingSavedQuery = false;
final LoaderManager loaderManager = getLoaderManager();
mUnfinishedLoadersCount.set(NUM_QUERY_LOADERS);
- loaderManager.restartLoader(LOADER_ID_DATABASE, null /* args */, this /* callback */);
- loaderManager.restartLoader(LOADER_ID_INSTALLED_APPS, null /* args */, this /* callback */);
- loaderManager.restartLoader(LOADER_ID_ACCESSIBILITY_SERVICES, null /* args */,
- this /* callback */);
- loaderManager.restartLoader(LOADER_ID_INPUT_DEVICES, null /* args */,
- this /* callback */);
+ loaderManager.restartLoader(
+ SearchLoaderId.DATABASE, null /* args */, this /* callback */);
+ loaderManager.restartLoader(
+ SearchLoaderId.INSTALLED_APPS, null /* args */, this /* callback */);
+ loaderManager.restartLoader(
+ SearchLoaderId.ACCESSIBILITY_SERVICES, null /* args */, this /* callback */);
+ loaderManager.restartLoader(
+ SearchLoaderId.INPUT_DEVICES, null /* args */, this /* callback */);
}
public String getQuery() {
diff --git a/tests/robotests/src/com/android/settings/search/SearchFragmentTest.java b/tests/robotests/src/com/android/settings/search/SearchFragmentTest.java
index 71c31aa..d78a818 100644
--- a/tests/robotests/src/com/android/settings/search/SearchFragmentTest.java
+++ b/tests/robotests/src/com/android/settings/search/SearchFragmentTest.java
@@ -410,9 +410,9 @@
fragment.onIndexingFinished();
- verify(loaderManager).initLoader(eq(SearchFragment.LOADER_ID_DATABASE),
+ verify(loaderManager).initLoader(eq(SearchFragment.SearchLoaderId.DATABASE),
eq(null), any(LoaderManager.LoaderCallbacks.class));
- verify(loaderManager).initLoader(eq(SearchFragment.LOADER_ID_INSTALLED_APPS),
+ verify(loaderManager).initLoader(eq(SearchFragment.SearchLoaderId.INSTALLED_APPS),
eq(null), any(LoaderManager.LoaderCallbacks.class));
}