Bug 1232651 - don't show search suggestions in private browsing r=mcomella draft
authorAndrzej Hunt <ahunt@mozilla.com>
Wed, 06 Jan 2016 15:43:13 -0800
changeset 327242 28f8d7b9c275a2576594ea8bc7fa0ba0a9f00bc7
parent 327241 75c6a6c1fec29d30999b7b0f6251ada5d0efc0c2
child 513670 4bd2789d7e9795e67cb6c0571d3d5de9448f5fa4
push id10208
push userahunt@mozilla.com
push dateFri, 29 Jan 2016 21:43:31 +0000
reviewersmcomella
bugs1232651
milestone47.0a1
Bug 1232651 - don't show search suggestions in private browsing r=mcomella We also remove the special case for updating mSuggestClient when search engines are modified during private browsing - this causes a weird edge case where we potentially show history suggestions after switching search engines (this seems to be because having mSuggestClient == null avoids us even reaching the stage where we usually recycle unneeded suggestions).
mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
@@ -248,17 +248,17 @@ public class BrowserSearch extends HomeF
         super.onDestroy();
 
         mSearchEngines = null;
     }
 
     @Override
     public void onHiddenChanged(boolean hidden) {
         if (!hidden) {
-            Tab tab = Tabs.getInstance().getSelectedTab();
+            final Tab tab = Tabs.getInstance().getSelectedTab();
             final boolean isPrivate = (tab != null && tab.isPrivate());
 
             // Removes Search Suggestions Loader if in private browsing mode
             // Loader may have been inserted when browsing in normal tab
             if (isPrivate) {
                 getLoaderManager().destroyLoader(LOADER_ID_SUGGESTION);
             }
 
@@ -588,17 +588,23 @@ public class BrowserSearch extends HomeF
         return searchDomains(searchTerm);
     }
 
     public void resetScrollState() {
         mSearchEngineBar.scrollToPosition(0);
     }
 
     private void filterSuggestions() {
-        if (mSuggestClient == null || (!mSuggestionsEnabled && !mSavedSearchesEnabled)) {
+        Tab tab = Tabs.getInstance().getSelectedTab();
+        final boolean isPrivate = (tab != null && tab.isPrivate());
+
+        // mSuggestClient may be null if we haven't received our search engine list yet - hence
+        // we need to exit here in that case.
+        if (isPrivate || mSuggestClient == null || (!mSuggestionsEnabled && !mSavedSearchesEnabled)) {
+            mSearchHistorySuggestions.clear();
             return;
         }
 
         // Suggestions from search engine
         if (mSearchEngineSuggestionLoaderCallbacks == null) {
             mSearchEngineSuggestionLoaderCallbacks = new SearchEngineSuggestionLoaderCallbacks();
         }
         getLoaderManager().restartLoader(LOADER_ID_SUGGESTION, null, mSearchEngineSuggestionLoaderCallbacks);
@@ -665,48 +671,42 @@ public class BrowserSearch extends HomeF
                 final SearchEngine engine = new SearchEngine((Context) getActivity(), engineJSON);
 
                 if (engine.name.equals(suggestEngine) && suggestTemplate != null) {
                     // Suggest engine should be at the front of the list.
                     // We're baking in an assumption here that the suggest engine
                     // is also the default engine.
                     searchEngines.add(0, engine);
 
-                    // The only time Tabs.getInstance().getSelectedTab() should
-                    // be null is when we're restoring after a crash. We should
-                    // never restore private tabs when that happens, so it
-                    // should be safe to assume that null means non-private.
-                    Tab tab = Tabs.getInstance().getSelectedTab();
-                    final boolean isPrivate = (tab != null && tab.isPrivate());
-
-                    // Only create a new instance of SuggestClient if it hasn't been
-                    // set yet.
-                    maybeSetSuggestClient(suggestTemplate, isPrivate);
+                    ensureSuggestClientIsSet(suggestTemplate);
                 } else {
                     searchEngines.add(engine);
                 }
             }
 
             // checking if the new searchEngine is different from mSearchEngine, will have to re-layout if yes
             boolean change = shouldUpdateSearchEngine(searchEngines);
 
             if (mAdapter != null && change) {
                 mSearchEngines = Collections.unmodifiableList(searchEngines);
                 mLastLocale = Locale.getDefault();
                 updateSearchEngineBar();
 
                 mAdapter.notifyDataSetChanged();
             }
 
+            final Tab tab = Tabs.getInstance().getSelectedTab();
+            final boolean isPrivate = (tab != null && tab.isPrivate());
+
             // Show suggestions opt-in prompt only if suggestions are not enabled yet,
             // user hasn't been prompted and we're not on a private browsing tab.
             // The prompt might have been inflated already when this view was previously called.
             // Remove the opt-in prompt if it has been inflated in the view and dealt with by the user,
             // or if we're on a private browsing tab
-            if (!mSuggestionsEnabled && !suggestionsPrompted && mSuggestClient != null) {
+            if (!mSuggestionsEnabled && !suggestionsPrompted && !isPrivate) {
                 showSuggestionsOptIn();
             } else {
                 removeSuggestionsOptIn();
             }
 
         } catch (JSONException e) {
             Log.e(LOGTAG, "Error getting search engine JSON", e);
         }
@@ -730,22 +730,17 @@ public class BrowserSearch extends HomeF
     @Override
     public void onSearchBarClickListener(final SearchEngine searchEngine) {
         Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.LIST_ITEM,
                 "searchenginebar");
 
         mSearchListener.onSearch(searchEngine, mSearchTerm);
     }
 
-    private void maybeSetSuggestClient(final String suggestTemplate, final boolean isPrivate) {
-        if (isPrivate) {
-            mSuggestClient = null;
-            return;
-        }
-
+    private void ensureSuggestClientIsSet(final String suggestTemplate) {
         if (mSuggestClient != null) {
             return;
         }
 
         mSuggestClient = sSuggestClientFactory.getSuggestClient(getActivity(), suggestTemplate, SUGGESTION_TIMEOUT, NETWORK_SUGGESTION_MAX);
     }
 
     private void showSuggestionsOptIn() {