Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. r=sebastian, a=lizzard
authorJan Henning <jh+bugzilla@buttercookie.de>
Sun, 26 Feb 2017 13:39:49 +0100
changeset 376602 3dadcef3629a9e3cce5a47116cad84591220c087
parent 376601 ccfe0752e919fef62cb476e3fd16fe13db49b96d
child 376603 1c85043498f0975b39d43e53f1a12029f5ff2e57
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian, lizzard
bugs1342718
milestone53.0a2
Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. r=sebastian, a=lizzard If the user has deactivated search suggestions (either live suggestions from the search engine or those coming from our history), we shouldn't even bother to restart the corresponding loader in that case, so as to avoid - wasting processing and network resources - and perhaps more importantly, not leaking the user's search terms to the default search engine if the user doesn't want that kind of suggestions. At the moment we only exit early from filterSuggestions() when in private mode or if both kinds of search suggestions have been deactivated, but we don't properly handle the case where only one kind of search suggestions has been deactivated. This should also improve the display of search history results if the user has deactivated the display of live search suggestions, since currently duplicates between the fresh suggestions and the search history are always removed from the latter even when we're not displaying the former. MozReview-Commit-ID: IOTMLRaZeyP
mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
mobile/android/base/java/org/mozilla/gecko/home/SearchEngineRow.java
mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java
@@ -662,26 +662,32 @@ public class BrowserSearch extends HomeF
         // 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();
+        if (mSuggestionsEnabled) {
+            if (mSearchEngineSuggestionLoaderCallbacks == null) {
+                mSearchEngineSuggestionLoaderCallbacks = new SearchEngineSuggestionLoaderCallbacks();
+            }
+            getLoaderManager().restartLoader(LOADER_ID_SUGGESTION, null, mSearchEngineSuggestionLoaderCallbacks);
         }
-        getLoaderManager().restartLoader(LOADER_ID_SUGGESTION, null, mSearchEngineSuggestionLoaderCallbacks);
 
         // Saved suggestions
-        if (mSearchHistorySuggestionLoaderCallback == null) {
-            mSearchHistorySuggestionLoaderCallback = new SearchHistorySuggestionLoaderCallbacks();
+        if (mSavedSearchesEnabled) {
+            if (mSearchHistorySuggestionLoaderCallback == null) {
+                mSearchHistorySuggestionLoaderCallback = new SearchHistorySuggestionLoaderCallbacks();
+            }
+            getLoaderManager().restartLoader(LOADER_ID_SAVED_SUGGESTION, null, mSearchHistorySuggestionLoaderCallback);
+        } else {
+            mSearchHistorySuggestions.clear();
         }
-        getLoaderManager().restartLoader(LOADER_ID_SAVED_SUGGESTION, null, mSearchHistorySuggestionLoaderCallback);
     }
 
     private void setSuggestions(ArrayList<String> suggestions) {
         ThreadUtils.assertOnUiThread();
 
         // mSearchEngines may be null if the setSuggestions calls after onDestroy (bug 1310621).
         // So drop the suggestions if search engines are not available
         if (mSearchEngines != null && !mSearchEngines.isEmpty()) {
@@ -868,24 +874,24 @@ public class BrowserSearch extends HomeF
             return;
         }
 
         // Make suggestions appear immediately after the user opts in
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
                 SuggestClient client = mSuggestClient;
-                if (client != null) {
+                if (client != null && enabled) {
                     client.query(mSearchTerm);
                 }
             }
         });
 
         PrefsHelper.setPref("browser.search.suggest.prompted", true);
-        PrefsHelper.setPref("browser.search.suggest.enabled", enabled);
+        PrefsHelper.setPref(GeckoPreferences.PREFS_SEARCH_SUGGESTIONS_ENABLED, enabled);
 
         Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.BUTTON, (enabled ? "suggestions_optin_yes" : "suggestions_optin_no"));
 
         TranslateAnimation slideAnimation = new TranslateAnimation(0, mSuggestionsOptInPrompt.getWidth(), 0, 0);
         slideAnimation.setDuration(ANIMATION_DURATION);
         slideAnimation.setInterpolator(new AccelerateInterpolator());
         slideAnimation.setFillAfter(true);
         final View prompt = mSuggestionsOptInPrompt.findViewById(R.id.prompt);
