Bug 1464454 - Expire adaptive history after 90 days and limit the number of top adaptive history matches in the Address Bar. r=adw

Reduce adaptive history domination of the Address Bar results by expiring unused
entries sooner and limiting the number of top adaptive results.

MozReview-Commit-ID: EGOs6rVYGj6

--HG--
rename : toolkit/components/places/tests/unit/test_adaptive.js => toolkit/components/places/tests/unifiedcomplete/test_adaptive.js
rename : toolkit/components/places/tests/unit/test_adaptive_bug527311.js => toolkit/components/places/tests/unifiedcomplete/test_adaptive_behaviors.js
extra : rebase_source : 490d6373b2001101b961e4d2d12b6c02272300a4

toolkit/components/places/UnifiedComplete.js

@@ -60,6 +60,7 @@ const PREF_OTHER_DEFAULTS = new Map([
 const QUERYTYPE_FILTERED            = 0;
 const QUERYTYPE_AUTOFILL_ORIGIN     = 1;
 const QUERYTYPE_AUTOFILL_URL        = 2;
+const QUERYTYPE_ADAPTIVE            = 3;
 
 // This separator is used as an RTL-friendly way to split the title and tags.
 // It can also be used by an nsIAutoCompleteResult consumer to re-split the
@@ -930,6 +931,10 @@ function Search(searchString, searchParam, autocompleteListener,
     }
   }
 
+  // Used to limit the number of adaptive results.
+  this._adaptiveCount = 0;
+  this._extraAdaptiveRows = [];
+
   // This is a replacement for this._result.matchCount, to be used when you need
   // to check how many "current" matches have been inserted.
   // Indeed this._result.matchCount may include matches from the previous search.
@@ -1211,6 +1216,12 @@ Search.prototype = {
         return;
     }
 
+    // If we have some unused adaptive matches, add them now.
+    while (this._extraAdaptiveRows.length &&
+           this._currentMatchCount < Prefs.get("maxRichResults")) {
+      this._addFilteredQueryMatch(this._extraAdaptiveRows.shift());
+    }
+
     // Ideally we should wait until MATCH_BOUNDARY_ANYWHERE, but that query
     // may be really slow and we may end up showing old results for too long.
     this._cleanUpNonCurrentMatches(MATCHTYPE.GENERAL);
@@ -1806,6 +1817,9 @@ Search.prototype = {
         this._result.setDefaultIndex(0);
         this._addURLAutofillMatch(row);
         break;
+      case QUERYTYPE_ADAPTIVE:
+        this._addAdaptiveQueryMatch(row);
+        break;
       case QUERYTYPE_FILTERED:
         this._addFilteredQueryMatch(row);
         break;
@@ -2116,6 +2130,22 @@ Search.prototype = {
     });
   },
 
