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
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -99,16 +99,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.
@@ -3088,28 +3092,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]