author | Edouard Oger <eoger@fastmail.com> |
Tue, 03 Oct 2017 14:45:11 -0400 | |
changeset 385735 | 7b8f1bcaf50db587e235b3f03835adb48bfeb84c |
parent 385734 | 6c7b05c7855148adba9fb99643627cbb85a6ff92 |
child 385736 | e92ca171760d45bc16813e6f235bb7bd2c48ca81 |
push id | 32664 |
push user | archaeopteryx@coole-files.de |
push date | Thu, 12 Oct 2017 09:34:55 +0000 |
treeherder | mozilla-central@a32c32d9631c [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | tcsc |
bugs | 1401700 |
milestone | 58.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
|
--- a/services/sync/modules/engines/clients.js +++ b/services/sync/modules/engines/clients.js @@ -45,16 +45,17 @@ XPCOMUtils.defineLazyModuleGetter(this, XPCOMUtils.defineLazyModuleGetter(this, "getRepairResponder", "resource://services-sync/collection_repair.js"); 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 = [SYNC_API_VERSION]; +const LAST_MODIFIED_ON_PROCESS_COMMAND_PREF = "services.sync.clients.lastModifiedOnProcessCommands"; function hasDupeCommand(commands, action) { if (!commands) { return false; } return commands.some(other => other.command == action.command && Utils.deepEquals(other.args, action.args)); } @@ -87,16 +88,27 @@ this.ClientEngine = function ClientEngin ClientEngine.prototype = { __proto__: SyncEngine.prototype, _storeObj: ClientStore, _recordObj: ClientsRec, _trackerObj: ClientsTracker, 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; }, get lastRecordUpload() { return Svc.Prefs.get(this.name + ".lastRecordUpload", 0); }, @@ -387,16 +399,17 @@ ClientEngine.prototype = { } } let localFxADeviceId = await fxAccounts.getDeviceId(); // Bug 1264498: Mobile clients don't remove themselves from the clients // collection when the user disconnects Sync, so we mark as stale clients // 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]); let idToLastModifiedList = Object.entries(this._incomingClients) .sort((a, b) => b[1] - a[1]); for (let [id, serverLastModified] of idToLastModifiedList) { let record = this._store._remoteClients[id]; // stash the server last-modified time on the record. @@ -665,19 +678,22 @@ ClientEngine.prototype = { /** * Check if the local client has any remote commands and perform them. * * @return false to abort sync */ 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)); let didRemoveCommand = false; let URIsToDisplay = []; // Process each command in order. for (let rawCommand of commands) { let shouldRemoveCommand = true; // most commands are auto-removed.
--- a/services/sync/tests/unit/test_bookmark_repair.js +++ b/services/sync/tests/unit/test_bookmark_repair.js @@ -33,16 +33,17 @@ const BOOKMARK_REPAIR_STATE_PREFS = [ ]; let clientsEngine; let bookmarksEngine; var recordedEvents = []; add_task(async function setup() { clientsEngine = Service.clientsEngine; + clientsEngine.ignoreLastModifiedOnProcessCommands = true; bookmarksEngine = Service.engineManager.get("bookmarks"); await generateNewKeys(Service.collectionKeys); Service.recordTelemetryEvent = (object, method, value, extra = undefined) => { recordedEvents.push({ object, method, value, extra }); }; }); @@ -195,16 +196,17 @@ add_task(async function test_bookmark_re "Should not record repair telemetry after sending repair request"); _("Back up repair state to restore later"); let restoreInitialRepairState = backupPrefs(BOOKMARK_REPAIR_STATE_PREFS); // 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; _("Restore missing bookmark"); // Pretend Sync wrote the bookmark, so that we upload it as part of the // repair instead of the sync. bookmarkInfo.source = PlacesUtils.bookmarks.SOURCE_SYNC; await PlacesUtils.bookmarks.insert(bookmarkInfo); @@ -302,16 +304,17 @@ add_task(async function test_bookmark_re }, }]); await revalidationPromise; ok(!Services.prefs.prefHasUserValue("services.sync.repairs.bookmarks.state"), "Should clear repair pref after successfully completing repair"); } finally { await cleanup(server); clientsEngine = Service.clientsEngine = new ClientEngine(Service); + clientsEngine.ignoreLastModifiedOnProcessCommands = true; clientsEngine.initialize(); } }); add_task(async function test_repair_client_missing() { enableValidationPrefs(); _("Ensure that a record missing from the client only will get re-downloaded from the server");
--- a/services/sync/tests/unit/test_clients_engine.js +++ b/services/sync/tests/unit/test_clients_engine.js @@ -1347,17 +1347,17 @@ add_task(async function test_keep_cleare }, { command: "displayURI", args: ["https://deviceclink2.com", deviceCID, "Device C link 2"], flowID: Utils.makeGUID(), }], version: "48", protocols: ["1.5"], - }), now - 10)); + }), now - 5)); // Simulate reboot SyncEngine.prototype._uploadOutgoing = oldUploadOutgoing; engine = Service.clientsEngine = new ClientEngine(Service); await engine.initialize(); commandsProcessed = 0; engine._handleDisplayURIs = (uris) => { commandsProcessed = uris.length }; @@ -1789,13 +1789,31 @@ add_task(async function process_incoming await engine._processIncoming(); ok(stubRefresh.notCalled, "Should not refresh the known stale clients since it's already populated"); stubProcessIncoming.restore(); 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; run_next_test(); }