Backed out changeset 0a18936814a7 (bug 1587228) for causing bc failures in... (ed528a58) · Commits · The Tor Project / Applications / Mullvad Browser

browser/base/content/browser-sync.js

+36 −7

Original line number	Diff line number	Diff line
		@@ -402,11 +402,20 @@ var gSync = {
		},

		getSendTabTargets() {
		const targets = [];
		if (
		UIState.get().status != UIState.STATUS_SIGNED_IN \|\|
		!fxAccounts.device.recentDeviceList
		) {
		// If sync is not enabled, then there's no point looking for sync clients.
		// If sync is simply not ready or hasn't yet synced the clients engine, we
		// just assume the fxa device doesn't have a sync record - in practice,
		// that just means we don't attempt to fall back to the "old" sendtab should
		// "new" sendtab fail.
		// We should just kill "old" sendtab now all our mobile browsers support
		// "new".
		let getClientRecord = () => undefined;
		if (UIState.get().syncEnabled && Weave.Service.clientsEngine) {
		getClientRecord = id =>
		Weave.Service.clientsEngine.getClientByFxaDeviceId(id);
		}
		let targets = [];
		if (!fxAccounts.device.recentDeviceList) {
		return targets;
		}
		for (let d of fxAccounts.device.recentDeviceList) {
		@@ -414,8 +423,12 @@ var gSync = {
		continue;
		}

		if (fxAccounts.commands.sendTab.isDeviceCompatible(d)) {
		targets.push(d);
		let clientRecord = getClientRecord(d.id);
		if (clientRecord \|\| fxAccounts.commands.sendTab.isDeviceCompatible(d)) {
		targets.push({
		clientRecord,
		...d,
		});
		}
		}
		return targets.sort((a, b) => a.name.localeCompare(b.name));
		@@ -1263,9 +1276,12 @@ var gSync = {
		// Returns true if we managed to send the tab to any targets, false otherwise.
		async sendTabToDevice(url, targets, title) {
		const fxaCommandsDevices = [];
		const oldSendTabClients = [];
		for (const target of targets) {
		if (fxAccounts.commands.sendTab.isDeviceCompatible(target)) {
		fxaCommandsDevices.push(target);
		} else if (target.clientRecord) {
		oldSendTabClients.push(target.clientRecord);
		} else {
		this.log.error(`Target ${target.id} unsuitable for send tab.`);
		}
		@@ -1310,6 +1326,19 @@ var gSync = {
		numFailed++;
		}
		}
		for (let client of oldSendTabClients) {
		try {
		this.log.info(`Sending a tab to ${client.id} using Sync.`);
		await Weave.Service.clientsEngine.sendURIToClientForDisplay(
		url,
		client.id,
		title
		);
		} catch (e) {
		numFailed++;
		this.log.error("Could not send tab to device.", e);
		}
		}
		return numFailed < targets.length; // Good enough.
		},

browser/components/BrowserGlue.jsm

+3 −0

Original line number	Diff line number	Diff line
		@@ -1061,6 +1061,7 @@ BrowserGlue.prototype = {
		}
		break;
		case "fxaccounts:commands:open-uri":
		case "weave:engine:clients:display-uris":
		this._onDisplaySyncURIs(subject);
		break;
		case "session-save":
		@@ -1216,6 +1217,7 @@ BrowserGlue.prototype = {
		os.addObserver(this, "fxaccounts:verify_login");
		os.addObserver(this, "fxaccounts:device_disconnected");
		os.addObserver(this, "fxaccounts:commands:open-uri");
		os.addObserver(this, "weave:engine:clients:display-uris");
		os.addObserver(this, "session-save");
		os.addObserver(this, "places-init-complete");
		os.addObserver(this, "distribution-customization-complete");
		@@ -1269,6 +1271,7 @@ BrowserGlue.prototype = {
		os.removeObserver(this, "fxaccounts:verify_login");
		os.removeObserver(this, "fxaccounts:device_disconnected");
		os.removeObserver(this, "fxaccounts:commands:open-uri");
		os.removeObserver(this, "weave:engine:clients:display-uris");
		os.removeObserver(this, "session-save");
		if (this._bookmarksBackupIdleTime) {
		this._userIdleService.removeIdleObserver(

services/sync/modules/engines/clients.js

+74 −0

Original line number	Diff line number	Diff line
		@@ -748,6 +748,11 @@ ClientEngine.prototype = {
		desc: "Delete all client data for engine",
		},
		logout: { args: 0, importance: 0, desc: "Log out client" },
		displayURI: {
		args: 3,
		importance: 1,
		desc: "Instruct a client to display a URI",
		},
		},

		/**
		@@ -820,6 +825,7 @@ ClientEngine.prototype = {
		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.
		@@ -847,6 +853,10 @@ ClientEngine.prototype = {
		case "logout":
		this.service.logout();
		return false;
		case "displayURI":
		let [uri, clientId, title] = args;
		URIsToDisplay.push({ uri, clientId, title });
		break;
		default:
		this._log.warn("Received an unknown command: " + command);
		break;
		@@ -861,6 +871,10 @@ ClientEngine.prototype = {
		await this._tracker.addChangedID(this.localID);
		}

		if (URIsToDisplay.length) {
		this._handleDisplayURIs(URIsToDisplay);
		}

		return true;
		})();
		},
		@@ -920,6 +934,66 @@ ClientEngine.prototype = {
		}
		},

		/**
		* Send a URI to another client for display.
		*
		* A side effect is the score is increased dramatically to incur an
		* immediate sync.
		*
		* If an unknown client ID is specified, sendCommand() will throw an
		* Error object.
		*
		* @param uri
		* URI (as a string) to send and display on the remote client
		* @param clientId
		* ID of client to send the command to. If not defined, will be sent
		* to all remote clients.
		* @param title
		* Title of the page being sent.
		*/
		async sendURIToClientForDisplay(uri, clientId, title) {
		this._log.trace(
		"Sending URI to client: " + uri + " -> " + clientId + " (" + title + ")"
		);
		await this.sendCommand("displayURI", [uri, this.localID, title], clientId);

		this._tracker.score += SCORE_INCREMENT_XLARGE;
		},

		/**
		* Handle a bunch of received 'displayURI' commands.
		*
		* Interested parties should observe the "weave:engine:clients:display-uris"
		* topic. The callback will receive an array as the subject parameter
		* containing objects with the following keys:
		*
		* uri URI (string) that is requested for display.
		* sender.id ID of client that sent the command.
		* sender.name Name of client that sent the command.
		* title Title of page that loaded URI (likely) corresponds to.
		*
		* The 'data' parameter to the callback will not be defined.
		*
		* @param uris
		* An array containing URI objects to display
		* @param uris[].uri
		* String URI that was received
		* @param uris[].clientId
		* ID of client that sent URI
		* @param uris[].title
		* String title of page that URI corresponds to. Older clients may not
		* send this.
		*/
		_handleDisplayURIs(uris) {
		uris.forEach(uri => {
		uri.sender = {
		id: uri.clientId,
		name: this.getClientName(uri.clientId),
		};
		});
		Svc.Obs.notify("weave:engine:clients:display-uris", uris);
		},

		async _removeRemoteClient(id) {
		delete this._store._remoteClients[id];
		await this._tracker.removeChangedID(id);

services/sync/tests/unit/test_clients_engine.js

+270 −45

Original line number	Diff line number	Diff line
		@@ -1165,6 +1165,99 @@ add_task(async function test_refresh_fxa_device_list() {
		}
		});

		add_task(async function test_send_uri_to_client_for_display() {
		_("Ensure sendURIToClientForDisplay() sends command properly.");

		let tracker = engine._tracker;
		let store = engine._store;

		let remoteId = Utils.makeGUID();
		let rec = new ClientsRec("clients", remoteId);
		rec.name = "remote";
		await store.create(rec);
		await store.createRecord(remoteId, "clients");

		await tracker.clearChangedIDs();
		let initialScore = tracker.score;

		let uri = "http://www.mozilla.org/";
		let title = "Title of the Page";
		await engine.sendURIToClientForDisplay(uri, remoteId, title);

		let newRecord = store._remoteClients[remoteId];

		notEqual(newRecord, undefined);
		let clientCommands = (await engine._readCommands())[remoteId];
		equal(clientCommands.length, 1);

		let command = clientCommands[0];
		equal(command.command, "displayURI");
		equal(command.args.length, 3);
		equal(command.args[0], uri);
		equal(command.args[1], engine.localID);
		equal(command.args[2], title);

		ok(tracker.score > initialScore);
		ok(tracker.score - initialScore >= SCORE_INCREMENT_XLARGE);

		_("Ensure unknown client IDs result in exception.");
		let unknownId = Utils.makeGUID();
		let error;

		try {
		await engine.sendURIToClientForDisplay(uri, unknownId);
		} catch (ex) {
		error = ex;
		}

		equal(error.message.indexOf("Unknown remote client ID: "), 0);

		await cleanup();
		});

		add_task(async function test_receive_display_uri() {
		_("Ensure processing of received 'displayURI' commands works.");

		// We don't set up WBOs and perform syncing because other tests verify
		// the command API works as advertised. This saves us a little work.

		let uri = "http://www.mozilla.org/";
		let remoteId = Utils.makeGUID();
		let title = "Page Title!";

		let command = {
		command: "displayURI",
		args: [uri, remoteId, title],
		};

		engine.localCommands = [command];

		// Received 'displayURI' command should result in the topic defined below
		// being called.
		let ev = "weave:engine:clients:display-uris";

		let promiseDisplayURI = new Promise(resolve => {
		let handler = function(subject, data) {
		Svc.Obs.remove(ev, handler);

		resolve({ subject, data });
		};

		Svc.Obs.add(ev, handler);
		});

		ok(await engine.processIncomingCommands());

		let { subject, data } = await promiseDisplayURI;

		equal(subject[0].uri, uri);
		equal(subject[0].clientId, remoteId);
		equal(subject[0].title, title);
		equal(data, null);

		await cleanup();
		});

		add_task(async function test_optional_client_fields() {
		_("Ensure that we produce records with the fields added in Bug 1097222.");

		@@ -1209,8 +1302,12 @@ add_task(async function test_merge_commands() {
		type: "desktop",
		commands: [
		{
		command: "wipeEngine",
		args: ["history"],
		command: "displayURI",
		args: [
		"https://example.com",
		engine.localID,
		"Yak Herders Anonymous",
		],
		flowID: Utils.makeGUID(),
		},
		],
		@@ -1254,8 +1351,12 @@ add_task(async function test_merge_commands() {
		desktopPayload.commands,
		[
		{
		command: "wipeEngine",
		args: ["history"],
		command: "displayURI",
		args: [
		"https://example.com",
		engine.localID,
		"Yak Herders Anonymous",
		],
		},
		{
		command: "logout",
		@@ -1314,8 +1415,12 @@ add_task(async function test_duplicate_remote_commands() {
		ok(engine.isFirstSync);
		await syncClientsEngine(server);

		_("Send command to client to wipe history engine");
		await engine.sendCommand("wipeEngine", ["history"]);
		_("Send tab to client");
		await engine.sendCommand("displayURI", [
		"https://example.com",
		engine.localID,
		"Yak Herders Anonymous",
		]);
		await syncClientsEngine(server);

		_(
		@@ -1333,8 +1438,12 @@ add_task(async function test_duplicate_remote_commands() {
		now - 10
		);

		_("Send another command to the desktop client to wipe tabs engine");
		await engine.sendCommand("wipeEngine", ["tabs"], desktopID);
		_("Send another tab to the desktop client");
		await engine.sendCommand(
		"displayURI",
		["https://foobar.com", engine.localID, "Foo bar!"],
		desktopID
		);
		await syncClientsEngine(server);

		let desktopPayload = collection.cleartext(desktopID);
		@@ -1342,8 +1451,8 @@ add_task(async function test_duplicate_remote_commands() {
		desktopPayload.commands,
		[
		{
		command: "wipeEngine",
		args: ["tabs"],
		command: "displayURI",
		args: ["https://foobar.com", engine.localID, "Foo bar!"],
		},
		],
		"Should only send the second command to the desktop client"
		@@ -1379,8 +1488,8 @@ add_task(async function test_upload_after_reboot() {
		type: "desktop",
		commands: [
		{
		command: "wipeEngine",
		args: ["history"],
		command: "displayURI",
		args: ["https://deviceclink.com", deviceCID, "Device C link"],
		flowID: Utils.makeGUID(),
		},
		],
		@@ -1407,8 +1516,12 @@ add_task(async function test_upload_after_reboot() {
		ok(engine.isFirstSync);
		await syncClientsEngine(server);

		_("Send command to client to wipe tab engine");
		await engine.sendCommand("wipeEngine", ["tabs"], deviceBID);
		_("Send tab to client");
		await engine.sendCommand(
		"displayURI",
		["https://example.com", engine.localID, "Yak Herders Anonymous"],
		deviceBID
		);

		const oldUploadOutgoing = SyncEngine.prototype._uploadOutgoing;
		SyncEngine.prototype._uploadOutgoing = async () =>
		@@ -1420,8 +1533,8 @@ add_task(async function test_upload_after_reboot() {
		deviceBPayload.commands,
		[
		{
		command: "wipeEngine",
		args: ["history"],
		command: "displayURI",
		args: ["https://deviceclink.com", deviceCID, "Device C link"],
		},
		],
		"Should be the same because the upload failed"
		@@ -1452,8 +1565,12 @@ add_task(async function test_upload_after_reboot() {
		deviceBPayload.commands,
		[
		{
		command: "wipeEngine",
		args: ["tabs"],
		command: "displayURI",
		args: [
		"https://example.com",
		engine.localID,
		"Yak Herders Anonymous",
		],
		},
		],
		"Should only had written our outgoing command"
		@@ -1491,13 +1608,13 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
		type: "desktop",
		commands: [
		{
		command: "wipeEngine",
		args: ["history"],
		command: "displayURI",
		args: ["https://deviceblink.com", deviceBID, "Device B link"],
		flowID: Utils.makeGUID(),
		},
		{
		command: "wipeEngine",
		args: ["tabs"],
		command: "displayURI",
		args: ["https://deviceclink.com", deviceCID, "Device C link"],
		flowID: Utils.makeGUID(),
		},
		],
		@@ -1538,8 +1655,8 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
		SyncEngine.prototype._uploadOutgoing = async () =>
		engine._onRecordsWritten([], [deviceBID]);
		let commandsProcessed = 0;
		engine.service.wipeClient = _engine => {
		commandsProcessed++;
		engine._handleDisplayURIs = uris => {
		commandsProcessed = uris.length;
		};

		await syncClientsEngine(server);
		@@ -1551,18 +1668,18 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
		localRemoteRecord.commands,
		[
		{
		command: "wipeEngine",
		args: ["history"],
		command: "displayURI",
		args: ["https://deviceblink.com", deviceBID, "Device B link"],
		},
		{
		command: "wipeEngine",
		args: ["tabs"],
		command: "displayURI",
		args: ["https://deviceclink.com", deviceCID, "Device C link"],
		},
		],
		"Should be the same because the upload failed"
		);

		// Another client sends a wipe command
		// Another client sends another link
		collection.insertRecord(
		{
		id: engine.localID,
		@@ -1570,18 +1687,18 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
		type: "desktop",
		commands: [
		{
		command: "wipeEngine",
		args: ["history"],
		command: "displayURI",
		args: ["https://deviceblink.com", deviceBID, "Device B link"],
		flowID: Utils.makeGUID(),
		},
		{
		command: "wipeEngine",
		args: ["tabs"],
		command: "displayURI",
		args: ["https://deviceclink.com", deviceCID, "Device C link"],
		flowID: Utils.makeGUID(),
		},
		{
		command: "wipeEngine",
		args: ["bookmarks"],
		command: "displayURI",
		args: ["https://deviceclink2.com", deviceCID, "Device C link 2"],
		flowID: Utils.makeGUID(),
		},
		],
		@@ -1597,8 +1714,8 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
		await engine.initialize();

		commandsProcessed = 0;
		engine.service.wipeClient = _engine => {
		commandsProcessed++;
		engine._handleDisplayURIs = uris => {
		commandsProcessed = uris.length;
		};
		await syncClientsEngine(server);
		await engine.processIncomingCommands();
		@@ -1697,6 +1814,86 @@ add_task(async function test_deleted_commands() {
		}
		});

		add_task(async function test_send_uri_ack() {
		_("Ensure a sent URI is deleted when the client syncs");

		let now = new_timestamp();
		let server = await serverForFoo(engine);

		await SyncTestingInfrastructure(server);
		await generateNewKeys(Service.collectionKeys);

		try {
		let fakeSenderID = Utils.makeGUID();

		_("Initial sync for empty clients collection");
		await syncClientsEngine(server);
		let collection = server.getCollection("foo", "clients");

		collection.updateRecord(
		engine.localID,
		payload => {
		_("Send a URL to the device on the server");
		payload.commands = [
		{
		command: "displayURI",
		args: [
		"https://example.com",
		fakeSenderID,
		"Yak Herders Anonymous",
		],
		flowID: Utils.makeGUID(),
		},
		];
		},
		now - 10
		);

		_("Sync again");
		await syncClientsEngine(server);

		compareCommands(
		engine.localCommands,
		[
		{
		command: "displayURI",
		args: ["https://example.com", fakeSenderID, "Yak Herders Anonymous"],
		},
		],
		"Should receive incoming URI"
		);
		ok(
		await engine.processIncomingCommands(),
		"Should process incoming commands"
		);
		const clearedCommands = (await engine._readCommands())[engine.localID];
		compareCommands(
		clearedCommands,
		[
		{
		command: "displayURI",
		args: ["https://example.com", fakeSenderID, "Yak Herders Anonymous"],
		},
		],
		"Should mark the commands as cleared after processing"
		);

		_("Check that the command was removed on the server");
		await syncClientsEngine(server);
		let ourPayload = collection.cleartext(engine.localID);
		ok(ourPayload, "Should upload the synced client record");
		deepEqual(ourPayload.commands, [], "Should not reupload cleared commands");
		} finally {
		await cleanup();

		try {
		server.deleteCollections("foo");
		} finally {
		await promiseStopServer(server);
		}
		}
		});

		add_task(async function test_command_sync() {
		_("Notify other clients when writing their record.");

		@@ -1902,15 +2099,31 @@ add_task(async function test_duplicate_commands_telemetry() {

		await syncClientsEngine(server);
		// Make sure deduping works before syncing
		await engine.sendCommand("wipeEngine", ["history"], remoteId);
		await engine.sendCommand("wipeEngine", ["history"], remoteId);
		await engine.sendURIToClientForDisplay(
		"https://example.com",
		remoteId,
		"Example"
		);
		await engine.sendURIToClientForDisplay(
		"https://example.com",
		remoteId,
		"Example"
		);
		equal(events.length, 1);
		await syncClientsEngine(server);
		// And after syncing.
		await engine.sendCommand("wipeEngine", ["history"], remoteId);
		await engine.sendURIToClientForDisplay(
		"https://example.com",
		remoteId,
		"Example"
		);
		equal(events.length, 1);
		// Ensure we aren't deduping commands to different clients
		await engine.sendCommand("wipeEngine", ["history"], remoteId2);
		await engine.sendURIToClientForDisplay(
		"https://example.com",
		remoteId2,
		"Example"
		);
		equal(events.length, 2);
		} finally {
		Service.recordTelemetryEvent = origRecordTelemetryEvent;
		@@ -2056,7 +2269,11 @@ add_task(async function test_create_record_command_limit() {
		_("Send a fairly sane number of commands.");

		for (let i = 0; i < 5; ++i) {
		await engine.sendCommand("wipeEngine", [`history: ${i}`], remoteId);
		await engine.sendURIToClientForDisplay(
		`https://www.example.com/1/${i}`,
		remoteId,
		`Page 1.${i}`
		);
		}

		await syncClientsEngine(server);
		@@ -2071,7 +2288,11 @@ add_task(async function test_create_record_command_limit() {
		_("Send a not-sane number of commands.");
		// Much higher than the maximum number of commands we could actually fit.
		for (let i = 0; i < 500; ++i) {
		await engine.sendCommand("wipeEngine", [`tabs: ${i}`], remoteId);
		await engine.sendURIToClientForDisplay(
		`https://www.example.com/2/${i}`,
		remoteId,
		`Page 2.${i}`
		);
		}

		await syncClientsEngine(server);
		@@ -2092,8 +2313,12 @@ add_task(async function test_create_record_command_limit() {
		equal(firstCommand.command, "wipeEngine");
		_("And the last command in the list should be the last command we sent.");
		let lastCommand = remoteCommands[remoteCommands.length - 1];
		equal(lastCommand.command, "wipeEngine");
		deepEqual(lastCommand.args, ["tabs: 499"]);
		equal(lastCommand.command, "displayURI");
		deepEqual(lastCommand.args, [
		"https://www.example.com/2/499",
		engine.localID,
		"Page 2.499",
		]);
		} finally {
		maxSizeStub.restore();
		await cleanup();