Bug 1336233 - Fix intermittent failures in browser_multi_redirect_frecency.js by ensuring the idle service updates for frecency are ignored. r=mak
authorMark Banner <standard8@mozilla.com>
Mon, 06 Feb 2017 14:16:20 +0000
changeset 341449 6c6a814dd5af372e8c8bd9b46006ed0236cc0e7c
parent 341448 74050c8ec23b9fee58c70c16f182c2dc1524f797
child 341450 3711f6ee47a8ba486c72f409ab9f4d70738321ba
push id86727
push userkwierso@gmail.com
push dateThu, 09 Feb 2017 00:21:26 +0000
treeherdermozilla-inbound@55a4f5189115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1336233
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 1336233 - Fix intermittent failures in browser_multi_redirect_frecency.js by ensuring the idle service updates for frecency are ignored. r=mak MozReview-Commit-ID: L8AgWOAGF9g
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/tests/PlacesTestUtils.jsm
toolkit/components/places/tests/browser/browser_multi_redirect_frecency.js
toolkit/components/places/tests/browser/browser_redirect.js
toolkit/components/places/tests/unit/test_frecency_decay.js
toolkit/components/places/tests/unit/xpcshell.ini
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -98,16 +98,20 @@ using namespace mozilla::places;
 #define PREF_FREC_DEFAULT_VISIT_BONUS_DEF       0
 #define PREF_FREC_UNVISITED_BOOKMARK_BONUS      "places.frecency.unvisitedBookmarkBonus"
 #define PREF_FREC_UNVISITED_BOOKMARK_BONUS_DEF  140
 #define PREF_FREC_UNVISITED_TYPED_BONUS         "places.frecency.unvisitedTypedBonus"
 #define PREF_FREC_UNVISITED_TYPED_BONUS_DEF     200
 #define PREF_FREC_RELOAD_VISIT_BONUS            "places.frecency.reloadVisitBonus"
 #define PREF_FREC_RELOAD_VISIT_BONUS_DEF        0
 
+// 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
+
 // 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.
 #define RENEW_CACHED_NOW_TIMEOUT ((int32_t)3 * PR_MSEC_PER_SEC)
 
 // character-set annotation
 #define CHARSET_ANNO NS_LITERAL_CSTRING("URIProperties/characterSet")
 
 // These macros are used when splitting history by date.
@@ -3087,28 +3091,34 @@ public:
 } // namespace
 
 nsresult
 nsNavHistory::DecayFrecency()
 {
   nsresult rv = FixInvalidFrecencies();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  float decayRate = Preferences::GetFloat(PREF_FREC_DECAY_RATE, 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
   // not get an updated frecency.  A scaling factor of .975 results in .5 the
   // original value after 28 days.
   // When changing the scaling factor, ensure that the barrier in
   // moz_places_afterupdate_frecency_trigger still ignores these changes.
   nsCOMPtr<mozIStorageAsyncStatement> decayFrecency = mDB->GetAsyncStatement(
-    "UPDATE moz_places SET frecency = ROUND(frecency * .975) "
+    "UPDATE moz_places SET frecency = ROUND(frecency * :decay_rate) "
     "WHERE frecency > 0"
   );
   NS_ENSURE_STATE(decayFrecency);
 
+  rv = decayFrecency->BindDoubleByName(NS_LITERAL_CSTRING("decay_rate"),
+                                       static_cast<double>(decayRate));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // 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"
   );
   NS_ENSURE_STATE(decayAdaptive);
 
   // Delete any adaptive entries that won't help in ordering anymore.
--- a/toolkit/components/places/tests/PlacesTestUtils.jsm
+++ b/toolkit/components/places/tests/PlacesTestUtils.jsm
@@ -156,16 +156,35 @@ this.PlacesTestUtils = Object.freeze({
       `SELECT count(*) FROM moz_historyvisits v
        JOIN moz_places h ON h.id = v.place_id
        WHERE url_hash = hash(:url) AND url = :url`,
       { url });
     return rows[0].getResultByIndex(0);
   }),
 
   /**
+   * Asynchronously checks the frecency for a specified page.
+   * @param aURI
+   *        nsIURI or address to look for.
+   *
+   * @return {Promise}
+   * @resolves Returns the frecency.
+   * @rejects JavaScript exception.
+   */
+  frecencyInDB: Task.async(function* (aURI) {
+    let url = aURI instanceof Ci.nsIURI ? new URL(aURI.spec) : new URL(aURI);
+    let db = yield PlacesUtils.promiseDBConnection();
+    let rows = yield db.executeCached(
+      `SELECT frecency FROM moz_places
+       WHERE url_hash = hash(:url) AND url = :url`,
+      { url: url.href });
+    return rows[0].getResultByIndex(0);
+  }),
+
+  /**
    * Marks all syncable bookmarks as synced by setting their sync statuses to
    * "NORMAL", resetting their change counters, and removing all tombstones.
    * Used by tests to avoid calling `PlacesSyncUtils.bookmarks.pullChanges`
    * and `PlacesSyncUtils.bookmarks.pushChanges`.
    *
    * @resolves When all bookmarks have been updated.
    * @rejects JavaScript exception.
    */
