Bug 664590 - Do not track file:/// visits. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Wed, 06 Dec 2017 16:55:45 -0500
changeset 713134 c02e42ab225ddab425f879e2c1972826e9d019e6
parent 713074 f1329009bf0da7b40229ea75ffe18f201b71359e
child 744254 95987c67b5948bde9b16e72d5b860f5ae36ba0fe
push id93555
push userbmo:eoger@fastmail.com
push dateTue, 19 Dec 2017 18:21:03 +0000
reviewersmarkh
bugs664590
milestone59.0a1
Bug 664590 - Do not track file:/// visits. r?markh MozReview-Commit-ID: 6oVDkJU7kai
services/sync/modules/engines/history.js
services/sync/tests/unit/test_history_store.js
services/sync/tests/unit/test_history_tracker.js
--- a/services/sync/modules/engines/history.js
+++ b/services/sync/modules/engines/history.js
@@ -64,16 +64,20 @@ HistoryEngine.prototype = {
     notifyHistoryObservers("onBeginUpdateBatch");
     try {
       await SyncEngine.prototype._processIncoming.call(this, newitems);
     } finally {
       notifyHistoryObservers("onEndUpdateBatch");
     }
   },
 
+  shouldSyncURL(url) {
+    return !url.startsWith("file:");
+  },
+
   async pullNewChanges() {
     let modifiedGUIDs = Object.keys(this._tracker.changedIDs);
     if (!modifiedGUIDs.length) {
       return {};
     }
 
     let guidsToRemove = await PlacesSyncUtils.history.determineNonSyncableGuids(modifiedGUIDs);
     this._tracker.removeChangedID(...guidsToRemove);
@@ -169,16 +173,19 @@ HistoryStore.prototype = {
     this.setGUID(info.url, newID);
   },
 
   async getAllIDs() {
     let urls = await PlacesSyncUtils.history.getAllURLs({ since: new Date((Date.now() - THIRTY_DAYS_IN_MS)), limit: MAX_HISTORY_UPLOAD });
 
     let urlsByGUID = {};
     for (let url of urls) {
+      if (!this.engine.shouldSyncURL(url)) {
+        continue;
+      }
       let guid = await this.GUIDForUri(url, true);
       urlsByGUID[guid] = url;
     }
     return urlsByGUID;
   },
 
   async applyIncomingBatch(records) {
     let failed = [];
@@ -289,17 +296,18 @@ HistoryStore.prototype = {
     record.uri = CommonUtils.makeURI(record.histUri);
 
     if (!Utils.checkGUID(record.id)) {
       this._log.warn("Encountered record with invalid GUID: " + record.id);
       return false;
     }
     record.guid = record.id;
 
-    if (!this._canAddURI(record.uri)) {
+    if (!this._canAddURI(record.uri) ||
+        !this.engine.shouldSyncURL(record.uri.spec)) {
       this._log.trace("Ignoring record " + record.id + " with URI "
                       + 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
@@ -450,17 +458,17 @@ HistoryTracker.prototype = {
 
   onVisit(uri, vid, time, session, referrer, trans, guid) {
     if (this.ignoreAll) {
       this._log.trace("ignoreAll: ignoring visit for " + guid);
       return;
     }
 
     this._log.trace("onVisit: " + uri.spec);
-    if (this.addChangedID(guid)) {
+    if (this.engine.shouldSyncURL(uri.spec) && this.addChangedID(guid)) {
       this.score += SCORE_INCREMENT_SMALL;
     }
   },
 
   onClearHistory() {
     this._log.trace("onClearHistory");
     // Note that we're going to trigger a sync, but none of the cleared
     // pages are tracked, so the deletions will not be propagated.
--- a/services/sync/tests/unit/test_history_store.js
+++ b/services/sync/tests/unit/test_history_store.js
@@ -422,12 +422,44 @@ add_task(async function test_chunking() 
                   ],
                 ]
                );
   } finally {
     store.MAX_VISITS_PER_INSERT = mvpi;
   }
 });
 
+add_task(async function test_getAllIDs_filters_file_uris() {
+  let uri = CommonUtils.makeURI("file:///Users/eoger/tps/config.json");
+  let visitAddedPromise = promiseVisit("added", uri);
+  await PlacesTestUtils.addVisits({
+    uri,
+    visitDate: Date.now() * 1000,
+    transition: PlacesUtils.history.TRANSITION_LINK
+  });
+  await visitAddedPromise;
+
+  do_check_attribute_count(await store.getAllIDs(), 0);
+
+  await PlacesTestUtils.clearHistory();
+});
+
+add_task(async function test_applyIncomingBatch_filters_file_uris() {
+  const guid = Utils.makeGUID();
+  let uri = CommonUtils.makeURI("file:///Users/eoger/tps/config.json");
+  await applyEnsureNoFailures([
+    {id: guid,
+     histUri: uri.spec,
+     title: "TPS CONFIG",
+     visits: [{date: TIMESTAMP3,
+               type: Ci.nsINavHistoryService.TRANSITION_TYPED}]}
+  ]);
+  do_check_false((await store.itemExists(guid)));
+  let queryres = await PlacesUtils.history.fetch(uri.spec, {
+    includeVisits: true,
+  });
+  do_check_null(queryres);
+});
+
 add_task(async function cleanup() {
   _("Clean up.");
   await PlacesTestUtils.clearHistory();
 });
--- a/services/sync/tests/unit/test_history_tracker.js
+++ b/services/sync/tests/unit/test_history_tracker.js
@@ -207,16 +207,33 @@ add_task(async function test_stop_tracki
   _("Notifying twice won't do any harm.");
   await stopTracking();
   await addVisit("stop_tracking_twice2");
   await verifyTrackerEmpty();
 
   await cleanup();
 });
 
+add_task(async function test_filter_file_uris() {
+  await startTracking();
+
+  let uri = CommonUtils.makeURI("file:///Users/eoger/tps/config.json");
+  let visitAddedPromise = promiseVisit("added", uri);
+  await PlacesTestUtils.addVisits({
+    uri,
+    visitDate: Date.now() * 1000,
+    transition: PlacesUtils.history.TRANSITION_LINK
+  });
+  await visitAddedPromise;
+
+  await verifyTrackerEmpty();
+  await stopTracking();
+  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 = await engine._store.GUIDForUri(hiddenURI.spec);
   _(`Hidden visit GUID: ${hiddenGUID}`);