PawletOS/app_Settings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Prevent race condition between SavedQuery and Query Loader

Browse Source 
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
This commit is contained in:
Matthew Fritze
2017-08-16 15:45:52 -07:00
parent ccbb613fa9
commit 21aed344fc
3 changed files with 48 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
src/com/android/settings/search/SavedQueryController.java
View File

@@ -34,10 +34,6 @@ public class SavedQueryController implements LoaderManager.LoaderCallbacks,
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 @@ public class SavedQueryController implements LoaderManager.LoaderCallbacks,
@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 @@ public class SavedQueryController implements LoaderManager.LoaderCallbacks,
@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 class SavedQueryController implements LoaderManager.LoaderCallbacks,
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 class SavedQueryController implements LoaderManager.LoaderCallbacks,
*/
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 */);
}
}

60
src/com/android/settings/search/SearchFragment.java
View File

@@ -77,15 +77,19 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
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 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
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 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
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 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
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 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
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() {

4
tests/robotests/src/com/android/settings/search/SearchFragmentTest.java
View File

@@ -410,9 +410,9 @@ public class SearchFragmentTest {
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));
}