Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. r=sebastian
authorJan Henning <jh+bugzilla@buttercookie.de>
Sun, 26 Feb 2017 13:39:49 +0100
changeset 394351 f1b38b06e80c2bcadf738cfe58c0f9162dc0d971
parent 394350 98ae7b16f95de0d5f544c4a32fd2c3744eadde9e
child 394352 b4bb760c8767d3afc769648fcb697da6f86b9a8a
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1342718
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1342718 - Don't query for search engine suggestions if we're not displaying them. r=sebastian 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
@@ -167,16 +167,17 @@ public class GeckoPreferences
     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_SHOW_QUIT_MENU = NON_PREF_PREFIX + "distribution.show_quit_menu";
+    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";
@@ -1243,16 +1244,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) {