Clean up search fragment loader lifecycle.

- Programatically create SearchView to make it always expand across
  entire action bar.
- Store current query text during screen rotation
- Restart loader when query text changes

Bug: 33354491
Test: RunSettingsRoboTests
Change-Id: I63838a38514569aac60c5d67ac52ac06a7acd5a3

res/menu/search_options_menu.xml

@@ -1,23 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<!-- Copyright (C) 2016 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.
--->
-<menu xmlns:android="http://schemas.android.com/apk/res/android">
-    <item
-        android:id="@+id/search"
-        android:title="@string/search_menu"
-        android:icon="@*android:drawable/ic_search_api_material"
-        android:showAsAction="collapseActionView|ifRoom"
-        android:actionViewClass="android.widget.SearchView"/>
-</menu>

src/com/android/settings/dashboard/DashboardContainerFragment.java

@@ -28,9 +28,9 @@ import android.view.View;
 import android.view.ViewGroup;
 
 import com.android.internal.logging.nano.MetricsProto;
-import com.android.settings.core.InstrumentedFragment;
 import com.android.settings.R;
 import com.android.settings.SettingsActivity;
+import com.android.settings.core.InstrumentedFragment;
 import com.android.settings.core.instrumentation.MetricsFeatureProvider;
 import com.android.settings.overlay.FeatureFactory;
 import com.android.settings.overlay.SupportFeatureProvider;
