Bug 1336518 - Move Sync history queries into PlacesSyncUtils. r=kitcambridge draft
authortiago <tiago.paez11@gmail.com>
Sun, 30 Jul 2017 17:46:56 -0300
changeset 618228 dcb8207378afc6ce2995acfe4f235f3ed728a188
parent 618186 6d1b50a370b4adffbb1ee73b9f51707c90d6a2b1
child 639996 70f72ca5b6305818b20c597c64a2c3df2b842925
push id71253
push userbmo:tiago.paez11@gmail.com
push dateSun, 30 Jul 2017 21:04:49 +0000
reviewerskitcambridge
bugs1336518
milestone56.0a1
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",