Prevent search crashes from uninstalled apps

All search results are now refreshed when resuming the
search fragment, to prevent crashes from results that
no longer exist.

Bug: 34817357
Test: make RunSettingsRoboTests
Change-Id: I96a0cbfee711ab9dee49d56bfdc4e885202d9ecd

diff --git a/src/com/android/settings/search2/DatabaseIndexingManager.java b/src/com/android/settings/search2/DatabaseIndexingManager.java
index 30aad76..c2e6ba8 100644 (file)
--- a/src/com/android/settings/search2/DatabaseIndexingManager.java
+++ b/src/com/android/settings/search2/DatabaseIndexingManager.java
@@ -517,7 +517,7 @@ public class DatabaseIndexingManager {
             if (count > 0) {
                 while (cursor.moveToNext()) {
                     final int providerRank = cursor.getInt(COLUMN_INDEX_XML_RES_RANK);
-
+                    // TODO remove provider rank
                     final int xmlResId = cursor.getInt(COLUMN_INDEX_XML_RES_RESID);
 
                     final String className = cursor.getString(COLUMN_INDEX_XML_RES_CLASS_NAME);
@@ -562,7 +562,7 @@ public class DatabaseIndexingManager {
             if (count > 0) {
                 while (cursor.moveToNext()) {
                     final int providerRank = cursor.getInt(COLUMN_INDEX_RAW_RANK);
-
+                    // TODO Remove rank
                     final String title = cursor.getString(COLUMN_INDEX_RAW_TITLE);
                     final String summaryOn = cursor.getString(COLUMN_INDEX_RAW_SUMMARY_ON);
                     final String summaryOff = cursor.getString(COLUMN_INDEX_RAW_SUMMARY_OFF);

diff --git a/src/com/android/settings/search2/SearchFragment.java b/src/com/android/settings/search2/SearchFragment.java
index a8d219c..545a225 100644 (file)
--- a/src/com/android/settings/search2/SearchFragment.java
+++ b/src/com/android/settings/search2/SearchFragment.java
@@ -168,6 +168,17 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
     }
 
     @Override
+    public void onResume() {
+        super.onResume();
+        if (TextUtils.isEmpty(mQuery)) {
+            return;
+        }
+        final String query = mQuery;
+        mQuery = "";
+        onQueryTextChange(query);
+    }
+
+    @Override
     public void onStop() {
         super.onStop();
         final Activity activity = getActivity();
@@ -206,7 +217,6 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
         mResultClickCount = 0;
         mNeverEnteredQuery = false;
         mQuery = query;
-        mSearchAdapter.clearResults();
 
         if (isEmptyQuery) {
             final LoaderManager loaderManager = getLoaderManager();
@@ -251,8 +261,15 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O
         if (mUnfinishedLoadersCount.decrementAndGet() != 0) {
             return;
         }
+
         final int resultCount = mSearchAdapter.displaySearchResults(mQuery);
-        mNoResultsView.setVisibility(resultCount == 0 ? View.VISIBLE : View.GONE);
+
+        if (resultCount == 0) {
+            mNoResultsView.setVisibility(View.VISIBLE);
+        } else {
+            mNoResultsView.setVisibility(View.GONE);
+            mResultsRecyclerView.scrollToPosition(0);
+        }
         mSearchFeatureProvider.showFeedbackButton(this, getView());
     }
 

diff --git a/src/com/android/settings/search2/SearchResult.java b/src/com/android/settings/search2/SearchResult.java
index 6b27d89..36fe5b3 100644 (file)
--- a/src/com/android/settings/search2/SearchResult.java
+++ b/src/com/android/settings/search2/SearchResult.java
@@ -93,7 +93,7 @@ public class SearchResult implements Comparable<SearchResult> {
         icon = builder.mIcon;
         payload = builder.mResultPayload;
         viewType = payload.getType();
-        stableId = Objects.hash(title, summary, breadcrumbs, rank, icon, payload, viewType);
+        stableId = Objects.hash(title, summary, breadcrumbs, rank, viewType);
     }
 
     @Override
@@ -104,6 +104,22 @@ public class SearchResult implements Comparable<SearchResult> {
         return this.rank - searchResult.rank;
     }
 
+    @Override
+    public boolean equals(Object that) {
+        if (this == that) {
+            return true;
+        }
+        if (!(that instanceof SearchResult)) {
+            return false;
+        }
+        return this.stableId == ((SearchResult) that).stableId;
+    }
+
+    @Override
+    public int hashCode() {
+        return (int) stableId;
+    }
+
     public static class Builder {
         protected CharSequence mTitle;
         protected CharSequence mSummary;
@@ -127,19 +143,19 @@ public class SearchResult implements Comparable<SearchResult> {
             return this;
         }
 
-        public Builder  addRank(int rank) {
+        public Builder addRank(int rank) {
             if (rank >= 0 && rank <= 9) {
                 mRank = rank;
             }
             return this;
         }
 
-        public Builder  addIcon(Drawable icon) {
+        public Builder addIcon(Drawable icon) {
             mIcon = icon;
             return this;
         }
 
-        public Builder  addPayload(ResultPayload payload) {
+        public Builder addPayload(ResultPayload payload) {
             mResultPayload = payload;
             return this;
         }

diff --git a/src/com/android/settings/search2/SearchResultDiffCallback.java b/src/com/android/settings/search2/SearchResultDiffCallback.java
new file mode 100644 (file)
index 0000000..9bd1bde
--- /dev/null
+++ b/src/com/android/settings/search2/SearchResultDiffCallback.java
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package com.android.settings.search2;
+
+import android.support.v7.util.DiffUtil;
+
+import java.util.List;
+
+/**
+ * Callback for DiffUtil to elegantly update search data when the query changes.
+ */
+public class SearchResultDiffCallback extends DiffUtil.Callback {
+
+    private List<SearchResult> mOldList;
+    private List<SearchResult> mNewList;
+
+    public SearchResultDiffCallback(List<SearchResult> oldList, List<SearchResult> newList) {
+        mOldList = oldList;
+        mNewList = newList;
+    }
+
+    @Override
+    public int getOldListSize() {
+        return mOldList.size();
+    }
+
+    @Override
+    public int getNewListSize() {
+        return mNewList.size();
+    }
+
+    @Override
+    public boolean areItemsTheSame(int oldItemPosition, int newItemPosition) {
+        return mOldList.get(oldItemPosition).equals(mNewList.get(newItemPosition));
+    }
+
+    @Override
+    public boolean areContentsTheSame(int oldItemPosition, int newItemPosition) {
+        return mOldList.get(oldItemPosition).equals(mNewList.get(newItemPosition));
+    }
+}

diff --git a/src/com/android/settings/search2/SearchResultsAdapter.java b/src/com/android/settings/search2/SearchResultsAdapter.java
index d0ea5bf..5da5966 100644 (file)
--- a/src/com/android/settings/search2/SearchResultsAdapter.java
+++ b/src/com/android/settings/search2/SearchResultsAdapter.java
@@ -19,6 +19,7 @@ package com.android.settings.search2;
 import android.content.Context;
 import android.support.annotation.MainThread;
 import android.support.annotation.VisibleForTesting;
+import android.support.v7.util.DiffUtil;
 import android.support.v7.widget.RecyclerView;
 import android.util.ArrayMap;
 import android.view.LayoutInflater;
@@ -37,8 +38,9 @@ import static com.android.settings.search2.SearchResult.TOP_RANK;
 
 public class SearchResultsAdapter extends RecyclerView.Adapter<SearchViewHolder> {
 
-    private final List<SearchResult> mSearchResults;
     private final SearchFragment mFragment;
+
+    private List<SearchResult> mSearchResults;
     private Map<String, List<? extends SearchResult>> mResultsMap;
     private final SearchFeatureProvider mSearchFeatureProvider;
 
@@ -132,7 +134,7 @@ public class SearchResultsAdapter extends RecyclerView.Adapter<SearchViewHolder>
                 .get(InstalledAppResultLoader.class.getName());
         final int dbSize = (databaseResults != null) ? databaseResults.size() : 0;
         final int appSize = (installedAppResults != null) ? installedAppResults.size() : 0;
-        final List<SearchResult> results = new ArrayList<>(dbSize + appSize);
+        final List<SearchResult> newResults = new ArrayList<>(dbSize + appSize);
 
         int dbIndex = 0;
         int appIndex = 0;
@@ -140,29 +142,34 @@ public class SearchResultsAdapter extends RecyclerView.Adapter<SearchViewHolder>
 
         while (rank <= BOTTOM_RANK) {
             while ((dbIndex < dbSize) && (databaseResults.get(dbIndex).rank == rank)) {
-                results.add(databaseResults.get(dbIndex++));
+                newResults.add(databaseResults.get(dbIndex++));
             }
             while ((appIndex < appSize) && (installedAppResults.get(appIndex).rank == rank)) {
-                results.add(installedAppResults.get(appIndex++));
+                newResults.add(installedAppResults.get(appIndex++));
             }
             rank++;
         }
 
         while (dbIndex < dbSize) {
-            results.add(databaseResults.get(dbIndex++));
+            newResults.add(databaseResults.get(dbIndex++));
         }
         while (appIndex < appSize) {
-            results.add(installedAppResults.get(appIndex++));
+            newResults.add(installedAppResults.get(appIndex++));
         }
 
-        if (mSearchFeatureProvider
-                .isSmartSearchRankingEnabled(mFragment.getContext().getApplicationContext())) {
+        final boolean isSmartSearchRankingEnabled = mSearchFeatureProvider
+                .isSmartSearchRankingEnabled(mFragment.getContext().getApplicationContext());
+
+        if (isSmartSearchRankingEnabled) {
             // TODO: run this in parallel to loading the results if takes too long
-            mSearchFeatureProvider.rankSearchResults(query, results);
+            mSearchFeatureProvider.rankSearchResults(query, newResults);
         }
 
-        mSearchResults.addAll(results);
-        notifyDataSetChanged();
+        final DiffUtil.DiffResult diffResult = DiffUtil.calculateDiff(
+                new SearchResultDiffCallback(mSearchResults, newResults),
+                isSmartSearchRankingEnabled);
+        mSearchResults = newResults;
+        diffResult.dispatchUpdatesTo(this);
 
         return mSearchResults.size();
     }

diff --git a/tests/robotests/src/com/android/settings/search/SearchResultsAdapterTest.java b/tests/robotests/src/com/android/settings/search/SearchResultsAdapterTest.java
index c507c79..3007f5e 100644 (file)
--- a/tests/robotests/src/com/android/settings/search/SearchResultsAdapterTest.java
+++ b/tests/robotests/src/com/android/settings/search/SearchResultsAdapterTest.java
@@ -57,6 +57,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import static com.google.common.truth.Truth.assertThat;
@@ -75,6 +76,8 @@ public class SearchResultsAdapterTest {
     private Context mContext;
     private String mLoaderClassName;
 
+    private String[] TITLES = {"alpha", "bravo", "charlie", "appAlpha", "appBravo", "appCharlie"};
+
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
@@ -86,13 +89,13 @@ public class SearchResultsAdapterTest {
     }
 
     @Test
-    public void testNoResultsAdded_EmptyListReturned() {
+    public void testNoResultsAdded_emptyListReturned() {
         List<SearchResult> updatedResults = mAdapter.getSearchResults();
         assertThat(updatedResults).isEmpty();
     }
 
     @Test
-    public void testSingleSourceMerge_ExactCopyReturned() {
+    public void testSingleSourceMerge_exactCopyReturned() {
         ArrayList<SearchResult> intentResults = getIntentSampleResults();
         mAdapter.addSearchResults(intentResults, mLoaderClassName);
         mAdapter.displaySearchResults("");
@@ -102,7 +105,7 @@ public class SearchResultsAdapterTest {
     }
 
     @Test
-    public void testCreateViewHolder_ReturnsIntentResult() {
+    public void testCreateViewHolder_returnsIntentResult() {
         ViewGroup group = new FrameLayout(mContext);
         SearchViewHolder view = mAdapter.onCreateViewHolder(group,
                 ResultPayload.PayloadType.INTENT);
@@ -110,7 +113,7 @@ public class SearchResultsAdapterTest {
     }
 
     @Test
-    public void testCreateViewHolder_ReturnsInlineSwitchResult() {
+    public void testCreateViewHolder_returnsInlineSwitchResult() {
         ViewGroup group = new FrameLayout(mContext);
         SearchViewHolder view = mAdapter.onCreateViewHolder(group,
                 ResultPayload.PayloadType.INLINE_SWITCH);
@@ -118,20 +121,42 @@ public class SearchResultsAdapterTest {
     }
 
     @Test
-    public void testEndToEndSearch_ProperResultsMerged() {
-        mAdapter.addSearchResults(getDummyAppResults(),
+    public void testEndToEndSearch_properResultsMerged_correctOrder() {
+        mAdapter.addSearchResults(getDummyAppResults(), InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(getDummyDbResults(), DatabaseResultLoader.class.getName());
+        int count = mAdapter.displaySearchResults("");
+
+        List<SearchResult> results = mAdapter.getSearchResults();
+        assertThat(results.get(0).title).isEqualTo(TITLES[0]); // alpha
+        assertThat(results.get(1).title).isEqualTo(TITLES[3]); // appAlpha
+        assertThat(results.get(2).title).isEqualTo(TITLES[4]); // appBravo
+        assertThat(results.get(3).title).isEqualTo(TITLES[1]); // bravo
+        assertThat(results.get(4).title).isEqualTo(TITLES[5]); // appCharlie
+        assertThat(results.get(5).title).isEqualTo(TITLES[2]); // charlie
+        assertThat(count).isEqualTo(6);
+    }
+
+    @Test
+    public void testEndToEndSearch_addResults_resultsAddedInOrder() {
+        List<AppSearchResult> appResults = getDummyAppResults();
+        List<SearchResult> dbResults = getDummyDbResults();
+        // Add two individual items
+        mAdapter.addSearchResults(appResults.subList(0,1),
                 InstalledAppResultLoader.class.getName());
-        mAdapter.addSearchResults(getDummyDbResults(),
-                DatabaseResultLoader.class.getName());
+        mAdapter.addSearchResults(dbResults.subList(0,1), DatabaseResultLoader.class.getName());
+        mAdapter.displaySearchResults("");
+        // Add super-set of items
+        mAdapter.addSearchResults(appResults, InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(dbResults, DatabaseResultLoader.class.getName());
         int count = mAdapter.displaySearchResults("");
 
         List<SearchResult> results = mAdapter.getSearchResults();
-        assertThat(results.get(0).title).isEqualTo("alpha");
-        assertThat(results.get(1).title).isEqualTo("appAlpha");
-        assertThat(results.get(2).title).isEqualTo("appBravo");
-        assertThat(results.get(3).title).isEqualTo("bravo");
-        assertThat(results.get(4).title).isEqualTo("appCharlie");
-        assertThat(results.get(5).title).isEqualTo("Charlie");
+        assertThat(results.get(0).title).isEqualTo(TITLES[0]); // alpha
+        assertThat(results.get(1).title).isEqualTo(TITLES[3]); // appAlpha
+        assertThat(results.get(2).title).isEqualTo(TITLES[4]); // appBravo
+        assertThat(results.get(3).title).isEqualTo(TITLES[1]); // bravo
+        assertThat(results.get(4).title).isEqualTo(TITLES[5]); // appCharlie
+        assertThat(results.get(5).title).isEqualTo(TITLES[2]); // charlie
         assertThat(count).isEqualTo(6);
     }
 
@@ -146,26 +171,46 @@ public class SearchResultsAdapterTest {
     @Test
     public void testDisplayResults_ShouldRunSmartRankingIfEnabled() {
         when(mSearchFeatureProvider.isSmartSearchRankingEnabled(any()))
-            .thenReturn(true);
+                .thenReturn(true);
         mAdapter.displaySearchResults("");
         verify(mSearchFeatureProvider, times(1)).rankSearchResults(anyString(), anyList());
     }
 
+    @Test
+    public void testEndToEndSearch_removeResults_resultsAdded() {
+        List<AppSearchResult> appResults = getDummyAppResults();
+        List<SearchResult> dbResults = getDummyDbResults();
+        // Add list of items
+        mAdapter.addSearchResults(appResults, InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(dbResults, DatabaseResultLoader.class.getName());
+        mAdapter.displaySearchResults("");
+        // Add subset of items
+        mAdapter.addSearchResults(appResults.subList(0,1),
+                InstalledAppResultLoader.class.getName());
+        mAdapter.addSearchResults(dbResults.subList(0,1), DatabaseResultLoader.class.getName());
+        int count = mAdapter.displaySearchResults("");
+
+        List<SearchResult> results = mAdapter.getSearchResults();
+        assertThat(results.get(0).title).isEqualTo(TITLES[0]);
+        assertThat(results.get(1).title).isEqualTo(TITLES[3]);
+        assertThat(count).isEqualTo(2);
+    }
+
     private List<SearchResult> getDummyDbResults() {
         List<SearchResult> results = new ArrayList<>();
         ResultPayload payload = new ResultPayload(new Intent());
         SearchResult.Builder builder = new SearchResult.Builder();
         builder.addPayload(payload);
 
-        builder.addTitle("alpha")
+        builder.addTitle(TITLES[0])
                 .addRank(1);
         results.add(builder.build());
 
-        builder.addTitle("bravo")
+        builder.addTitle(TITLES[1])
                 .addRank(3);
         results.add(builder.build());
 
-        builder.addTitle("Charlie")
+        builder.addTitle(TITLES[2])
                 .addRank(6);
         results.add(builder.build());
 
@@ -178,15 +223,15 @@ public class SearchResultsAdapterTest {
         AppSearchResult.Builder builder = new AppSearchResult.Builder();
         builder.addPayload(payload);
 
-        builder.addTitle("appAlpha")
+        builder.addTitle(TITLES[3])
                 .addRank(1);
         results.add(builder.build());
 
-        builder.addTitle("appBravo")
+        builder.addTitle(TITLES[4])
                 .addRank(2);
         results.add(builder.build());
 
-        builder.addTitle("appCharlie")
+        builder.addTitle(TITLES[5])
                 .addRank(4);
         results.add(builder.build());