--- a/mobile/android/base/java/org/mozilla/gecko/home/SearchEngineRow.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/SearchEngineRow.java
@@ -403,19 +403,21 @@ class SearchEngineRow extends AnimatedHe
 
             if (currentSearchHistory.length() > 50 || Pattern.matches("^(https?|ftp|file)://.*", currentSearchHistory)) {
                 searchHistoryIterator.remove();
             }
         }
 
 
         List<String> searchEngineSuggestions = new ArrayList<String>();
-        for (String suggestion : searchEngine.getSuggestions()) {
-            searchHistorySuggestions.remove(suggestion);
-            searchEngineSuggestions.add(suggestion);
+        if (searchSuggestionsEnabled) {
+            for (String suggestion : searchEngine.getSuggestions()) {
+                searchHistorySuggestions.remove(suggestion);
+                searchEngineSuggestions.add(suggestion);
+            }
         }
         // Make sure the search term itself isn't duplicated. This is more important on phones than tablets where screen
         // space is more precious.
         searchHistorySuggestions.remove(getSuggestionTextFromView(mUserEnteredView));
 
         // Trim the history suggestions down to the maximum allowed.
         if (searchHistorySuggestions.size() >= mMaxSavedSuggestions) {
             // The second index to subList() is exclusive, so this looks like an off by one error but it is not.
--- a/mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java
+++ b/mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java
@@ -170,16 +170,17 @@ public class GeckoPreferences
     public static final String PREFS_NOTIFICATIONS_WHATS_NEW = NON_PREF_PREFIX + "notifications.whats_new";
     public static final String PREFS_APP_UPDATE_LAST_BUILD_ID = "app.update.last_build_id";
     public static final String PREFS_READ_PARTNER_CUSTOMIZATIONS_PROVIDER = NON_PREF_PREFIX + "distribution.read_partner_customizations_provider";
     public static final String PREFS_READ_PARTNER_BOOKMARKS_PROVIDER = NON_PREF_PREFIX + "distribution.read_partner_bookmarks_provider";
     public static final String PREFS_CUSTOM_TABS = NON_PREF_PREFIX + "customtabs";
     public static final String PREFS_ACTIVITY_STREAM = NON_PREF_PREFIX + "activitystream";
     public static final String PREFS_CATEGORY_EXPERIMENTAL_FEATURES = NON_PREF_PREFIX + "category_experimental";
     public static final String PREFS_COMPACT_TABS = NON_PREF_PREFIX + "compact_tabs";
+    public static final String PREFS_SEARCH_SUGGESTIONS_ENABLED = "browser.search.suggest.enabled";
 
     private static final String ACTION_STUMBLER_UPLOAD_PREF = "STUMBLER_PREF";
 
 
     // This isn't a Gecko pref, even if it looks like one.
     private static final String PREFS_BROWSER_LOCALE = "locale";
 
     public static final String PREFS_RESTORE_SESSION = NON_PREF_PREFIX + "restoreSession3";
@@ -1238,16 +1239,20 @@ public class GeckoPreferences
                 @Override
                 public void run() {
                     GeckoAppShell.scheduleRestart();
                 }
             }, 1000);
         } else if (HANDLERS.containsKey(prefName)) {
             PrefHandler handler = HANDLERS.get(prefName);
             handler.onChange(this, preference, newValue);
+        } else if (PREFS_SEARCH_SUGGESTIONS_ENABLED.equals(prefName)) {
+            // Tell Gecko to transmit the current search engine data again, so
+            // BrowserSearch is notified immediately about the new enabled state.
+            EventDispatcher.getInstance().dispatch("SearchEngines:GetVisible", null);
         }
 
         // Send Gecko-side pref changes to Gecko
         if (isGeckoPref(prefName)) {
             PrefsHelper.setPref(prefName, newValue, true /* flush */);
         }
 
         if (preference instanceof ListPreference) {