--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -9,24 +9,26 @@ this.EXPORTED_SYMBOLS = [
var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
Cu.import("resource://services-common/async.js");
Cu.import("resource://services-common/stringbundle.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/resource.js");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://gre/modules/Services.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts",
"resource://gre/modules/FxAccounts.jsm");
const CLIENTS_TTL = 1814400; // 21 days
const CLIENTS_TTL_REFRESH = 604800; // 7 days
+const STALE_CLIENT_REMOTE_AGE = 604800; // 7 days
const SUPPORTED_PROTOCOL_VERSIONS = ["1.1", "1.5"];
function hasDupeCommand(commands, action) {
if (!commands) {
return false;
}
return commands.some(other => other.command == action.command &&
@@ -168,28 +170,46 @@ ClientEngine.prototype = {
this.lastRecordUpload = Date.now() / 1000;
}
SyncEngine.prototype._syncStartup.call(this);
},
_processIncoming() {
// Fetch all records from the server.
this.lastSync = 0;
- this._incomingClients = [];
+ this._incomingClients = {};
try {
SyncEngine.prototype._processIncoming.call(this);
// Since clients are synced unconditionally, any records in the local store
// that don't exist on the server must be for disconnected clients. Remove
// them, so that we don't upload records with commands for clients that will
// never see them. We also do this to filter out stale clients from the
// tabs collection, since showing their list of tabs is confusing.
- let remoteClientIDs = Object.keys(this._store._remoteClients);
- let staleIDs = Utils.arraySub(remoteClientIDs, this._incomingClients);
- for (let staleID of staleIDs) {
- this._removeRemoteClient(staleID);
+ for (let id in this._store._remoteClients) {
+ if (!this._incomingClients[id]) {
+ this._log.info(`Removing local state for deleted client ${id}`);
+ this._removeRemoteClient(id);
+ }
+ }
+ // Bug 1264498: Mobile clients don't remove themselves from the clients
+ // collection when the user disconnects Sync, so we filter out clients
+ // with the same name that haven't synced in over a week.
+ delete this._incomingClients[this.localID];
+ let names = new Set([this.localName]);
+ for (let id in this._incomingClients) {
+ let record = this._store._remoteClients[id];
+ if (!names.has(record.name)) {
+ names.add(record.name);
+ continue;
+ }
+ let remoteAge = AsyncResource.serverTime - this._incomingClients[id];
+ if (remoteAge > STALE_CLIENT_REMOTE_AGE) {
+ this._log.info(`Hiding stale client ${id} with age ${remoteAge}`);
+ this._removeRemoteClient(id);
+ }
}
} finally {
this._incomingClients = null;
}
},
_uploadOutgoing() {
this._clearedCommands = null;
@@ -214,17 +234,17 @@ ClientEngine.prototype = {
Services.telemetry.getHistogramById(hid).add(count);
}
SyncEngine.prototype._syncFinish.call(this);
},
_reconcile: function _reconcile(item) {
// Every incoming record is reconciled, so we use this to track the
// contents of the collection on the server.
- this._incomingClients.push(item.id);
+ this._incomingClients[item.id] = item.modified;
if (!this._store.itemExists(item.id)) {
return true;
}
// Clients are synced unconditionally, so we'll always have new records.
// Unfortunately, this will cause the scheduler to use the immediate sync
// interval for the multi-device case, instead of the active interval. We
// work around this by updating the record during reconciliation, and
--- a/services/sync/tests/unit/test_clients_engine.js
+++ b/services/sync/tests/unit/test_clients_engine.js
@@ -490,25 +490,138 @@ add_test(function test_process_incoming_
_("Ensures local commands are executed");
engine.localCommands = [{ command: "logout", args: [] }];
let ev = "weave:service:logout:finish";
var handler = function() {
Svc.Obs.remove(ev, handler);
+
+ Svc.Prefs.resetBranch("");
+ Service.recordManager.clearCache();
+ engine._resetClient();
+
run_next_test();
};
Svc.Obs.add(ev, handler);
// logout command causes processIncomingCommands to return explicit false.
do_check_false(engine.processIncomingCommands());
});
+add_test(function test_filter_duplicate_names() {
+ _("Ensure that we exclude clients with identical names that haven't synced in a week.");
+
+ let now = Date.now() / 1000;
+ let contents = {
+ meta: {global: {engines: {clients: {version: engine.version,
+ syncID: engine.syncID}}}},
+ clients: {},
+ crypto: {}
+ };
+ let server = serverForUsers({"foo": "password"}, contents);
+ let user = server.user("foo");
+
+ new SyncTestingInfrastructure(server.server);
+ generateNewKeys(Service.collectionKeys);
+
+ // Synced recently.
+ let recentID = Utils.makeGUID();
+ server.insertWBO("foo", "clients", new ServerWBO(recentID, encryptPayload({
+ id: recentID,
+ name: "My Phone",
+ type: "mobile",
+ commands: [],
+ version: "48",
+ protocols: ["1.5"],
+ }), now - 10));
+
+ // Dupe of our client, synced more than 1 week ago.
+ let dupeID = Utils.makeGUID();
+ server.insertWBO("foo", "clients", new ServerWBO(dupeID, encryptPayload({
+ id: dupeID,
+ name: engine.localName,
+ type: "desktop",
+ commands: [],
+ version: "48",
+ protocols: ["1.5"],
+ }), now - 604810));
+
+ // Synced more than 1 week ago, but not a dupe.
+ let oldID = Utils.makeGUID();
+ server.insertWBO("foo", "clients", new ServerWBO(oldID, encryptPayload({
+ id: oldID,
+ name: "My old desktop",
+ type: "desktop",
+ commands: [],
+ version: "48",
+ protocols: ["1.5"],
+ }), now - 604820));
+
+ try {
+ let store = engine._store;
+
+ _("First sync");
+ strictEqual(engine.lastRecordUpload, 0);
+ engine._sync();
+ ok(engine.lastRecordUpload > 0);
+ deepEqual(user.collection("clients").keys().sort(),
+ [recentID, dupeID, oldID, engine.localID].sort(),
+ "Our record should be uploaded on first sync");
+ deepEqual(Object.keys(store.getAllIDs()).sort(),
+ [recentID, oldID, engine.localID].sort(),
+ "Fresh clients should be downloaded on first sync");
+
+ _("Broadcast logout to all clients");
+ engine.sendCommand("logout", []);
+ engine._sync();
+
+ let collection = server.getCollection("foo", "clients");
+ let recentPayload = JSON.parse(JSON.parse(collection.payload(recentID)).ciphertext);
+ deepEqual(recentPayload.commands, [{ command: "logout", args: [] }],
+ "Should send commands to the recent client");
+
+ let oldPayload = JSON.parse(JSON.parse(collection.payload(oldID)).ciphertext);
+ deepEqual(oldPayload.commands, [{ command: "logout", args: [] }],
+ "Should send commands to the week-old client");
+
+ let dupePayload = JSON.parse(JSON.parse(collection.payload(dupeID)).ciphertext);
+ deepEqual(dupePayload.commands, [],
+ "Should not send commands to the dupe client");
+
+ _("Update the dupe client's modified time");
+ server.insertWBO("foo", "clients", new ServerWBO(dupeID, encryptPayload({
+ id: dupeID,
+ name: engine.localName,
+ type: "desktop",
+ commands: [],
+ version: "48",
+ protocols: ["1.5"],
+ }), now - 10));
+
+ _("Second sync.");
+ engine._sync();
+
+ deepEqual(Object.keys(store.getAllIDs()).sort(),
+ [recentID, oldID, dupeID, engine.localID].sort(),
+ "Stale client synced, so it should no longer be marked as a dupe");
+ } finally {
+ Svc.Prefs.resetBranch("");
+ Service.recordManager.clearCache();
+
+ try {
+ server.deleteCollections("foo");
+ } finally {
+ server.stop(run_next_test);
+ }
+ }
+});
+
add_test(function test_command_sync() {
_("Ensure that commands are synced across clients.");
engine._store.wipe();
generateNewKeys(Service.collectionKeys);
let contents = {
meta: {global: {engines: {clients: {version: engine.version,