@@ -77,7 +77,7 @@ public final class DashboardContainerFragment extends InstrumentedFragment {
 
         // check if support tab needs to be selected
         final String selectedTab = getArguments().
-            getString(EXTRA_SELECT_SETTINGS_TAB, ARG_SUMMARY_TAB);
+                getString(EXTRA_SELECT_SETTINGS_TAB, ARG_SUMMARY_TAB);
         if (TextUtils.equals(selectedTab, ARG_SUPPORT_TAB)) {
             mViewPager.setCurrentItem(INDEX_SUPPORT_FRAGMENT);
         } else {

src/com/android/settings/dashboard/DashboardSummary.java

@@ -22,7 +22,6 @@ import android.os.AsyncTask;
 import android.os.Bundle;
 import android.os.Handler;
 import android.support.v7.widget.LinearLayoutManager;
-import android.support.v7.widget.SimpleItemAnimator;
 import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;

src/com/android/settings/search2/SearchFeatureProvider.java

@@ -16,7 +16,7 @@
 package com.android.settings.search2;
 
 import android.app.Activity;
-import android.widget.SearchView;
+import android.content.Context;
 import android.view.Menu;
 
 /**
@@ -31,8 +31,14 @@ public interface SearchFeatureProvider {
 
     /**
      * Inserts the Menu items into Settings activity.
+     *
      * @param menu Items will be inserted into this menu.
      * @param activity The activity that precedes SearchActivity.
      */
     void setUpSearchMenu(Menu menu, Activity activity);
+
+    /**
+     * Returns a new loader to search in index database.
+     */
+    DatabaseResultLoader getDatabaseSearchLoader(Context context, String query);
 }

src/com/android/settings/search2/SearchFeatureProviderImpl.java

@@ -24,6 +24,9 @@ import android.view.Menu;
 
 import android.view.MenuItem;
 import com.android.settings.R;
+import com.android.settings.utils.AsyncLoader;
+
+import java.util.List;
 
 /**
  * FeatureProvider for the refactored search code.
@@ -60,4 +63,9 @@ public class SearchFeatureProviderImpl implements SearchFeatureProvider {
 
         menuItem.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM);
     }
+
+    @Override
+    public DatabaseResultLoader getDatabaseSearchLoader(Context context, String query) {
+        return new DatabaseResultLoader(context, query);
+    }
 }

src/com/android/settings/search2/SearchFragment.java

@@ -16,107 +16,110 @@
 
 package com.android.settings.search2;
 
+import android.app.ActionBar;
 import android.app.Activity;
+import android.app.LoaderManager;
+import android.content.Context;
 import android.content.Loader;
 import android.os.Bundle;
-import android.app.LoaderManager;
+import android.support.annotation.VisibleForTesting;
 import android.support.v7.widget.LinearLayoutManager;
 import android.support.v7.widget.RecyclerView;
+import android.text.TextUtils;
 import android.view.LayoutInflater;
-import android.view.MenuItem;
 import android.view.View;
 import android.view.ViewGroup;
-import android.view.Menu;
+import android.widget.LinearLayout.LayoutParams;
-import android.view.MenuInflater;
 import android.widget.SearchView;
 
 import com.android.internal.logging.nano.MetricsProto;
 import com.android.settings.R;
 import com.android.settings.core.InstrumentedFragment;
+import com.android.settings.overlay.FeatureFactory;
 
 import java.util.List;
 
 public class SearchFragment extends InstrumentedFragment implements
-        SearchView.OnQueryTextListener, MenuItem.OnActionExpandListener,
+        SearchView.OnQueryTextListener, LoaderManager.LoaderCallbacks<List<SearchResult>> {
-        LoaderManager.LoaderCallbacks<List<SearchResult>>  {
 
+    // State values
+    static final String STATE_QUERY = "query";
+
+    // Loader IDs
     private static final int DATABASE_LOADER_ID = 0;
 
-    private SearchResultsAdapter mSearchAdapter;
+    @VisibleForTesting
+    String mQuery;
 
+    private SearchFeatureProvider mSearchFeatureProvider;
     private DatabaseResultLoader mSearchLoader;
 
+    private SearchResultsAdapter mSearchAdapter;
     private RecyclerView mResultsRecyclerView;
-    private SearchView mSearchView;
-    private MenuItem mSearchMenuItem;
 
-    private String mQuery;
+    @Override
+    public int getMetricsCategory() {
+        return MetricsProto.MetricsEvent.DASHBOARD_SEARCH_RESULTS;
+    }
+
+    @Override
+    public void onAttach(Context context) {
+        super.onAttach(context);
+        mSearchFeatureProvider = FeatureFactory.getFactory(context)
+                .getSearchFeatureProvider(context);
+    }
 
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
         setHasOptionsMenu(true);
-
         mSearchAdapter = new SearchResultsAdapter();
-
+        if (savedInstanceState != null) {
-        final LoaderManager loaderManager = getLoaderManager();
+            mQuery = savedInstanceState.getString(STATE_QUERY);
-        loaderManager.initLoader(DATABASE_LOADER_ID, null, this);
+            getLoaderManager().initLoader(DATABASE_LOADER_ID, null, this);
+        }
+        final ActionBar actionBar = getActivity().getActionBar();
+        actionBar.setCustomView(makeSearchView(actionBar, mQuery));
+        actionBar.setDisplayShowCustomEnabled(true);
+        actionBar.setDisplayShowTitleEnabled(false);
     }
 
     @Override
     public View onCreateView(LayoutInflater inflater, ViewGroup container,
-                             Bundle savedInstanceState) {
+            Bundle savedInstanceState) {
         final View view = inflater.inflate(R.layout.search_panel_2, container, false);
         mResultsRecyclerView = (RecyclerView) view.findViewById(R.id.list_results);
-
         mResultsRecyclerView.setAdapter(mSearchAdapter);
         mResultsRecyclerView.setLayoutManager(new LinearLayoutManager(getActivity()));
         return view;
     }
 
     @Override
-    public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
+    public void onSaveInstanceState(Bundle outState) {
-        super.onCreateOptionsMenu(menu, inflater);
+        super.onSaveInstanceState(outState);
-        inflater.inflate(R.menu.search_options_menu, menu);
+        outState.putString(STATE_QUERY, mQuery);
-
-
-        mSearchMenuItem = menu.findItem(R.id.search);
-
-        mSearchView = (SearchView) mSearchMenuItem.getActionView();
-        mSearchView.setOnQueryTextListener(this);
-        mSearchView.setMaxWidth(Integer.MAX_VALUE);
-        mSearchMenuItem.expandActionView();
-    }
-
-    @Override
-    public boolean onMenuItemActionExpand(MenuItem item) {
-        return true;
-    }
-
-    @Override
-    public boolean onMenuItemActionCollapse(MenuItem item) {
-        // Return false to prevent the search box from collapsing.
-        return false;
     }
 
     @Override
     public boolean onQueryTextChange(String query) {
-        if (query == null || query.equals(mQuery)) {
+        if (TextUtils.equals(query, mQuery)) {
-            return false;
+            return true;
         }
-
         mQuery = query;
-        clearLoaders();
+        mSearchAdapter.clearResults();
 
-        final LoaderManager loaderManager = getLoaderManager();
+        if (TextUtils.isEmpty(mQuery)) {
-        loaderManager.restartLoader(DATABASE_LOADER_ID, null, this);
+            getLoaderManager().destroyLoader(DATABASE_LOADER_ID);
+        } else {
+            restartLoaders();
+        }
 
         return true;
     }
 
     @Override
     public boolean onQueryTextSubmit(String query) {
-        return false;
+        return true;
     }
 
     @Override
@@ -125,7 +128,7 @@ public class SearchFragment extends InstrumentedFragment implements
 
         switch (id) {
             case DATABASE_LOADER_ID:
-                mSearchLoader = new DatabaseResultLoader(activity, mQuery);
+                mSearchLoader = mSearchFeatureProvider.getDatabaseSearchLoader(activity, mQuery);
                 return mSearchLoader;
             default:
                 return null;
@@ -142,17 +145,22 @@ public class SearchFragment extends InstrumentedFragment implements
     }
 
     @Override
-    public void onLoaderReset(Loader<List<SearchResult>> loader) { }
+    public void onLoaderReset(Loader<List<SearchResult>> loader) {
-
-    @Override
-    public int getMetricsCategory() {
-        return MetricsProto.MetricsEvent.DASHBOARD_SEARCH_RESULTS;
     }
 
-    private void clearLoaders() {
+    private void restartLoaders() {
-        if (mSearchLoader != null) {
+        final LoaderManager loaderManager = getLoaderManager();
-            mSearchLoader.cancelLoad();
+        loaderManager.restartLoader(DATABASE_LOADER_ID, null /* args */, this /* callback */);
-            mSearchLoader = null;
+    }
-        }
+
+    private SearchView makeSearchView(ActionBar actionBar, String query) {
+        final SearchView searchView = new SearchView(actionBar.getThemedContext());
+        searchView.setIconifiedByDefault(false);
+        searchView.setQuery(query, false /* submitQuery */);
+        searchView.setOnQueryTextListener(this);
+        final LayoutParams lp =
+                new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT);
+        searchView.setLayoutParams(lp);
+        return searchView;
     }
 }

