Bug 1336518 - Move Sync history queries into PlacesSyncUtils. r=kitcambridge
authortiago <tiago.paez11@gmail.com>
Sun, 30 Jul 2017 17:46:56 -0300
changeset 423048 9c4690da59fabb5edbd20b63649ba119bb72b9e2
parent 423047 efc1451e4b0b694c33f32d6f0bc7911afa0e837c
child 423049 799a8785eab17f7049c6042706b6e0e3a082970a
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskitcambridge
bugs1336518
milestone56.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 1336518 - Move Sync history queries into PlacesSyncUtils. r=kitcambridge MozReview-Commit-ID: Lood8ivLeJf
services/sync/modules/engines/history.js
services/sync/tests/unit/test_history_store.js
services/sync/tests/unit/test_history_tracker.js
toolkit/components/places/PlacesSyncUtils.jsm
toolkit/components/places/tests/unit/test_sync_utils.js
--- a/services/sync/modules/engines/history.js
+++ b/services/sync/modules/engines/history.js
@@ -4,28 +4,32 @@
 
 this.EXPORTED_SYMBOLS = ["HistoryEngine", "HistoryRec"];
 
 var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cu = Components.utils;
 var Cr = Components.results;
 
-const HISTORY_TTL = 5184000; // 60 days
+const HISTORY_TTL = 5184000; // 60 days in milliseconds
+const THIRTY_DAYS_IN_MS = 2592000000; // 30 days in milliseconds
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://services-common/async.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/util.js");
 
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "PlacesSyncUtils",
+                                  "resource://gre/modules/PlacesSyncUtils.jsm");
+
 this.HistoryRec = function HistoryRec(collection, id) {
   CryptoWrapper.call(this, collection, id);
 }
 HistoryRec.prototype = {
   __proto__: CryptoWrapper.prototype,
   _logName: "Sync.Record.History",
   ttl: HISTORY_TTL
 };
@@ -66,51 +70,18 @@ HistoryEngine.prototype = {
   },
 
   async pullNewChanges() {
     let modifiedGUIDs = Object.keys(this._tracker.changedIDs);
     if (!modifiedGUIDs.length) {
       return {};
     }
 
-    let db = PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase)
-                        .DBConnection;
-
-    // Filter out hidden pages and `TRANSITION_FRAMED_LINK` visits. These are
-    // excluded when rendering the history menu, so we use the same constraints
-    // for Sync. We also don't want to sync `TRANSITION_EMBED` visits, but those
-    // aren't stored in the database.
-    for (let startIndex = 0;
-         startIndex < modifiedGUIDs.length;
-         startIndex += SQLITE_MAX_VARIABLE_NUMBER) {
-
-      let chunkLength = Math.min(SQLITE_MAX_VARIABLE_NUMBER,
-                                 modifiedGUIDs.length - startIndex);
-
-      let query = `
-        SELECT DISTINCT p.guid FROM moz_places p
-        JOIN moz_historyvisits v ON p.id = v.place_id
-        WHERE p.guid IN (${new Array(chunkLength).fill("?").join(",")}) AND
-              (p.hidden = 1 OR v.visit_type IN (0,
-                ${PlacesUtils.history.TRANSITION_FRAMED_LINK}))
-      `;
-
-      let statement = db.createAsyncStatement(query);
-      try {
-        for (let i = 0; i < chunkLength; i++) {
-          statement.bindByIndex(i, modifiedGUIDs[startIndex + i]);
-        }
-        let results = Async.querySpinningly(statement, ["guid"]);
-        let guids = results.map(result => result.guid);
-        this._tracker.removeChangedID(...guids);
-      } finally {
-        statement.finalize();
-      }
-    }
-
+    let guidsToRemove = await PlacesSyncUtils.history.determineNonSyncableGuids(modifiedGUIDs);
+    this._tracker.removeChangedID(...guidsToRemove);
     return this._tracker.changedIDs;
   },
 };
 
 function HistoryStore(name, engine) {
   Store.call(this, name, engine);
 
   // Explicitly nullify our references to our cached services so we don't leak
@@ -141,122 +112,69 @@ HistoryStore.prototype = {
     }
 
     this._log.trace("Creating SQL statement: " + query);
     let db = PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase)
                         .DBConnection;
     return this._stmts[query] = db.createAsyncStatement(query);
   },
 