+  // This is the same as _addFilteredQueryMatch, but it only returns a few
+  // results, caching the others. If at the end we don't find other results, we
+  // can add these.
+  _addAdaptiveQueryMatch(row) {
+    // Allow one quarter of the results to be adaptive results.
+    // Note: ideally adaptive results should have their own provider and the
+    // results muxer should decide what to show.  But that's too complex to
+    // support in the current code, so that's left for a future refactoring.
+    if (this._adaptiveCount < Math.ceil(Prefs.get("maxRichResults") / 4)) {
+      this._addFilteredQueryMatch(row);
+    } else {
+      this._extraAdaptiveRows.push(row);
+    }
+    this._adaptiveCount++;
+  },
+
   _addFilteredQueryMatch(row) {
     let match = {};
     match.placeId = row.getResultByIndex(QUERYINDEX_PLACEID);
@@ -2278,7 +2308,7 @@ Search.prototype = {
       {
         parent: PlacesUtils.tagsFolderId,
         search_string: this._searchString,
-        query_type: QUERYTYPE_FILTERED,
+        query_type: QUERYTYPE_ADAPTIVE,
         matchBehavior: this._matchBehavior,
         searchBehavior: this._behavior,
         userContextId: this._userContextId,

toolkit/components/places/nsNavHistory.cpp

@@ -110,6 +110,8 @@ using namespace mozilla::places;
 // This is a 'hidden' pref for the purposes of unit tests.
 #define PREF_FREC_DECAY_RATE     "places.frecency.decayRate"
 #define PREF_FREC_DECAY_RATE_DEF 0.975f
+// An adaptive history entry is removed if unused for these many days.
+#define ADAPTIVE_HISTORY_EXPIRE_DAYS 90
 
 // In order to avoid calling PR_now() too often we use a cached "now" value
 // for repeating stuff.  These are milliseconds between "now" cache refreshes.
@@ -2521,7 +2523,12 @@ nsNavHistory::DecayFrecency()
   nsresult rv = FixInvalidFrecencies();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  float decayRate = Preferences::GetFloat(PREF_FREC_DECAY_RATE, PREF_FREC_DECAY_RATE_DEF);
+  float decayRate = Preferences::GetFloat(PREF_FREC_DECAY_RATE,
+                                          PREF_FREC_DECAY_RATE_DEF);
+  if (decayRate > 1.0f) {
+    MOZ_ASSERT(false, "The frecency decay rate should not be greater than 1.0");
+    decayRate = PREF_FREC_DECAY_RATE_DEF;
+  }
 
   // Globally decay places frecency rankings to estimate reduced frecency
   // values of pages that haven't been visited for a while, i.e., they do
@@ -2534,7 +2541,6 @@ nsNavHistory::DecayFrecency()
     "WHERE frecency > 0"
   );
   NS_ENSURE_STATE(decayFrecency);
-
   rv = decayFrecency->BindDoubleByName(NS_LITERAL_CSTRING("decay_rate"),
                                        static_cast<double>(decayRate));
   NS_ENSURE_SUCCESS(rv, rv);
@@ -2542,15 +2548,22 @@ nsNavHistory::DecayFrecency()
   // Decay potentially unused adaptive entries (e.g. those that are at 1)
   // to allow better chances for new entries that will start at 1.
   nsCOMPtr<mozIStorageAsyncStatement> decayAdaptive = mDB->GetAsyncStatement(
-    "UPDATE moz_inputhistory SET use_count = use_count * .975"
+    "UPDATE moz_inputhistory SET use_count = use_count * :decay_rate"
   );
   NS_ENSURE_STATE(decayAdaptive);
+  rv = decayAdaptive->BindDoubleByName(NS_LITERAL_CSTRING("decay_rate"),
+                                       static_cast<double>(decayRate));
+  NS_ENSURE_SUCCESS(rv, rv);
 
   // Delete any adaptive entries that won't help in ordering anymore.
   nsCOMPtr<mozIStorageAsyncStatement> deleteAdaptive = mDB->GetAsyncStatement(
-    "DELETE FROM moz_inputhistory WHERE use_count < .01"
+    "DELETE FROM moz_inputhistory WHERE use_count < :use_count"
   );
   NS_ENSURE_STATE(deleteAdaptive);
+  rv = deleteAdaptive->BindDoubleByName(NS_LITERAL_CSTRING("use_count"),
+                                        std::pow(static_cast<double>(decayRate),
+                                                 ADAPTIVE_HISTORY_EXPIRE_DAYS));
+  NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<mozIStorageConnection> conn = mDB->MainConn();
   if (!conn) {
@@ -2563,8 +2576,7 @@ nsNavHistory::DecayFrecency()
   };
   nsCOMPtr<mozIStoragePendingStatement> ps;
   RefPtr<PlacesDecayFrecencyCallback> cb = new PlacesDecayFrecencyCallback();
-  rv = conn->ExecuteAsync(stmts, ArrayLength(stmts), cb,
-                                     getter_AddRefs(ps));
+  rv = conn->ExecuteAsync(stmts, ArrayLength(stmts), cb, getter_AddRefs(ps));
   NS_ENSURE_SUCCESS(rv, rv);
 
   mDecayFrecencyPendingCount++;

toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js

@@ -492,3 +492,28 @@ add_task(async function ensure_search_engine() {
   let engine = Services.search.getEngineByName("MozSearch");
   Services.search.currentEngine = engine;
 });
+
+/**
+ * Add a adaptive result for a given (url, string) tuple.
+ * @param {string} aUrl
+ *        The url to add an adaptive result for.
+ * @param {string} aSearch
+ *        The string to add an adaptive result for.
+ * @resolves When the operation is complete.
+ */
+function addAdaptiveFeedback(aUrl, aSearch) {
+  let promise = TestUtils.topicObserved("places-autocomplete-feedback-updated");
+  let thing = {
+    QueryInterface: ChromeUtils.generateQI([Ci.nsIAutoCompleteInput,
+                                            Ci.nsIAutoCompletePopup,
+                                            Ci.nsIAutoCompleteController]),
+    get popup() { return thing; },
+    get controller() { return thing; },
+    popupOpen: true,
+    selectedIndex: 0,
+    getValueAt: () => aUrl,
+    searchString: aSearch
+  };
+  Services.obs.notifyObservers(thing, "autocomplete-will-enter-text");
+  return promise;
+}

toolkit/components/places/tests/unifiedcomplete/test_adaptive.js

@@ -18,53 +18,6 @@
  * learning.
  */
 
-function AutoCompleteInput(aSearches) {
-  this.searches = aSearches;
-}
-AutoCompleteInput.prototype = {
-  constructor: AutoCompleteInput,
-
-  get minResultsForPopup() {
-    return 0;
-  },
-  get timeout() {
-    return 10;
-  },
-  get searchParam() {
-    return "";
-  },
-  get textValue() {
-    return "";
-  },
-  get disableAutoComplete() {
-    return false;
-  },
-  get completeDefaultIndex() {
-    return false;
-  },
-
-  get searchCount() {
-    return this.searches.length;
-  },
-  getSearchAt(aIndex) {
-    return this.searches[aIndex];
-  },
-
-  onSearchBegin() {},
-  onSearchComplete() {},
-
-  get popupOpen() {
-    return false;
-  },
-  popup: {
-    set selectedIndex(aIndex) {},
-    invalidate() {},
-    QueryInterface: ChromeUtils.generateQI([Ci.nsIAutoCompletePopup])
-  },
-
-  QueryInterface: ChromeUtils.generateQI([Ci.nsIAutoCompleteInput])
-};
-
 /**
  * Checks that autocomplete results are ordered correctly.
  */
@@ -80,8 +33,10 @@ function ensure_results(expected, searchTerm, callback) {
 
   input.onSearchComplete = function() {
     Assert.equal(controller.searchStatus,
-                 Ci.nsIAutoCompleteController.STATUS_COMPLETE_MATCH);
-    Assert.equal(controller.matchCount, expected.length);
+                 Ci.nsIAutoCompleteController.STATUS_COMPLETE_MATCH,
+                 "The search should be complete");
+    Assert.equal(controller.matchCount, expected.length,
+                 "All the expected results should have been found");
     for (let i = 0; i < controller.matchCount; i++) {
       print("Testing for '" + expected[i].uri.spec + "' got '" + controller.getValueAt(i) + "'");
       Assert.equal(controller.getValueAt(i), expected[i].uri.spec);
@@ -368,6 +323,21 @@ var tests = [
     await task_setCountRank(uri1, c1, c1, s2, "tag");
     await task_setCountRank(uri2, c1, c2, s2);
   },
+  // Test that many results are all shown if no other results are available.
+  async function() {
+    print("Test 14 -  many results");
+    let n = 10;
+    observer.results = Array(n).fill(0).map(
+      (e, i) => makeResult(Services.io.newURI("http://site.tld/" + i))
+    );
+    observer.search = s2;
+    observer.runCount = n * (n + 1) / 2;
+    let c = n;
+    for (let result of observer.results) {
+      task_setCountRank(result.uri, c, c, s2);
+      c--;
+    }
+  },
 ];
 
 /**
@@ -376,7 +346,12 @@ var tests = [
 add_task(async function test_adaptive() {
   // Disable autoFill for this test.
   Services.prefs.setBoolPref("browser.urlbar.autoFill", false);
-  registerCleanupFunction(() => Services.prefs.clearUserPref("browser.urlbar.autoFill"));
+
+  registerCleanupFunction(async function() {
+    await PlacesUtils.bookmarks.eraseEverything();
+    await PlacesUtils.history.clear();
+  });
+
   for (let test of tests) {
     // Cleanup.
     await PlacesUtils.bookmarks.eraseEverything();

toolkit/components/places/tests/unifiedcomplete/test_adaptive_behaviors.js

@@ -0,0 +1,40 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// Test for Bug 527311
+// Addressbar suggests adaptive results regardless of the requested behavior.
+
+const TEST_URL = "http://adapt.mozilla.org/";
+const SEARCH_STRING = "adapt";
+const SUGGEST_TYPES = ["history", "bookmark", "openpage"];
+
+add_task(async function test_adaptive_search_specific() {
+  Services.prefs.setBoolPref("browser.urlbar.autoFill", false);
+
+  // Add a bookmark to our url.
+  await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    title: "test_book",
+    url: TEST_URL,
+  });
+  registerCleanupFunction(async function() {
+    await PlacesUtils.bookmarks.eraseEverything();
+  });
+
+  // We want to search only history.
+  for (let type of SUGGEST_TYPES) {
+    type == "history" ? Services.prefs.setBoolPref("browser.urlbar.suggest." + type, true)
+                      : Services.prefs.setBoolPref("browser.urlbar.suggest." + type, false);
+  }
+
+  // Add an adaptive entry.
+  await addAdaptiveFeedback(TEST_URL, SEARCH_STRING);
+
+  await check_autocomplete({
+    search: SEARCH_STRING,
+    matches: [],
+  });
+});

toolkit/components/places/tests/unifiedcomplete/test_adaptive_limited.js

@@ -0,0 +1,41 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that top adaptive results are limited, remaining ones are enqueued.
+
+add_task(async function() {
+  Services.prefs.setBoolPref("browser.urlbar.autoFill", false);
+
+  let n = 10;
+  let uris = Array(n).fill(0).map((e, i) => "http://site.tld/" + i);
+
+  // Add a bookmark to one url.
+  let bm = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    title: "test_book",
+    url: uris.shift(),
+  });
+
+  // Make remaining ones adaptive results.
+  for (let uri of uris) {
+    await PlacesTestUtils.addVisits(uri);
+    await addAdaptiveFeedback(uri, "book");
+  }
+
+  registerCleanupFunction(async function() {
+    await PlacesUtils.bookmarks.eraseEverything();
+    await PlacesUtils.history.clear();
+  });
+
+  let matches = uris.map(uri => ({ uri: Services.io.newURI(uri),
+                                   title: "test visit for " + uri }));
+  let book_index = Math.ceil(Services.prefs.getIntPref("browser.urlbar.maxRichResults") / 4);
+  matches.splice(book_index, 0, { uri: Services.io.newURI(bm.url.href),
+                                  title: "test_book", "style": ["bookmark"] });
+
+  await check_autocomplete({
+    search: "book",
+    matches,
+    checkSorting: true,
+  });
+});

toolkit/components/places/tests/unifiedcomplete/xpcshell.ini

@@ -13,6 +13,9 @@ support-files =
 [test_417798.js]
 [test_418257.js]
 [test_422277.js]
+[test_adaptive.js]
+[test_adaptive_behaviors.js]
+[test_adaptive_limited.js]
 [test_autocomplete_functional.js]
 [test_autocomplete_stopSearch_no_throw.js]
 [test_autofill_origins.js]

toolkit/components/places/tests/unit/test_adaptive_bug527311.js

@@ -1,137 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-const TEST_URL = "http://adapt.mozilla.org/";
-const SEARCH_STRING = "adapt";
-const SUGGEST_TYPES = ["history", "bookmark", "openpage"];
-
-ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-
-const PLACES_AUTOCOMPLETE_FEEDBACK_UPDATED_TOPIC =
-  "places-autocomplete-feedback-updated";
-
-function cleanup() {
-  for (let type of SUGGEST_TYPES) {
-    Services.prefs.clearUserPref("browser.urlbar.suggest." + type);
-  }
-}
-
-function AutoCompleteInput(aSearches) {
-  this.searches = aSearches;
-}
-AutoCompleteInput.prototype = {
-  constructor: AutoCompleteInput,
-  searches: null,
-  minResultsForPopup: 0,
-  timeout: 10,
-  searchParam: "",
-  textValue: "",
-  disableAutoComplete: false,
-  completeDefaultIndex: false,
-
-  get searchCount() {
-    return this.searches.length;
-  },
-
-  getSearchAt: function ACI_getSearchAt(aIndex) {
-    return this.searches[aIndex];
-  },
-
-  onSearchComplete: function ACI_onSearchComplete() {},
-
-  popupOpen: false,
-
-  popup: {
-    setSelectedIndex() {},
-    invalidate() {},
-
-    QueryInterface: ChromeUtils.generateQI(["nsIAutoCompletePopup"])
-  },
-
-  onSearchBegin() {},
-
-  QueryInterface: ChromeUtils.generateQI(["nsIAutoCompleteInput"])
-};
-
-
-function check_results() {
-  return new Promise(resolve => {
-    let controller = Cc["@mozilla.org/autocomplete/controller;1"].
-                     getService(Ci.nsIAutoCompleteController);
-    let input = new AutoCompleteInput(["unifiedcomplete"]);
-    controller.input = input;
-
-    input.onSearchComplete = function() {
-      Assert.equal(controller.searchStatus,
-                   Ci.nsIAutoCompleteController.STATUS_COMPLETE_NO_MATCH);
-      Assert.equal(controller.matchCount, 0);
-
-      PlacesUtils.bookmarks.eraseEverything().then(() => {
-        cleanup();
-        resolve();
-      });
-    };
-
-    controller.startSearch(SEARCH_STRING);
-  });
-}
-
-
-function addAdaptiveFeedback(aUrl, aSearch) {
-  return new Promise(resolve => {
-    let observer = {
-      observe(aSubject, aTopic, aData) {
-        Services.obs.removeObserver(observer, PLACES_AUTOCOMPLETE_FEEDBACK_UPDATED_TOPIC);
-        do_timeout(0, resolve);
-      }
-    };
-    Services.obs.addObserver(observer, PLACES_AUTOCOMPLETE_FEEDBACK_UPDATED_TOPIC);
-
-    let thing = {
-      QueryInterface: ChromeUtils.generateQI([Ci.nsIAutoCompleteInput,
-                                              Ci.nsIAutoCompletePopup,
-                                              Ci.nsIAutoCompleteController]),
-      get popup() { return thing; },
-      get controller() { return thing; },
-      popupOpen: true,
-      selectedIndex: 0,
-      getValueAt: () => aUrl,
-      searchString: aSearch
-    };
-
-    Services.obs.notifyObservers(thing, "autocomplete-will-enter-text");
-  });
-}
-
-
-add_task(function init() {
-  Services.prefs.setBoolPref("browser.urlbar.autoFill", false);
-  registerCleanupFunction(() => {
-    Services.prefs.clearUserPref("browser.urlbar.autoFill");
-  });
-});
-
-add_task(async function test_adaptive_search_specific() {
-  // Add a bookmark to our url.
-  await PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-    title: "test_book",
-    url: TEST_URL,
-  });
-
-  // We want to search only history.
-  for (let type of SUGGEST_TYPES) {
-    type == "history" ? Services.prefs.setBoolPref("browser.urlbar.suggest." + type, true)
-                      : Services.prefs.setBoolPref("browser.urlbar.suggest." + type, false);
-  }
-
-  // Add an adaptive entry.
-  await addAdaptiveFeedback(TEST_URL, SEARCH_STRING);
-
-  await check_results();
-
-  await PlacesTestUtils.promiseAsyncUpdates();
-});

toolkit/components/places/tests/unit/xpcshell.ini

@@ -42,8 +42,6 @@ skip-if = os == "linux"
 [test_1085291.js]
 [test_1105208.js]
 [test_1105866.js]
-[test_adaptive.js]
-[test_adaptive_bug527311.js]
 [test_annotations.js]
 [test_asyncExecuteLegacyQueries.js]
 [test_async_transactions.js]