Prevent race conditions after dropping database
Block all loaders from accessing the database
before indexing is complete.
Bug: 37501479
Test: make RunSettingsRoboTests
Change-Id: I2af98dcf6bceacbeffa70bd40879c97bb0cbc828
Merged-In: I02f8423c0ffc27abbb8ceb61a8c47d2f0796d0bb
diff --git a/src/com/android/settings/SettingsActivity.java b/src/com/android/settings/SettingsActivity.java
index 34990ec..3e45af7 100644
--- a/src/com/android/settings/SettingsActivity.java
+++ b/src/com/android/settings/SettingsActivity.java
@@ -79,7 +79,7 @@
private static final String LOG_TAG = "Settings";
- private static final int LOADER_ID_INDEXABLE_CONTENT_MONITOR = 1;
+ public static final int LOADER_ID_INDEXABLE_CONTENT_MONITOR = 1;
// Constants for state save/restore
private static final String SAVE_KEY_CATEGORIES = ":settings:categories";
diff --git a/src/com/android/settings/search/IndexingCallback.java b/src/com/android/settings/search/IndexingCallback.java
new file mode 100644
index 0000000..b4b6eab
--- /dev/null
+++ b/src/com/android/settings/search/IndexingCallback.java
@@ -0,0 +1,12 @@
+package com.android.settings.search;
+
+/**
+ * Callback for Settings search indexing.
+ */
+public interface IndexingCallback {
+
+ /**
+ * Called when Indexing is finished.
+ */
+ void onIndexingFinished();
+}
diff --git a/src/com/android/settings/search2/DatabaseIndexingManager.java b/src/com/android/settings/search2/DatabaseIndexingManager.java
index 80ae54f..c627ea5 100644
--- a/src/com/android/settings/search2/DatabaseIndexingManager.java
+++ b/src/com/android/settings/search2/DatabaseIndexingManager.java
@@ -42,6 +42,7 @@
import com.android.settings.core.PreferenceController;
import com.android.settings.search.IndexDatabaseHelper;
import com.android.settings.search.Indexable;
+import com.android.settings.search.IndexingCallback;
import com.android.settings.search.SearchIndexableRaw;
import com.android.settings.search.SearchIndexableResources;
@@ -132,7 +133,8 @@
private final String mBaseAuthority;
- private final AtomicBoolean mIsAvailable = new AtomicBoolean(false);
+ @VisibleForTesting
+ final AtomicBoolean mIsIndexingComplete = new AtomicBoolean(false);
@VisibleForTesting
final UpdateData mDataToProcess = new UpdateData();
@@ -147,17 +149,13 @@
mContext = context;
}
- public boolean isAvailable() {
- return mIsAvailable.get();
+ public boolean isIndexingComplete() {
+ return mIsIndexingComplete.get();
}
- public void indexDatabase() {
- AsyncTask.execute(new Runnable() {
- @Override
- public void run() {
- performIndexing();
- }
- });
+ public void indexDatabase(IndexingCallback callback) {
+ IndexingTask task = new IndexingTask(callback);
+ task.execute();
}
/**
@@ -171,15 +169,12 @@
final List<ResolveInfo> list =
mContext.getPackageManager().queryIntentContentProviders(intent, 0);
- final String localeStr = Locale.getDefault().toString();
- final String fingerprint = Build.FINGERPRINT;
+ String localeStr = Locale.getDefault().toString();
+ String fingerprint = Build.FINGERPRINT;
final boolean isFullIndex = isFullIndex(localeStr, fingerprint);
- // Drop the database when the locale or build has changed. This eliminates rows which are
- // dynamically inserted in the old language, or deprecated settings.
if (isFullIndex) {
- final SQLiteDatabase db = getWritableDatabase();
- IndexDatabaseHelper.getInstance(mContext).reconstruct(db);
+ rebuildDatabase();
}
for (final ResolveInfo info : list) {
@@ -218,6 +213,18 @@
}
/**
+ * Reconstruct the database in the following cases:
+ * - Language has changed
+ * - Build has changed
+ */
+ private void rebuildDatabase() {
+ // Drop the database when the locale or build has changed. This eliminates rows which are
+ // dynamically inserted in the old language, or deprecated settings.
+ final SQLiteDatabase db = getWritableDatabase();
+ IndexDatabaseHelper.getInstance(mContext).reconstruct(db);
+ }
+
+ /**
* Adds new data to the database and verifies the correctness of the ENABLED column.
* First, the data to be updated and all non-indexable keys are copied locally.
* Then all new data to be added is inserted.
@@ -229,7 +236,6 @@
*/
@VisibleForTesting
void updateDatabase(boolean needsReindexing, String localeStr) {
- mIsAvailable.set(false);
final UpdateData copy;
synchronized (mDataToProcess) {
@@ -264,8 +270,6 @@
} finally {
database.endTransaction();
}
-
- mIsAvailable.set(true);
}
/**
@@ -1223,4 +1227,33 @@
}
}
}
+
+ public class IndexingTask extends AsyncTask<Void, Void, Void> {
+
+ @VisibleForTesting
+ IndexingCallback mCallback;
+
+ public IndexingTask(IndexingCallback callback) {
+ mCallback = callback;
+ }
+
+ @Override
+ protected void onPreExecute() {
+ mIsIndexingComplete.set(false);
+ }
+
+ @Override
+ protected Void doInBackground(Void... voids) {
+ performIndexing();
+ return null;
+ }
+
+ @Override
+ protected void onPostExecute(Void aVoid) {
+ mIsIndexingComplete.set(true);
+ if (mCallback != null) {
+ mCallback.onIndexingFinished();
+ }
+ }
+ }
}
\ No newline at end of file
diff --git a/src/com/android/settings/search2/SearchFeatureProvider.java b/src/com/android/settings/search2/SearchFeatureProvider.java
index d3dc24b..fa052fc 100644
--- a/src/com/android/settings/search2/SearchFeatureProvider.java
+++ b/src/com/android/settings/search2/SearchFeatureProvider.java
@@ -21,6 +21,7 @@
import android.view.View;
import com.android.settings.dashboard.SiteMapManager;
+import com.android.settings.search.IndexingCallback;
/**
* FeatureProvider for Settings Search
@@ -68,7 +69,12 @@
/**
* Updates the Settings indexes
*/
- void updateIndex(Context context);
+ void updateIndex(Context context, IndexingCallback callback);
+
+ /**
+ * @returns true when indexing is complete.
+ */
+ boolean isIndexingComplete(Context context);
/**
* Initializes the feedback button in case it was dismissed.
diff --git a/src/com/android/settings/search2/SearchFeatureProviderImpl.java b/src/com/android/settings/search2/SearchFeatureProviderImpl.java
index b180c98..4e47f9d 100644
--- a/src/com/android/settings/search2/SearchFeatureProviderImpl.java
+++ b/src/com/android/settings/search2/SearchFeatureProviderImpl.java
@@ -19,6 +19,7 @@
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
+import android.support.annotation.VisibleForTesting;
import android.util.Log;
import android.view.Menu;
import android.view.MenuItem;
@@ -26,6 +27,9 @@
import com.android.settings.R;
import com.android.settings.applications.PackageManagerWrapperImpl;
import com.android.settings.dashboard.SiteMapManager;
+import com.android.settings.search.IndexingCallback;
+
+import java.util.concurrent.atomic.AtomicBoolean;
/**
* FeatureProvider for the refactored search code.
@@ -88,6 +92,11 @@
return mDatabaseIndexingManager;
}
+ @Override
+ public boolean isIndexingComplete(Context context) {
+ return getIndexingManager(context).isIndexingComplete();
+ }
+
public SiteMapManager getSiteMapManager() {
if (mSiteMapManager == null) {
mSiteMapManager = new SiteMapManager();
@@ -96,9 +105,9 @@
}
@Override
- public void updateIndex(Context context) {
+ public void updateIndex(Context context, IndexingCallback callback) {
long indexStartTime = System.currentTimeMillis();
- getIndexingManager(context).indexDatabase();
+ getIndexingManager(context).indexDatabase(callback);
Log.d(TAG, "IndexDatabase() took " +
(System.currentTimeMillis() - indexStartTime) + " ms");
}
diff --git a/src/com/android/settings/search2/SearchFragment.java b/src/com/android/settings/search2/SearchFragment.java
index c5db08d..5ee22b5 100644
--- a/src/com/android/settings/search2/SearchFragment.java
+++ b/src/com/android/settings/search2/SearchFragment.java
@@ -41,12 +41,23 @@
import com.android.settings.core.InstrumentedFragment;
import com.android.settings.core.instrumentation.MetricsFeatureProvider;
import com.android.settings.overlay.FeatureFactory;
+import com.android.settings.search.IndexingCallback;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
+/**
+ * This fragment manages the lifecycle of indexing and searching.
+ *
+ * In onCreate, the indexing process is initiated in DatabaseIndexingManager.
+ * While the indexing is happening, loaders are blocked from accessing the database, but the user
+ * is free to start typing their query.
+ *
+ * When the indexing is complete, the fragment gets a callback to initialize the loaders and search
+ * the query if the user has entered text.
+ */
public class SearchFragment extends InstrumentedFragment implements SearchView.OnQueryTextListener,
- LoaderManager.LoaderCallbacks<List<? extends SearchResult>> {
+ LoaderManager.LoaderCallbacks<List<? extends SearchResult>>, IndexingCallback {
private static final String TAG = "SearchFragment";
@VisibleForTesting
@@ -59,26 +70,30 @@
private static final String STATE_RESULT_CLICK_COUNT = "state_result_click_count";
// Loader IDs
- private static final int LOADER_ID_DATABASE = 1;
- private static final int LOADER_ID_INSTALLED_APPS = 2;
+ @VisibleForTesting
+ static final int LOADER_ID_DATABASE = 1;
+ @VisibleForTesting
+ static final int LOADER_ID_INSTALLED_APPS = 2;
private static final int NUM_QUERY_LOADERS = 2;
- @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+ @VisibleForTesting
AtomicInteger mUnfinishedLoadersCount = new AtomicInteger(NUM_QUERY_LOADERS);
// Logging
- @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+ @VisibleForTesting
static final String RESULT_CLICK_COUNT = "settings_search_result_click_count";
@VisibleForTesting
String mQuery;
private boolean mNeverEnteredQuery = true;
- private boolean mShowingSavedQuery;
+ @VisibleForTesting
+ boolean mShowingSavedQuery;
private int mResultClickCount;
private MetricsFeatureProvider mMetricsFeatureProvider;
- private SavedQueryController mSavedQueryController;
+ @VisibleForTesting
+ SavedQueryController mSavedQueryController;
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
SearchFeatureProvider mSearchFeatureProvider;
@@ -87,7 +102,8 @@
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
RecyclerView mResultsRecyclerView;
- private SearchView mSearchView;
+ @VisibleForTesting
+ SearchView mSearchView;
private LinearLayout mNoResultsView;
@VisibleForTesting
@@ -116,6 +132,7 @@
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setHasOptionsMenu(true);
+
final LoaderManager loaderManager = getLoaderManager();
mSearchAdapter = new SearchResultsAdapter(this);
mSavedQueryController = new SavedQueryController(
@@ -127,15 +144,8 @@
mNeverEnteredQuery = savedInstanceState.getBoolean(STATE_NEVER_ENTERED_QUERY);
mResultClickCount = savedInstanceState.getInt(STATE_RESULT_CLICK_COUNT);
mShowingSavedQuery = savedInstanceState.getBoolean(STATE_SHOWING_SAVED_QUERY);
- if (mShowingSavedQuery) {
- mSavedQueryController.loadSavedQueries();
- } else {
- loaderManager.initLoader(LOADER_ID_DATABASE, null, this);
- loaderManager.initLoader(LOADER_ID_INSTALLED_APPS, null, this);
- }
} else {
mShowingSavedQuery = true;
- mSavedQueryController.loadSavedQueries();
}
final Activity activity = getActivity();
@@ -148,7 +158,7 @@
// Run the Index update only if we have some space
if (!Utils.isLowStorage(activity)) {
- mSearchFeatureProvider.updateIndex(activity);
+ mSearchFeatureProvider.updateIndex(activity, this /* indexingCallback */);
} else {
Log.w(TAG, "Cannot update the Indexer as we are running low on storage space!");
}
@@ -170,12 +180,7 @@
@Override
public void onResume() {
super.onResume();
- if (TextUtils.isEmpty(mQuery)) {
- return;
- }
- final String query = mQuery;
- mQuery = "";
- onQueryTextChange(query);
+ requery();
}
@Override
@@ -218,6 +223,11 @@
mNeverEnteredQuery = false;
mQuery = query;
+ // If indexing is not finished, register the query text, but don't search.
+ if (!mSearchFeatureProvider.isIndexingComplete(getActivity())) {
+ return true;
+ }
+
if (isEmptyQuery) {
final LoaderManager loaderManager = getLoaderManager();
loaderManager.destroyLoader(LOADER_ID_DATABASE);
@@ -276,6 +286,22 @@
public void onLoaderReset(Loader<List<? extends SearchResult>> loader) {
}
+ /**
+ * Gets called when Indexing is completed.
+ */
+ @Override
+ public void onIndexingFinished() {
+ if (mShowingSavedQuery) {
+ mSavedQueryController.loadSavedQueries();
+ } else {
+ final LoaderManager loaderManager = getLoaderManager();
+ loaderManager.initLoader(LOADER_ID_DATABASE, null, this);
+ loaderManager.initLoader(LOADER_ID_INSTALLED_APPS, null, this);
+ }
+
+ requery();
+ }
+
public void onSearchResultClicked() {
mSavedQueryController.saveQuery(mQuery);
mResultClickCount++;
@@ -309,6 +335,15 @@
return mSearchAdapter.getSearchResults();
}
+ private void requery() {
+ if (TextUtils.isEmpty(mQuery)) {
+ return;
+ }
+ final String query = mQuery;
+ mQuery = "";
+ onQueryTextChange(query);
+ }
+
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
SearchView makeSearchView(ActionBar actionBar, String query) {
final SearchView searchView = new SearchView(actionBar.getThemedContext());