Bug 1173359 - Exclude hidden pages and framed links from syncing. r=markh
MozReview-Commit-ID: 8I4Sulyr0H7
--- a/services/sync/modules/constants.js
+++ b/services/sync/modules/constants.js
@@ -187,12 +187,14 @@ SEAMONKEY_ID: "
TEST_HARNESS_ID: "xuth@mozilla.org",
MIN_PP_LENGTH: 12,
MIN_PASS_LENGTH: 8,
DEVICE_TYPE_DESKTOP: "desktop",
DEVICE_TYPE_MOBILE: "mobile",
+SQLITE_MAX_VARIABLE_NUMBER: 999,
+
})) {
this[key] = val;
this.EXPORTED_SYMBOLS.push(key);
}
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -156,29 +156,35 @@ Tracker.prototype = {
// Add/update the entry if we have a newer time.
if ((this.changedIDs[id] || -Infinity) < when) {
this._saveChangedID(id, when);
}
return true;
},
- removeChangedID: function (id) {
- if (!id) {
- this._log.warn("Attempted to remove undefined ID to tracker");
+ removeChangedID: function (...ids) {
+ if (!ids.length || this.ignoreAll) {
return false;
}
- if (this.ignoreAll || this._ignored.includes(id)) {
- return false;
+ for (let id of ids) {
+ if (!id) {
+ this._log.warn("Attempted to remove undefined ID from tracker");
+ continue;
+ }
+ if (this._ignored.includes(id)) {
+ this._log.debug(`Not removing ignored ID ${id} from tracker`);
+ continue;
+ }
+ if (this.changedIDs[id] != null) {
+ this._log.trace("Removing changed ID " + id);
+ delete this.changedIDs[id];
+ }
}
- if (this.changedIDs[id] != null) {
- this._log.trace("Removing changed ID " + id);
- delete this.changedIDs[id];
- this.saveChangedIDs();
- }
+ this.saveChangedIDs();
return true;
},
clearChangedIDs: function () {
this._log.trace("Clearing changed ID list");
this.changedIDs = {};
this.saveChangedIDs();
},
--- a/services/sync/modules/engines/history.js
+++ b/services/sync/modules/engines/history.js
@@ -59,25 +59,69 @@ HistoryEngine.prototype = {
}
notifyHistoryObservers("onBeginUpdateBatch");
try {
return SyncEngine.prototype._processIncoming.call(this, newitems);
} finally {
notifyHistoryObservers("onEndUpdateBatch");
}
},
+
+ pullNewChanges() {
+ let modifiedGUIDs = Object.keys(this._tracker.changedIDs);
+ if (!modifiedGUIDs.length) {
+ return new Changeset();
+ }
+
+ 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();
+ }
+ }
+
+ return new Changeset(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
Svc.Obs.add("places-shutdown", function() {
for (let query in this._stmts) {
- let stmt = this._stmts;
+ let stmt = this._stmts[query];
stmt.finalize();
}
this._stmts = {};
}, this);
}
HistoryStore.prototype = {
__proto__: Store.prototype,
@@ -160,22 +204,28 @@ HistoryStore.prototype = {
return this._getStmt(
"SELECT url, title, frecency " +
"FROM moz_places " +
"WHERE guid = :guid");
},
_urlCols: ["url", "title", "frecency"],
get _allUrlStm() {
- return this._getStmt(
- "SELECT url " +
- "FROM moz_places " +
- "WHERE last_visit_date > :cutoff_date " +
- "ORDER BY frecency DESC " +
- "LIMIT :max_results");
+ // 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"],
// 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);
},
--- a/services/sync/tests/unit/test_history_tracker.js
+++ b/services/sync/tests/unit/test_history_tracker.js
@@ -1,36 +1,38 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
+Cu.import("resource://gre/modules/PlacesDBUtils.jsm");
Cu.import("resource://gre/modules/PlacesUtils.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.import("resource://services-sync/engines.js");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/engines/history.js");
Cu.import("resource://services-sync/service.js");
Cu.import("resource://services-sync/util.js");
Service.engineManager.clear();
Service.engineManager.register(HistoryEngine);
var engine = Service.engineManager.get("history");
var tracker = engine._tracker;
// Don't write out by default.
tracker.persistChangedIDs = false;
-async function addVisit(suffix) {
+async function addVisit(suffix, referrer = null, transition = PlacesUtils.history.TRANSITION_LINK) {
let uriString = "http://getfirefox.com/" + suffix;
let uri = Utils.makeURI(uriString);
_("Adding visit for URI " + uriString);
await PlacesTestUtils.addVisits({
uri,
visitDate: Date.now() * 1000,
- transition: PlacesUtils.history.TRANSITION_LINK,
+ transition,
+ referrer,
});
return uri;
}
function run_test() {
initTestLogging("Trace");
Log.repository.getLogger("Sync.Tracker.History").level = Log.Level.Trace;
@@ -211,8 +213,39 @@ 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_hidden() {
+ await startTracking();
+
+ _("Add visit; should be hidden by the redirect");
+ let hiddenURI = await addVisit("hidden");
+ let hiddenGUID = engine._store.GUIDForUri(hiddenURI);
+ _(`Hidden visit GUID: ${hiddenGUID}`);
+
+ _("Add redirect visit; should be tracked");
+ let trackedURI = await addVisit("redirect", hiddenURI,
+ PlacesUtils.history.TRANSITION_REDIRECT_PERMANENT);
+ let trackedGUID = engine._store.GUIDForUri(trackedURI);
+ _(`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);
+ _(`Framed link visit GUID: ${embedGUID}`);
+
+ _("Run Places maintenance to mark redirect visit as hidden");
+ let maintenanceFinishedPromise =
+ promiseOneObserver("places-maintenance-finished");
+ PlacesDBUtils.maintenanceOnIdle();
+ await maintenanceFinishedPromise;
+
+ await verifyTrackedItems([trackedGUID]);
+
+ await cleanup();
+});