--- a/toolkit/components/places/tests/browser/browser_multi_redirect_frecency.js
+++ b/toolkit/components/places/tests/browser/browser_multi_redirect_frecency.js
@@ -7,17 +7,22 @@ const REDIRECT_SOURCE_VISIT_BONUS =
   Services.prefs.getIntPref("places.frecency.redirectSourceVisitBonus");
 // const LINK_VISIT_BONUS =
 //   Services.prefs.getIntPref("places.frecency.linkVisitBonus");
 // const TYPED_VISIT_BONUS =
 //   Services.prefs.getIntPref("places.frecency.typedVisitBonus");
 const PERM_REDIRECT_VISIT_BONUS =
   Services.prefs.getIntPref("places.frecency.permRedirectVisitBonus");
 
+// Ensure that decay frecency doesn't kick in during tests (as a result
+// of idle-daily).
+Services.prefs.setCharPref("places.frecency.decayRate", "1.0");
+
 registerCleanupFunction(function*() {
+  Services.prefs.clearUserPref("places.frecency.decayRate");
   yield PlacesTestUtils.clearHistory();
 });
 
 function promiseVisitedURIObserver(redirectURI, targetURI, expectedTargetFrecency) {
   // Create and add history observer.
   return new Promise(resolve => {
     let historyObserver = {
       _redirectNotified: false,
--- a/toolkit/components/places/tests/browser/browser_redirect.js
+++ b/toolkit/components/places/tests/browser/browser_redirect.js
@@ -7,17 +7,22 @@ const TARGET_URI = NetUtil.newURI("http:
 
 const REDIRECT_SOURCE_VISIT_BONUS =
   Services.prefs.getIntPref("places.frecency.redirectSourceVisitBonus");
 const LINK_VISIT_BONUS =
   Services.prefs.getIntPref("places.frecency.linkVisitBonus");
 const TYPED_VISIT_BONUS =
   Services.prefs.getIntPref("places.frecency.typedVisitBonus");
 
+// Ensure that decay frecency doesn't kick in during tests (as a result
+// of idle-daily).
+Services.prefs.setCharPref("places.frecency.decayRate", "1.0");
+
 registerCleanupFunction(function*() {
+  Services.prefs.clearUserPref("places.frecency.decayRate");
   yield PlacesTestUtils.clearHistory();
 });
 
 function promiseVisitedWithFrecency(expectedRedirectFrecency, expectedTargetFrecency) {
   // Create and add history observer.
   return new Promise(resolve => {
     let historyObserver = {
       _redirectNotified: false,
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unit/test_frecency_decay.js
@@ -0,0 +1,71 @@
+Cu.import("resource://gre/modules/PromiseUtils.jsm");
+
+const PREF_FREC_DECAY_RATE_DEF = 0.975;
+
+/**
+ * Promises that the frecency has been changed, and is the new value.
+ *
+ * @param {nsIURI} expectedURI The URI to check frecency for.
+ * @param {Number} expectedFrecency The expected frecency for the URI.
+ * @returns {Promise} A promise which is resolved when the URI is seen.
+ */
+function promiseFrecencyChanged(expectedURI, expectedFrecency) {
+  let deferred = PromiseUtils.defer();
+  let obs = new NavHistoryObserver();
+  obs.onFrecencyChanged =
+    (uri, newFrecency, guid, hidden, visitDate) => {
+      PlacesUtils.history.removeObserver(obs);
+      Assert.ok(!!uri, "uri should not be null");
+      Assert.ok(uri.equals(NetUtil.newURI(expectedURI)), "uri should be the expected one");
+      Assert.equal(newFrecency, expectedFrecency, "Frecency should be the expected one");
+      deferred.resolve();
+    };
+  PlacesUtils.history.addObserver(obs, false);
+  return deferred.promise;
+}
+
+/**
+ * Promises that the many frecencies changed notification has been seen.
+ *
+ * @returns {Promise} A promise which is resolved when the notification is seen.
+ */
+function promiseManyFrecenciesChanged() {
+  let deferred = PromiseUtils.defer();
+  let obs = new NavHistoryObserver();
+  obs.onManyFrecenciesChanged = () => {
+    PlacesUtils.history.removeObserver(obs);
+    Assert.ok(true);
+    deferred.resolve();
+  };
+  PlacesUtils.history.addObserver(obs, false);
+  return deferred.promise;
+}
+
+add_task(function* setup() {
+  Services.prefs.setCharPref("places.frecency.decayRate", PREF_FREC_DECAY_RATE_DEF);
+});
+
+add_task(function* test_frecency_decay() {
+  let unvisitedBookmarkFrecency = Services.prefs.getIntPref("places.frecency.unvisitedBookmarkBonus");
+
+  // Add a bookmark and check its frecency.
+  let url = "http://example.com/b";
+  let promiseOne = promiseFrecencyChanged(url, unvisitedBookmarkFrecency);
+  yield PlacesUtils.bookmarks.insert({
+    url,
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid
+  });
+  yield promiseOne;
+
+  // Trigger DecayFrecency.
+  let promiseMany = promiseManyFrecenciesChanged();
+  PlacesUtils.history.QueryInterface(Ci.nsIObserver)
+             .observe(null, "idle-daily", "");
+  yield promiseMany;
+
+  // Now check the new frecency is correct.
+  let newFrecency = yield PlacesTestUtils.frecencyInDB(url);
+
+  Assert.equal(newFrecency, Math.round(unvisitedBookmarkFrecency * PREF_FREC_DECAY_RATE_DEF),
+               "Frecencies should match");
+});
--- a/toolkit/components/places/tests/unit/xpcshell.ini
+++ b/toolkit/components/places/tests/unit/xpcshell.ini
@@ -89,16 +89,17 @@ skip-if = (os == "win" && os_version == 
 [test_bug636917_isLivemark.js]
 [test_childlessTags.js]
 [test_crash_476292.js]
 [test_database_replaceOnStartup.js]
 [test_download_history.js]
 # Bug 676989: test fails consistently on Android
 fail-if = os == "android"
 [test_frecency.js]
+[test_frecency_decay.js]
 [test_frecency_zero_updated.js]
 # Bug 676989: test hangs consistently on Android
 skip-if = os == "android"
 [test_getChildIndex.js]
 [test_getPlacesInfo.js]
 [test_history.js]
 [test_history_autocomplete_tags.js]
 [test_history_catobs.js]