diff --git a/services/sync/modules/engines/clients.js b/services/sync/modules/engines/clients.js index fe167b16fa48..614c410a8cef 100644 --- a/services/sync/modules/engines/clients.js +++ b/services/sync/modules/engines/clients.js @@ -50,6 +50,7 @@ const CLIENTS_TTL_REFRESH = 604800; // 7 days const STALE_CLIENT_REMOTE_AGE = 604800; // 7 days const SUPPORTED_PROTOCOL_VERSIONS = [SYNC_API_VERSION]; +const LAST_MODIFIED_ON_PROCESS_COMMAND_PREF = "services.sync.clients.lastModifiedOnProcessCommands"; function hasDupeCommand(commands, action) { if (!commands) { @@ -92,6 +93,17 @@ ClientEngine.prototype = { allowSkippedRecord: false, _knownStaleFxADeviceIds: null, + // These two properties allow us to avoid replaying the same commands + // continuously if we cannot manage to upload our own record. + _localClientLastModified: 0, + get _lastModifiedOnProcessCommands() { + return Services.prefs.getIntPref(LAST_MODIFIED_ON_PROCESS_COMMAND_PREF, -1); + }, + + set _lastModifiedOnProcessCommands(value) { + Services.prefs.setIntPref(LAST_MODIFIED_ON_PROCESS_COMMAND_PREF, value); + }, + // Always sync client data as it controls other sync behavior get enabled() { return true; @@ -392,6 +404,7 @@ ClientEngine.prototype = { // with the same name that haven't synced in over a week. // (Note we can't simply delete them, or we re-apply them next sync - see // bug 1287687) + this._localClientLastModified = Math.round(this._incomingClients[this.localID]); delete this._incomingClients[this.localID]; let names = new Set([this.localName]); let seenDeviceIds = new Set([localFxADeviceId]); @@ -670,9 +683,12 @@ ClientEngine.prototype = { */ async processIncomingCommands() { return this._notify("clients:process-commands", "", async function() { - if (!this.localCommands) { + if (!this.localCommands || + (this._lastModifiedOnProcessCommands == this._localClientLastModified + && !this.ignoreLastModifiedOnProcessCommands)) { return true; } + this._lastModifiedOnProcessCommands = this._localClientLastModified; const clearedCommands = await this._readCommands()[this.localID]; const commands = this.localCommands.filter(command => !hasDupeCommand(clearedCommands, command)); diff --git a/services/sync/tests/unit/test_bookmark_repair.js b/services/sync/tests/unit/test_bookmark_repair.js index 0538ebc54558..282afe75d54f 100644 --- a/services/sync/tests/unit/test_bookmark_repair.js +++ b/services/sync/tests/unit/test_bookmark_repair.js @@ -38,6 +38,7 @@ var recordedEvents = []; add_task(async function setup() { clientsEngine = Service.clientsEngine; + clientsEngine.ignoreLastModifiedOnProcessCommands = true; bookmarksEngine = Service.engineManager.get("bookmarks"); await generateNewKeys(Service.collectionKeys); @@ -200,6 +201,7 @@ add_task(async function test_bookmark_repair_integration() { // so now let's take over the role of that other client! _("Create new clients engine pretending to be remote client"); let remoteClientsEngine = Service.clientsEngine = new ClientEngine(Service); + remoteClientsEngine.ignoreLastModifiedOnProcessCommands = true; await remoteClientsEngine.initialize(); remoteClientsEngine.localID = remoteID; @@ -307,6 +309,7 @@ add_task(async function test_bookmark_repair_integration() { } finally { await cleanup(server); clientsEngine = Service.clientsEngine = new ClientEngine(Service); + clientsEngine.ignoreLastModifiedOnProcessCommands = true; clientsEngine.initialize(); } }); diff --git a/services/sync/tests/unit/test_clients_engine.js b/services/sync/tests/unit/test_clients_engine.js index 5b4a8f4d00de..ea0378dab764 100644 --- a/services/sync/tests/unit/test_clients_engine.js +++ b/services/sync/tests/unit/test_clients_engine.js @@ -1352,7 +1352,7 @@ add_task(async function test_keep_cleared_commands_after_reboot() { }], version: "48", protocols: ["1.5"], - }), now - 10)); + }), now - 5)); // Simulate reboot SyncEngine.prototype._uploadOutgoing = oldUploadOutgoing; @@ -1794,6 +1794,24 @@ add_task(async function process_incoming_refreshes_known_stale_clients() { stubRefresh.restore(); }); +add_task(async function process_incoming_refreshes_known_stale_clients() { + Services.prefs.clearUserPref("services.sync.clients.lastModifiedOnProcessCommands"); + engine._localClientLastModified = Math.round(Date.now() / 1000); + + const stubRemoveLocalCommand = sinon.stub(engine, "removeLocalCommand"); + const tabProcessedSpy = sinon.spy(engine, "_handleDisplayURIs"); + engine.localCommands = [{ command: "displayURI", args: ["https://foo.bar", "fxaid1", "foo"] }]; + + await engine.processIncomingCommands(); + ok(tabProcessedSpy.calledOnce); + // Let's say we failed to upload and we end up calling processIncomingCommands again + await engine.processIncomingCommands(); + ok(tabProcessedSpy.calledOnce); + + tabProcessedSpy.restore(); + stubRemoveLocalCommand.restore(); +}); + function run_test() { initTestLogging("Trace"); Log.repository.getLogger("Sync.Engine.Clients").level = Log.Level.Trace;