Bug 664590 - Do not track file:/// visits. r=markh
authorEdouard Oger <eoger@fastmail.com>
Wed, 06 Dec 2017 16:55:45 -0500
changeset 448626 d57aa14966b9c25f40b8a5cb02a588ecead76c35
parent 448625 dc853f9fd97293ff703530d806ebf2633740ed69
child 448627 b3b472edbfb9f445e425c5977012ef6ba34032cd
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs664590
milestone59.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 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
@@ -453,17 +461,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}`);