Bug 1336282 - Moved bookmark frecency query into PlacesSyncUtils. r=kitcambridge

MozReview-Commit-ID: 4DDXOUQ2C8N

--HG--
extra : rebase_source : b4d1785e126b87672fc064dee329a791977b1372

services/sync/modules/engines/bookmarks.js

@@ -277,34 +277,6 @@ BookmarksEngine.prototype = {
   syncPriority: 4,
   allowSkippedRecord: false,
 
-  // A diagnostic helper to get the string value for a bookmark's URL given
-  // its ID. Always returns a string - on error will return a string in the
-  // form of "<description of error>" as this is purely for, eg, logging.
-  // (This means hitting the DB directly and we don't bother using a cached
-  // statement - we should rarely hit this.)
-  _getStringUrlForId(id) {
-    let url;
-    try {
-      let stmt = this._store._getStmt(`
-            SELECT h.url
-            FROM moz_places h
-            JOIN moz_bookmarks b ON h.id = b.fk
-            WHERE b.id = :id`);
-      stmt.params.id = id;
-      let rows = Async.querySpinningly(stmt, ["url"]);
-      url = rows.length == 0 ? "<not found>" : rows[0].url;
-    } catch (ex) {
-      if (Async.isShutdownException(ex)) {
-        throw ex;
-      }
-      if (ex instanceof Ci.mozIStorageError) {
-        url = `<failed: Storage error: ${ex.message} (${ex.result})>`;
-      } else {
-        url = `<failed: ${ex.toString()}>`;
-      }
-    }
-    return url;
-  },
 
   _guidMapFailed: false,
   _buildGUIDMap: function _buildGUIDMap() {
@@ -623,14 +595,6 @@ BookmarksEngine.prototype = {
 function BookmarksStore(name, engine) {
   Store.call(this, name, engine);
   this._itemsToDelete = new Set();
-  // Explicitly nullify our references to our cached services so we don't leak
-  Svc.Obs.add("places-shutdown", function() {
-    for (let query in this._stmts) {
-      let stmt = this._stmts[query];
-      stmt.finalize();
-    }
-    this._stmts = {};
-  }, this);
 }
 BookmarksStore.prototype = {
   __proto__: Store.prototype,
@@ -784,26 +748,6 @@ BookmarksStore.prototype = {
     return record;
   },
 
-  _stmts: {},
-  _getStmt(query) {
-    if (query in this._stmts) {
-      return this._stmts[query];
-    }
-
-    this._log.trace("Creating SQL statement: " + query);
-    let db = PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase)
-                        .DBConnection;
-    return this._stmts[query] = db.createAsyncStatement(query);
-  },
-
-  get _frecencyStm() {
-    return this._getStmt(
-        "SELECT frecency " +
-        "FROM moz_places " +
-        "WHERE url_hash = hash(:url) AND url = :url " +
-        "LIMIT 1");
-  },
-  _frecencyCols: ["frecency"],
 
   GUIDForId: function GUIDForId(id) {
     let guid = Async.promiseSpinningly(PlacesUtils.promiseItemGuid(id));
@@ -831,10 +775,9 @@ BookmarksStore.prototype = {
 
     // Add in the bookmark's frecency if we have something.
     if (record.bmkUri != null) {
-      this._frecencyStm.params.url = record.bmkUri;
-      let result = Async.querySpinningly(this._frecencyStm, this._frecencyCols);
-      if (result.length)
-        index += result[0].frecency;
+      let frecency = Async.promiseSpinningly(PlacesSyncUtils.history.fetchURLFrecency(record.bmkUri));
+      if (frecency != -1)
+        index += frecency;
     }
 
     return index;
@@ -860,8 +803,6 @@ function BookmarksTracker(name, engine) {
   this._batchSawScoreIncrement = false;
   this._migratedOldEntries = false;
   Tracker.call(this, name, engine);
-
-  Svc.Obs.add("places-shutdown", this);
 }
 BookmarksTracker.prototype = {
   __proto__: Tracker.prototype,

toolkit/components/places/PlacesSyncUtils.jsm

@@ -57,6 +57,22 @@ XPCOMUtils.defineLazyGetter(this, "ROOTS", () =>
   Object.keys(ROOT_SYNC_ID_TO_GUID)
 );
 
+const HistorySyncUtils = PlacesSyncUtils.history = Object.freeze({
+  fetchURLFrecency: Task.async(function* (url) {
+    let canonicalURL = PlacesUtils.SYNC_BOOKMARK_VALIDATORS.url(url);
+
+    let db = yield PlacesUtils.promiseDBConnection();
+    let rows = yield db.executeCached(`
+      SELECT frecency
+      FROM moz_places
+      WHERE url_hash = hash(:url) AND url = :url
+      LIMIT 1`,
+      { url:canonicalURL.href }
+    );
+    return rows.length ? rows[0].getResultByName("frecency") : -1;
+  }),
+});
+
 const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
   SMART_BOOKMARKS_ANNO: "Places/SmartBookmark",
   DESCRIPTION_ANNO: "bookmarkProperties/description",

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

@@ -171,6 +171,25 @@ var ignoreChangedRoots = Task.async(function* () {
   yield setChangesSynced(changes);
 });
 
+add_task(function* test_fetchURLFrecency() {
+  // Add visits to the following URLs and then check if frecency for those URLs is not -1.
+  let arrayOfURLsToVisit = ["https://www.mozilla.org/en-US/", "http://getfirefox.com", "http://getthunderbird.com"];
+  for (let url of arrayOfURLsToVisit) {
+    yield PlacesTestUtils.addVisits(url);
+  }
+  for (let url of arrayOfURLsToVisit) {
+    let frecency = yield PlacesSyncUtils.history.fetchURLFrecency(url);
+    equal(typeof frecency, "number");
+    notEqual(frecency, -1);
+  }
+  // Do not add visits to the following URLs, and then check if frecency for those URLs is -1.
+  let arrayOfURLsNotVisited = ["https://bugzilla.org", "https://example.org"];
+  for (let url of arrayOfURLsNotVisited) {
+    let frecency = yield PlacesSyncUtils.history.fetchURLFrecency(url);
+    equal(frecency, -1);
+  }
+});
+
 add_task(function* test_order() {
   do_print("Insert some bookmarks");
   let guids = yield populateTree(PlacesUtils.bookmarks.menuGuid, {