src/com/android/settings/search2/SearchResultsAdapter.java

@@ -28,10 +28,11 @@ import com.android.settings.search2.ResultPayload.PayloadType;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 public class SearchResultsAdapter extends Adapter<SearchViewHolder> {
-    private ArrayList<SearchResult> mSearchResults;
+    private final List<SearchResult> mSearchResults;
-    private HashMap<String, List<SearchResult>> mResultsMap;
+    private final Map<String, List<SearchResult>> mResultsMap;
 
     public SearchResultsAdapter() {
         mSearchResults = new ArrayList<>();
@@ -45,13 +46,19 @@ public class SearchResultsAdapter extends Adapter<SearchViewHolder> {
             return;
         }
         mResultsMap.put(loaderClassName, freshResults);
-        mSearchResults = mergeMappedResults();
+        mSearchResults.addAll(mergeMappedResults());
+        notifyDataSetChanged();
+    }
+
+    public void clearResults() {
+        mSearchResults.clear();
+        mResultsMap.clear();
         notifyDataSetChanged();
     }
 
     private ArrayList<SearchResult> mergeMappedResults() {
         ArrayList<SearchResult> mergedResults = new ArrayList<>();
-        for(String key : mResultsMap.keySet()) {
+        for (String key : mResultsMap.keySet()) {
             mergedResults.addAll(mResultsMap.get(key));
         }
         return mergedResults;
@@ -60,7 +67,7 @@ public class SearchResultsAdapter extends Adapter<SearchViewHolder> {
     @Override
     public SearchViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
         LayoutInflater inflater = LayoutInflater.from(parent.getContext());
-        switch(viewType) {
+        switch (viewType) {
             case PayloadType.INTENT:
                 View view = inflater.inflate(R.layout.search_intent_item, parent, false);
                 return new IntentSearchViewHolder(view);
@@ -95,7 +102,7 @@ public class SearchResultsAdapter extends Adapter<SearchViewHolder> {
     }
 
     @VisibleForTesting
-    public ArrayList<SearchResult> getSearchResults() {
+    public List<SearchResult> getSearchResults() {
         return mSearchResults;
     }
 }

tests/robotests/src/com/android/settings/search/SearchAdapterTest.java

@@ -20,20 +20,25 @@ package com.android.settings.search;
 import android.app.Activity;
 import android.content.Context;
 import android.graphics.drawable.Drawable;
+
+import com.android.settings.R;
 import com.android.settings.SettingsRobolectricTestRunner;
 import com.android.settings.TestConfig;
-import com.android.settings.search2.*;
+import com.android.settings.search2.DatabaseResultLoader;
+import com.android.settings.search2.IntentPayload;
+import com.android.settings.search2.ResultPayload;
+import com.android.settings.search2.SearchResult;
 import com.android.settings.search2.SearchResult.Builder;
-import com.android.settings.R;
+import com.android.settings.search2.SearchResultsAdapter;
-
-import java.util.ArrayList;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-
-import org.robolectric.annotation.Config;
 import org.robolectric.Robolectric;
+import org.robolectric.annotation.Config;
+
+import java.util.ArrayList;
+import java.util.List;
 
 import static com.google.common.truth.Truth.assertThat;
 
@@ -78,7 +83,7 @@ public class SearchAdapterTest {
 
     @Test
     public void testNoResultsAdded_EmptyListReturned() {
-        ArrayList<SearchResult> updatedResults = mAdapter.getSearchResults();
+        List<SearchResult> updatedResults = mAdapter.getSearchResults();
         assertThat(updatedResults).isEmpty();
     }
 
@@ -88,7 +93,7 @@ public class SearchAdapterTest {
         ArrayList<SearchResult> intentResults = getIntentSampleResults();
         mAdapter.mergeResults(intentResults, mLoaderClassName);
 
-        ArrayList<SearchResult> updatedResults = mAdapter.getSearchResults();
+        List<SearchResult> updatedResults = mAdapter.getSearchResults();
         assertThat(updatedResults).containsAllIn(intentResults);
     }
 
@@ -98,7 +103,7 @@ public class SearchAdapterTest {
         mAdapter.mergeResults(intentResults, mLoaderClassName);
         mAdapter.mergeResults(intentResults, mLoaderClassName);
 
-        ArrayList<SearchResult> updatedResults = mAdapter.getSearchResults();
+        List<SearchResult> updatedResults = mAdapter.getSearchResults();
         assertThat(updatedResults).containsAllIn(intentResults);
     }
 }

tests/robotests/src/com/android/settings/search2/SearchFragmentTest.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright (C) 2016 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.content.Context;
+import android.os.Bundle;
+
+import com.android.settings.R;
+import com.android.settings.SettingsRobolectricTestRunner;
+import com.android.settings.TestConfig;
+import com.android.settings.testutils.FakeFeatureFactory;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Answers;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.Robolectric;
+import org.robolectric.annotation.Config;
+import org.robolectric.util.ActivityController;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@RunWith(SettingsRobolectricTestRunner.class)
+@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
+public class SearchFragmentTest {
+
+    @Mock(answer = Answers.RETURNS_DEEP_STUBS)
+    private Context mContext;
+    @Mock
+    private DatabaseResultLoader mDatabaseResultLoader;
+    private FakeFeatureFactory mFeatureFactory;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+        FakeFeatureFactory.setupForTest(mContext);
+        mFeatureFactory = (FakeFeatureFactory) FakeFeatureFactory.getFactory(mContext);
+    }
+
+    @Test
+    public void screenRotate_shouldPersistQuery() {
+        when(mFeatureFactory.searchFeatureProvider
+                .getDatabaseSearchLoader(any(Context.class), anyString()))
+                .thenReturn(mDatabaseResultLoader);
+
+        final Bundle bundle = new Bundle();
+        final String testQuery = "test";
+        ActivityController<SearchActivity> activityController =
+                Robolectric.buildActivity(SearchActivity.class);
+        activityController.setup();
+        SearchFragment fragment = (SearchFragment) activityController.get().getFragmentManager()
+                .findFragmentById(R.id.main_content);
+
+        fragment.mQuery = testQuery;
+
+        activityController.saveInstanceState(bundle).pause().stop().destroy();
+
+        activityController = Robolectric.buildActivity(SearchActivity.class);
+        activityController.setup(bundle);
+
+        verify(mFeatureFactory.searchFeatureProvider)
+                .getDatabaseSearchLoader(any(Context.class), anyString());
+    }
+
+    @Test
+    public void queryTextChange_shouldTriggerLoader() {
+        when(mFeatureFactory.searchFeatureProvider
+                .getDatabaseSearchLoader(any(Context.class), anyString()))
+                .thenReturn(mDatabaseResultLoader);
+
+        final String testQuery = "test";
+        ActivityController<SearchActivity> activityController =
+                Robolectric.buildActivity(SearchActivity.class);
+        activityController.setup();
+        SearchFragment fragment = (SearchFragment) activityController.get().getFragmentManager()
+                .findFragmentById(R.id.main_content);
+
+        fragment.onQueryTextChange(testQuery);
+
+        verify(mFeatureFactory.searchFeatureProvider)
+                .getDatabaseSearchLoader(any(Context.class), anyString());
+    }
+}