From 39fbeac36eb760269e7ed4da75f44e66df216957 Mon Sep 17 00:00:00 2001 From: Ahsan Date: Thu, 9 Feb 2017 12:28:26 -0800 Subject: [PATCH] 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 | 65 +------------------ toolkit/components/places/PlacesSyncUtils.jsm | 16 +++++ .../places/tests/unit/test_sync_utils.js | 19 ++++++ 3 files changed, 38 insertions(+), 62 deletions(-) diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 4c559482e587..2a58c8f623c4 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/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 "" 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 ? "" : rows[0].url; - } catch (ex) { - if (Async.isShutdownException(ex)) { - throw ex; - } - if (ex instanceof Ci.mozIStorageError) { - url = ``; - } else { - url = ``; - } - } - 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, diff --git a/toolkit/components/places/PlacesSyncUtils.jsm b/toolkit/components/places/PlacesSyncUtils.jsm index 29306c1b5c37..eff2951ab3dd 100644 --- a/toolkit/components/places/PlacesSyncUtils.jsm +++ b/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", diff --git a/toolkit/components/places/tests/unit/test_sync_utils.js b/toolkit/components/places/tests/unit/test_sync_utils.js index f3507c048f6f..8601127d85a3 100644 --- a/toolkit/components/places/tests/unit/test_sync_utils.js +++ b/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, {