-  get _setGUIDStm() {
-    return this._getStmt(
-      "UPDATE moz_places " +
-      "SET guid = :guid " +
-      "WHERE url_hash = hash(:page_url) AND url = :page_url");
-  },
-
   // Some helper functions to handle GUIDs
-  setGUID: function setGUID(uri, guid) {
-    uri = uri.spec ? uri.spec : uri;
+  async setGUID(uri, guid) {
 
     if (!guid) {
       guid = Utils.makeGUID();
     }
 
-    let stmt = this._setGUIDStm;
-    stmt.params.guid = guid;
-    stmt.params.page_url = uri;
-    Async.querySpinningly(stmt);
+    try {
+      await PlacesSyncUtils.history.changeGuid(uri, guid);
+    } catch (e) {
+      this._log.error("Error setting GUID ${guid} for URI ${uri}", guid, uri);
+    }
+
     return guid;
   },
 
-  get _guidStm() {
-    return this._getStmt(
-      "SELECT guid " +
-      "FROM moz_places " +
-      "WHERE url_hash = hash(:page_url) AND url = :page_url");
-  },
-  _guidCols: ["guid"],
-
-  GUIDForUri: function GUIDForUri(uri, create) {
-    let stm = this._guidStm;
-    stm.params.page_url = uri.spec ? uri.spec : uri;
+  async GUIDForUri(uri, create) {
 
     // Use the existing GUID if it exists
-    let result = Async.querySpinningly(stm, this._guidCols)[0];
-    if (result && result.guid)
-      return result.guid;
-
-    // Give the uri a GUID if it doesn't have one
-    if (create)
-      return this.setGUID(uri);
-  },
-
-  get _visitStm() {
-    return this._getStmt(`/* do not warn (bug 599936) */
-      SELECT visit_type type, visit_date date
-      FROM moz_historyvisits
-      JOIN moz_places h ON h.id = place_id
-      WHERE url_hash = hash(:url) AND url = :url
-      ORDER BY date DESC LIMIT 20`);
-  },
-  _visitCols: ["date", "type"],
-
-  get _urlStm() {
-    return this._getStmt(
-      "SELECT url, title, frecency " +
-      "FROM moz_places " +
-      "WHERE guid = :guid");
-  },
-  _urlCols: ["url", "title", "frecency"],
+    let guid;
+    try {
+      guid = await PlacesSyncUtils.history.fetchGuidForURL(uri);
+    } catch (e) {
+      this._log.error("Error fetching GUID for URL ${uri}", uri);
+    }
 
-  get _allUrlStm() {
-    // Filter out hidden pages and framed link visits. See `pullNewChanges`
-    // for more info.
-    return this._getStmt(`
-      SELECT DISTINCT p.url
-      FROM moz_places p
-      JOIN moz_historyvisits v ON p.id = v.place_id
-      WHERE p.last_visit_date > :cutoff_date AND
-            p.hidden = 0 AND
-            v.visit_type NOT IN (0,
-              ${PlacesUtils.history.TRANSITION_FRAMED_LINK})
-      ORDER BY frecency DESC
-      LIMIT :max_results`);
-  },
-  _allUrlCols: ["url"],
+    // If the URI has an existing GUID, return it.
+    if (guid) {
+      return guid;
+    }
 
-  // See bug 320831 for why we use SQL here
-  _getVisits: function HistStore__getVisits(uri) {
-    this._visitStm.params.url = uri;
-    return Async.querySpinningly(this._visitStm, this._visitCols);
-  },
+    // If the URI doesn't have a GUID and we were indicated to create one.
+    if (create) {
+      return this.setGUID(uri);
+    }
 
-  // See bug 468732 for why we use SQL here
-  _findURLByGUID: function HistStore__findURLByGUID(guid) {
-    this._urlStm.params.guid = guid;
-    return Async.querySpinningly(this._urlStm, this._urlCols)[0];
+    // If the URI doesn't have a GUID and we didn't create one for it.
+    return null;
   },
 
   async changeItemID(oldID, newID) {
-    this.setGUID(this._findURLByGUID(oldID).url, newID);
+    this.setGUID(await PlacesSyncUtils.history.fetchURLInfoForGuid(oldID).url, newID);
   },
 
-
   async getAllIDs() {
-    // Only get places visited within the last 30 days (30*24*60*60*1000ms)
-    this._allUrlStm.params.cutoff_date = (Date.now() - 2592000000) * 1000;
-    this._allUrlStm.params.max_results = MAX_HISTORY_UPLOAD;
+    let urls = await PlacesSyncUtils.history.getAllURLs({ since: new Date((Date.now() - THIRTY_DAYS_IN_MS)), limit: MAX_HISTORY_UPLOAD });
 
-    let urls = Async.querySpinningly(this._allUrlStm, this._allUrlCols);
-    let self = this;
-    return urls.reduce(function(ids, item) {
-      ids[self.GUIDForUri(item.url, true)] = item.url;
-      return ids;
-    }, {});
+    let urlsByGUID = {};
+    for (let url of urls) {
+      let guid = await this.GUIDForUri(url, true);
+      urlsByGUID[guid] = url;
+    }
+    return urlsByGUID;
   },
 
   async applyIncomingBatch(records) {
     let failed = [];
     let blockers = [];
 
     // Convert incoming records to mozIPlaceInfo objects. Some records can be
     // ignored or handled directly, so we're rewriting the array in-place.
@@ -269,17 +187,17 @@ HistoryStore.prototype = {
         if (record.deleted) {
           let promise = this.remove(record);
           promise = promise.catch(ex => failed.push(record.id));
           blockers.push(promise);
 
           // No further processing needed. Remove it from the list.
           shouldApply = false;
         } else {
-          shouldApply = this._recordToPlaceInfo(record);
+          shouldApply = await this._recordToPlaceInfo(record);
         }
       } catch (ex) {
         if (Async.isShutdownException(ex)) {
           throw ex;
         }
         failed.push(record.id);
         shouldApply = false;
       }
@@ -310,17 +228,17 @@ HistoryStore.prototype = {
 
   /**
    * Converts a Sync history record to a mozIPlaceInfo.
    *
    * Throws if an invalid record is encountered (invalid URI, etc.),
    * returns true if the record is to be applied, false otherwise
    * (no visits to add, etc.),
    */
-  _recordToPlaceInfo: function _recordToPlaceInfo(record) {
+  async _recordToPlaceInfo(record) {
     // Sort out invalid URIs and ones Places just simply doesn't want.
     record.uri = Utils.makeURI(record.histUri);
     if (!record.uri) {
       this._log.warn("Attempted to process invalid URI, skipping.");
       throw new Error("Invalid URI in record");
     }
 
     if (!Utils.checkGUID(record.id)) {
@@ -334,17 +252,23 @@ HistoryStore.prototype = {
                       + record.uri.spec + ": can't add this URI.");
       return false;
     }
 
     // We dupe visits by date and type. So an incoming visit that has
     // the same timestamp and type as a local one won't get applied.
     // To avoid creating new objects, we rewrite the query result so we
     // can simply check for containment below.
-    let curVisits = this._getVisits(record.histUri);
+    let curVisits = [];
+    try {
+      curVisits = await PlacesSyncUtils.history.fetchVisitsForURL(record.histUri);
+    } catch (e) {
+      this._log.error("Error while fetching visits for URL ${record.histUri}", record.histUri);
+    }
+
     let i, k;
     for (i = 0; i < curVisits.length; i++) {
       curVisits[i] = curVisits[i].date + "," + curVisits[i].type;
     }
 
     // Walk through the visits, make sure we have sound data, and eliminate
     // dupes. The latter is done by rewriting the array in-place.
     for (i = 0, k = 0; i < record.visits.length; i++) {
@@ -397,27 +321,32 @@ HistoryStore.prototype = {
     if (removed) {
       this._log.trace("Removed page: " + record.id);
     } else {
       this._log.debug("Page already removed: " + record.id);
     }
   },
 
   async itemExists(id) {
-    return !!this._findURLByGUID(id);
+    return !!(await PlacesSyncUtils.history.fetchURLInfoForGuid(id));
   },
 
   async createRecord(id, collection) {
-    let foo = this._findURLByGUID(id);
+    let foo = await PlacesSyncUtils.history.fetchURLInfoForGuid(id);
     let record = new HistoryRec(collection, id);
     if (foo) {
       record.histUri = foo.url;
       record.title = foo.title;
       record.sortindex = foo.frecency;
-      record.visits = this._getVisits(record.histUri);
+      try {
+        record.visits = await PlacesSyncUtils.history.fetchVisitsForURL(record.histUri);
+      } catch (e) {
+        this._log.error("Error while fetching visits for URL ${record.histUri}", record.histUri);
+        record.visits = [];
+      }
     } else {
       record.deleted = true;
     }
 
     return record;
   },
 
   async wipe() {
--- a/services/sync/tests/unit/test_history_store.js
+++ b/services/sync/tests/unit/test_history_store.js
@@ -132,17 +132,17 @@ add_task(async function test_store_creat
     {id: tbguid,
      histUri: tburi.spec,
      title: "The bird is the word!",
      visits: [{date: TIMESTAMP3,
                type: Ci.nsINavHistoryService.TRANSITION_TYPED}]}
   ]);
   await onVisitObserved;
   try {
-    do_check_attribute_count(Async.promiseSpinningly(store.getAllIDs()), 2);
+    do_check_attribute_count(await store.getAllIDs(), 2);
     let queryres = queryHistoryVisits(tburi);
     do_check_eq(queryres.length, 1);
     do_check_eq(queryres[0].time, TIMESTAMP3);
     do_check_eq(queryres[0].title, "The bird is the word!");
   } catch (ex) {
     PlacesTestUtils.clearHistory();
     do_throw(ex);
   }
--- a/services/sync/tests/unit/test_history_tracker.js
+++ b/services/sync/tests/unit/test_history_tracker.js
@@ -139,17 +139,17 @@ add_task(async function test_start_track
 });
 
 add_task(async function test_track_delete() {
   _("Deletions are tracked.");
 
   // This isn't present because we weren't tracking when it was visited.
   await addVisit("track_delete");
   let uri = Utils.makeURI("http://getfirefox.com/track_delete");
-  let guid = engine._store.GUIDForUri(uri);
+  let guid = await engine._store.GUIDForUri(uri.spec);
   await verifyTrackerEmpty();
 
   await startTracking();
   let visitRemovedPromise = promiseVisit("removed", uri);
   let scorePromise = promiseOneObserver("weave:engine:score:updated");
   await PlacesUtils.history.remove(uri);
   await Promise.all([scorePromise, visitRemovedPromise]);
 
@@ -157,17 +157,17 @@ add_task(async function test_track_delet
   do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
 
   await cleanup();
 });
 
 add_task(async function test_dont_track_expiration() {
   _("Expirations are not tracked.");
   let uriToRemove = await addVisit("to_remove");
-  let guidToRemove = engine._store.GUIDForUri(uriToRemove);
+  let guidToRemove = await engine._store.GUIDForUri(uriToRemove.spec);
 
   await resetTracker();
   await verifyTrackerEmpty();
 
   await startTracking();
   let visitRemovedPromise = promiseVisit("removed", uriToRemove);
   let scorePromise = promiseOneObserver("weave:engine:score:updated");
 
@@ -211,29 +211,29 @@ add_task(async function test_stop_tracki
   await cleanup();
 });
 
 add_task(async function test_filter_hidden() {
   await startTracking();
 
   _("Add visit; should be hidden by the redirect");
   let hiddenURI = await addVisit("hidden");
-  let hiddenGUID = engine._store.GUIDForUri(hiddenURI);
+  let hiddenGUID = await engine._store.GUIDForUri(hiddenURI.spec);
   _(`Hidden visit GUID: ${hiddenGUID}`);
 
   _("Add redirect visit; should be tracked");
-  let trackedURI = await addVisit("redirect", hiddenURI,
+  let trackedURI = await addVisit("redirect", hiddenURI.spec,
     PlacesUtils.history.TRANSITION_REDIRECT_PERMANENT);
-  let trackedGUID = engine._store.GUIDForUri(trackedURI);
+  let trackedGUID = await engine._store.GUIDForUri(trackedURI.spec);
   _(`Tracked visit GUID: ${trackedGUID}`);
 
   _("Add visit for framed link; should be ignored");
   let embedURI = await addVisit("framed_link", null,
     PlacesUtils.history.TRANSITION_FRAMED_LINK);
-  let embedGUID = engine._store.GUIDForUri(embedURI);
+  let embedGUID = await engine._store.GUIDForUri(embedURI.spec);
   _(`Framed link visit GUID: ${embedGUID}`);
 
   _("Run Places maintenance to mark redirect visit as hidden");
   await PlacesDBUtils.maintenanceOnIdle();
 
   await verifyTrackedItems([trackedGUID]);
 
   await cleanup();
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -24,16 +24,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
  * `nsINavBookmarksService`, with special handling for smart bookmarks,
  * tags, keywords, synced annotations, and missing parents.
  */
 var PlacesSyncUtils = {};
 
 const { SOURCE_SYNC } = Ci.nsINavBookmarksService;
 
 const MICROSECONDS_PER_SECOND = 1000000;
+const SQLITE_MAX_VARIABLE_NUMBER = 999;
 
 const ORGANIZER_QUERY_ANNO = "PlacesOrganizer/OrganizerQuery";
 const ORGANIZER_ALL_BOOKMARKS_ANNO_VALUE = "AllBookmarks";
 const ORGANIZER_MOBILE_QUERY_ANNO_VALUE = "MobileBookmarks";
 const MOBILE_BOOKMARKS_PREF = "browser.bookmarks.showMobileBookmarks";
 
 // These are defined as lazy getters to defer initializing the bookmarks
 // service until it's needed.
@@ -54,30 +55,198 @@ XPCOMUtils.defineLazyGetter(this, "ROOT_
   [PlacesUtils.bookmarks.unfiledGuid]: "unfiled",
   [PlacesUtils.bookmarks.mobileGuid]: "mobile",
 }));
 
 XPCOMUtils.defineLazyGetter(this, "ROOTS", () =>
   Object.keys(ROOT_SYNC_ID_TO_GUID)
 );
 
+/**
+ * Auxiliary generator function that yields an array in chunks
+ *
+ * @param array
+ * @param chunkLength
+ * @yields {Array} New Array with the next chunkLength elements of array. If the array has less than chunkLength elements, yields all of them
+ */
+function* chunkArray(array, chunkLength) {
+  let startIndex = 0;
+  while (startIndex < array.length) {
+    yield array.slice(startIndex, startIndex += chunkLength);
+  }
+}
+
 const HistorySyncUtils = PlacesSyncUtils.history = Object.freeze({
+  /**
+   * Fetches the frecency for the URL provided
+   *
+   * @param url
+   * @returns {Number} The frecency of the given url
+   */
   async fetchURLFrecency(url) {
     let canonicalURL = PlacesUtils.SYNC_BOOKMARK_VALIDATORS.url(url);
 
     let db = await PlacesUtils.promiseDBConnection();
     let rows = await 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;
   },
+
+  /**
+   * Filters syncable places from a collection of places guids.
+   *
+   * @param guids
+   *
+   * @returns {Array} new Array with the guids that aren't syncable
+   */
+  async determineNonSyncableGuids(guids) {
+    // Filter out hidden pages and `TRANSITION_FRAMED_LINK` visits. These are
+    // excluded when rendering the history menu, so we use the same constraints
+    // for Sync. We also don't want to sync `TRANSITION_EMBED` visits, but those
+    // aren't stored in the database.
+    let db = await PlacesUtils.promiseDBConnection();
+    let nonSyncableGuids = [];
+    for (let chunk of chunkArray(guids, SQLITE_MAX_VARIABLE_NUMBER)) {
+      let rows = await db.execute(`
+        SELECT DISTINCT p.guid FROM moz_places p
+        JOIN moz_historyvisits v ON p.id = v.place_id
+        WHERE p.guid IN (${new Array(chunk.length).fill("?").join(",")}) AND
+            (p.hidden = 1 OR v.visit_type IN (0,
+              ${PlacesUtils.history.TRANSITION_FRAMED_LINK}))
+      `, chunk);
+      nonSyncableGuids = nonSyncableGuids.concat(rows.map(row => row.getResultByName("guid")));
+    }
+    return nonSyncableGuids;
+  },
+
+  /**
+   * Change the guid of the given uri
+   *
+   * @param uri
+   * @param guid
+   */
+  changeGuid(uri, guid) {
+      let canonicalURL = PlacesUtils.SYNC_BOOKMARK_VALIDATORS.url(uri);
+      let validatedGuid = PlacesUtils.BOOKMARK_VALIDATORS.guid(guid);
+      return PlacesUtils.withConnectionWrapper("HistorySyncUtils: changeGuid",
+        async function(db) {
+          await db.executeCached(`
+            UPDATE moz_places
+            SET guid = :guid
+            WHERE url_hash = hash(:page_url) AND url = :page_url`,
+            {guid: validatedGuid, page_url: canonicalURL.href});
+        });
+  },
+
+  /**
+   * Fetch the last 20 visits (date and type of it) corresponding to a given url
+   *
+   * @param url
+   * @returns {Array} Each element of the Array is an object with members: date and type
+   */
+  async fetchVisitsForURL(url) {
+    let canonicalURL = PlacesUtils.SYNC_BOOKMARK_VALIDATORS.url(url);
+    let db = await PlacesUtils.promiseDBConnection();
+    let rows = await db.executeCached(`
+      SELECT visit_type type, visit_date date
+      FROM moz_historyvisits
+      JOIN moz_places h ON h.id = place_id
+      WHERE url_hash = hash(:url) AND url = :url
+      ORDER BY date DESC LIMIT 20`, { url: canonicalURL.href }
+    );
+    return rows.map(row => {
+      let visitDate = row.getResultByName("date");
+      let visitType = row.getResultByName("type");
+      return { date: visitDate, type: visitType };
+    });
+  },
+
+  /**
+   * Fetches the guid of a uri
+   *
+   * @param uri
+   * @returns {String} The guid of the given uri
+   */
+  async fetchGuidForURL(url) {
+      let canonicalURL = PlacesUtils.SYNC_BOOKMARK_VALIDATORS.url(url);
+      let db = await PlacesUtils.promiseDBConnection();
+      let rows = await db.executeCached(`
+        SELECT guid
+        FROM moz_places
+        WHERE url_hash = hash(:page_url) AND url = :page_url`,
+        { page_url: canonicalURL.href }
+      );
+      if (rows.length == 0) {
+        return null;
+      }
+      return rows[0].getResultByName("guid");
+  },
+
+  /**
+   * Fetch information about a guid (url, title and frecency)
+   *
+   * @param guid
+   * @returns {Object} Object with three members: url, title and frecency of the given guid
+   */
+  async fetchURLInfoForGuid(guid) {
+    let db = await PlacesUtils.promiseDBConnection();
+    let rows = await db.executeCached(`
+      SELECT url, title, frecency
+      FROM moz_places
+      WHERE guid = :guid`,
+      { guid }
+    );
+    if (rows.length === 0) {
+      return null;
+    }
+    return {
+      url: rows[0].getResultByName("url"),
+      title: rows[0].getResultByName("title"),
+      frecency: rows[0].getResultByName("frecency"),
+    };
+  },
+
+  /**
+   * Get all URLs filtered by the limit and since members of the options object.
+   *
+   * @param options
+   *        Options object with two members, since and limit. Both of them must be provided
+   * @returns {Array} - Up to limit number of URLs starting from the date provided by since
+   */
+  async getAllURLs(options) {
+    // Check that the limit property is finite number.
+    if (!Number.isFinite(options.limit)) {
+      throw new Error("The number provided in options.limit is not finite.");
+    }
+    // Check that the since property is of type Date.
+    if (!options.since || Object.prototype.toString.call(options.since) != "[object Date]") {
+      throw new Error("The property since of the options object must be of type Date.");
+    }
+    let db = await PlacesUtils.promiseDBConnection();
+    let sinceInMicroseconds = PlacesUtils.toPRTime(options.since);
+    let rows = await db.executeCached(`
+      SELECT DISTINCT p.url
+      FROM moz_places p
+      JOIN moz_historyvisits v ON p.id = v.place_id
+      WHERE p.last_visit_date > :cutoff_date AND
+            p.hidden = 0 AND
+            v.visit_type NOT IN (0,
+              ${PlacesUtils.history.TRANSITION_FRAMED_LINK})
+      ORDER BY frecency DESC
+      LIMIT :max_results`,
+      { cutoff_date: sinceInMicroseconds, max_results: options.limit }
+    );
+    return rows.map(row => row.getResultByName("url"));
+  },
 });
 
 const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
   SMART_BOOKMARKS_ANNO: "Places/SmartBookmark",
   DESCRIPTION_ANNO: "bookmarkProperties/description",
   SIDEBAR_ANNO: "bookmarkProperties/loadInSidebar",
   SYNC_PARENT_ANNO: "sync/parent",
   SYNC_MOBILE_ROOT_ANNO: "mobile/bookmarksRoot",
--- a/toolkit/components/places/tests/unit/test_sync_utils.js
+++ b/toolkit/components/places/tests/unit/test_sync_utils.js
@@ -165,25 +165,198 @@ var ignoreChangedRoots = async function(
 add_task(async 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) {
     await PlacesTestUtils.addVisits(url);
   }
   for (let url of arrayOfURLsToVisit) {
     let frecency = await PlacesSyncUtils.history.fetchURLFrecency(url);
-    equal(typeof frecency, "number");
-    notEqual(frecency, -1);
+    equal(typeof frecency, "number", "The frecency should be of type: number");
+    notEqual(frecency, -1, "The frecency of this url should be different than -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 = await PlacesSyncUtils.history.fetchURLFrecency(url);
-    equal(frecency, -1);
+    equal(frecency, -1, "The frecency of this url should be -1");
+  }
+
+  // Remove the visits added during this test.
+ await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_determineNonSyncableGuids() {
+  // Add visits to the following URLs with different transition types.
+  let arrayOfVisits = [{ uri: "https://www.mozilla.org/en-US/", transition: TRANSITION_TYPED },
+                       { uri: "http://getfirefox.com/", transition: TRANSITION_LINK },
+                       { uri: "http://getthunderbird.com/", transition: TRANSITION_FRAMED_LINK }];
+  for (let visit of arrayOfVisits) {
+    await PlacesTestUtils.addVisits(visit);
+  }
+
+  // Fetch the guid for each visit.
+  let guids = [];
+  let dictURLGuid = {};
+  for (let visit of arrayOfVisits) {
+    let guid = await PlacesSyncUtils.history.fetchGuidForURL(visit.uri);
+    guids.push(guid);
+    dictURLGuid[visit.uri] = guid;
+  }
+
+  // Filter the visits.
+  let filteredGuids = await PlacesSyncUtils.history.determineNonSyncableGuids(guids);
+
+  // Check if the filtered visits are of type TRANSITION_FRAMED_LINK.
+  for (let visit of arrayOfVisits) {
+    if (visit.transition === TRANSITION_FRAMED_LINK) {
+      ok(filteredGuids.includes(dictURLGuid[visit.uri]), "This url should be one of the filtered guids.");
+    } else {
+      ok(!filteredGuids.includes(dictURLGuid[visit.uri]), "This url should not be one of the filtered guids.");
+    }
+  }
+
+  // Remove the visits added during this test.
+  await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_changeGuid() {
+  // Add some visits of the following URLs.
+  let arrayOfURLsToVisit = ["https://www.mozilla.org/en-US/", "http://getfirefox.com/", "http://getthunderbird.com/"];
+  for (let url of arrayOfURLsToVisit) {
+    await PlacesTestUtils.addVisits(url);
+  }
+
+  for (let url of arrayOfURLsToVisit) {
+    let originalGuid = await PlacesSyncUtils.history.fetchGuidForURL(url);
+    let newGuid = makeGuid();
+
+    // Change the original GUID for the new GUID.
+    await PlacesSyncUtils.history.changeGuid(url, newGuid);
+
+    // Fetch the GUID for this URL.
+    let newGuidFetched = await PlacesSyncUtils.history.fetchGuidForURL(url);
+
+    // Check that the URL has the new GUID as its GUID and not the original one.
+    equal(newGuid, newGuidFetched, "These should be equal since we changed the guid for the visit.");
+    notEqual(originalGuid, newGuidFetched, "These should be different since we changed the guid for the visit.");
+  }
+
+  // Remove the visits added during this test.
+  await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_fetchVisitsForURL() {
+  // Get the date for this moment and a date for a minute ago.
+  let now = new Date();
+  let aMinuteAgo = new Date(now.getTime() - (1 * 60000));
+
+  // Add some visits of the following URLs, specifying the transition and the visit date.
+  let arrayOfVisits = [{ uri: "https://www.mozilla.org/en-US/", transition: TRANSITION_TYPED, visitDate: aMinuteAgo },
+                       { uri: "http://getfirefox.com/", transition: TRANSITION_LINK, visitDate: aMinuteAgo },
+                       { uri: "http://getthunderbird.com/", transition: TRANSITION_LINK, visitDate: aMinuteAgo }];
+  for (let elem of arrayOfVisits) {
+    await PlacesTestUtils.addVisits(elem);
   }
+
+  for (let elem of arrayOfVisits) {
+    // Fetch all the visits for this URL.
+    let visits = await PlacesSyncUtils.history.fetchVisitsForURL(elem.uri);
+    // Since the visit we added will be the last one in the collection of visits, we get the index of it.
+    let iLast = visits.length - 1;
+
+    // The date is saved in _micro_seconds, here we change it to milliseconds.
+    let dateInMilliseconds = visits[iLast].date * 0.001;
+
+    // Check that the info we provided for this URL is the same one retrieved.
+    equal(dateInMilliseconds, elem.visitDate.getTime(), "The date we provided should be the same we retrieved.");
+    equal(visits[iLast].type, elem.transition, "The transition type we provided should be the same we retrieved.");
+  }
+
+  // Remove the visits added during this test.
+  await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_fetchGuidForURL() {
+  // Add some visits of the following URLs.
+  let arrayOfURLsToVisit = ["https://www.mozilla.org/en-US/", "http://getfirefox.com/", "http://getthunderbird.com/"];
+  for (let url of arrayOfURLsToVisit) {
+    await PlacesTestUtils.addVisits(url);
+  }
+
+  // This tries to test fetchGuidForURL in two ways:
+  // 1- By fetching the GUID, and then using that GUID to retrieve the info of the visit.
+  //    It then compares the URL with the URL that is on the visits info.
+  // 2- By creating a new GUID, changing the GUID for the visit, fetching the GUID and comparing them.
+  for (let url of arrayOfURLsToVisit) {
+    let guid = await PlacesSyncUtils.history.fetchGuidForURL(url)
+    let info = await PlacesSyncUtils.history.fetchURLInfoForGuid(guid);
+
+    let newGuid = makeGuid();
+    await PlacesSyncUtils.history.changeGuid(url, newGuid);
+    let newGuid2 = await PlacesSyncUtils.history.fetchGuidForURL(url);
+
+    equal(url, info.url, "The url provided and the url retrieved should be the same.");
+    equal(newGuid, newGuid2, "The changed guid and the retrieved guid should be the same.");
+  }
+
+  // Remove the visits added during this test.
+  await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_fetchURLInfoForGuid() {
+  // Add some visits of the following URLs. specifying the title.
+  let visits = [{ uri: "https://www.mozilla.org/en-US/", title: "mozilla" },
+                { uri: "http://getfirefox.com/", title: "firefox" },
+                { uri: "http://getthunderbird.com/", title: "thunderbird" }];
+  for (let visit of visits) {
+    await PlacesTestUtils.addVisits(visit);
+  }
+
+  for (let visit of visits) {
+    let guid = await PlacesSyncUtils.history.fetchGuidForURL(visit.uri);
+    let info = await PlacesSyncUtils.history.fetchURLInfoForGuid(guid);
+
+    // Compare the info returned by fetchURLInfoForGuid,
+    // URL and title should match while frecency must be different than -1.
+    equal(info.url, visit.uri, "The url provided should be the same as the url retrieved.");
+    equal(info.title, visit.title, "The title provided should be the same as the title retrieved.");
+    notEqual(info.frecency, -1, "The frecency of the visit should be different than -1.");
+  }
+
+  // Create a "fake" GUID and check that the result of fetchURLInfoForGuid is null.
+  let guid = makeGuid();
+  let info = await PlacesSyncUtils.history.fetchURLInfoForGuid(guid);
+
+  equal(info, null, "The information object of a non-existent guid should be null.");
+
+  // Remove the visits added during this test.
+  await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_getAllURLs() {
+  // Add some visits of the following URLs.
+  let arrayOfURLsToVisit = ["https://www.mozilla.org/en-US/", "http://getfirefox.com/", "http://getthunderbird.com/"];
+  for (let url of arrayOfURLsToVisit) {
+    await PlacesTestUtils.addVisits(url);
+  }
+
+  // Get all URLs.
+  let allURLs = await PlacesSyncUtils.history.getAllURLs({ since: new Date(Date.now() - 2592000000), limit: 5000 });
+
+  // The amount of URLs must be the same in both collections.
+  equal(allURLs.length, arrayOfURLsToVisit.length, "The amount of urls retrived should match the amount of urls provided.");
+
+  // Check that the correct URLs were retrived.
+  for (let url of arrayOfURLsToVisit) {
+    ok(allURLs.includes(url), "The urls retrieved should match the ones used in this test.");
+  }
+
+  // Remove the visits added during this test.
+  await PlacesTestUtils.clearHistory();
 });
 
 add_task(async function test_order() {
   do_print("Insert some bookmarks");
   let guids = await populateTree(PlacesUtils.bookmarks.menuGuid, {
     kind: "bookmark",
     title: "childBmk",
     url: "http://getfirefox.com",