Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2025-02-22 18:32:00 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1587228 - Removes support for old send-tab clients. r=markh

Browse Source 
* Removes the fallback to the old sent-tab
* Deletes the api to send URIs using commands
* Modifies tests that depended on that api to now use
   other commands, namely the wipeEngine command

Differential Revision: https://phabricator.services.mozilla.com/D118086
This commit is contained in:
Tarik Eshaq 2021-06-22 20:21:40 +00:00
parent b1ec341753
commit c8b1f02eae
4 changed files with 52 additions and 383 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
browser/base/content/browser-sync.js
View File

@ -402,20 +402,11 @@ var gSync = {
},
getSendTabTargets() {
// 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) {
const targets = [];
if (
UIState.get().status != UIState.STATUS_SIGNED_IN ||
!fxAccounts.device.recentDeviceList
) {
return targets;
}
for (let d of fxAccounts.device.recentDeviceList) {
@ -423,12 +414,8 @@ var gSync = {
continue;
}
let clientRecord = getClientRecord(d.id);
if (clientRecord || fxAccounts.commands.sendTab.isDeviceCompatible(d)) {
targets.push({
clientRecord,
...d,
});
if (fxAccounts.commands.sendTab.isDeviceCompatible(d)) {
targets.push(d);
}
}
return targets.sort((a, b) => a.name.localeCompare(b.name));
@ -1276,12 +1263,9 @@ 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.`);
}
@ -1326,19 +1310,6 @@ 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.
},

3
browser/components/BrowserGlue.jsm
View File

@ -1061,7 +1061,6 @@ BrowserGlue.prototype = {
}
break;
case "fxaccounts:commands:open-uri":
case "weave:engine:clients:display-uris":
this._onDisplaySyncURIs(subject);
break;
case "session-save":
@ -1217,7 +1216,6 @@ 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");
@ -1271,7 +1269,6 @@ 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(

74
services/sync/modules/engines/clients.js
View File

@ -748,11 +748,6 @@ 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",
},
},
/**
@ -825,7 +820,6 @@ 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.
@ -853,10 +847,6 @@ 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;
@ -871,10 +861,6 @@ ClientEngine.prototype = {
await this._tracker.addChangedID(this.localID);
}
if (URIsToDisplay.length) {
this._handleDisplayURIs(URIsToDisplay);
}
return true;
})();
},
@ -934,66 +920,6 @@ 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);

315
services/sync/tests/unit/test_clients_engine.js
View File

@ -1165,99 +1165,6 @@ 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.");
@ -1302,12 +1209,8 @@ add_task(async function test_merge_commands() {
type: "desktop",
commands: [
{
command: "displayURI",
args: [
"https://example.com",
engine.localID,
"Yak Herders Anonymous",
],
command: "wipeEngine",
args: ["history"],
flowID: Utils.makeGUID(),
},
],
@ -1351,12 +1254,8 @@ add_task(async function test_merge_commands() {
desktopPayload.commands,
[
{
command: "displayURI",
args: [
"https://example.com",
engine.localID,
"Yak Herders Anonymous",
],
command: "wipeEngine",
args: ["history"],
},
{
command: "logout",
@ -1415,12 +1314,8 @@ add_task(async function test_duplicate_remote_commands() {
ok(engine.isFirstSync);
await syncClientsEngine(server);
_("Send tab to client");
await engine.sendCommand("displayURI", [
"https://example.com",
engine.localID,
"Yak Herders Anonymous",
]);
_("Send command to client to wipe history engine");
await engine.sendCommand("wipeEngine", ["history"]);
await syncClientsEngine(server);
_(
@ -1438,12 +1333,8 @@ add_task(async function test_duplicate_remote_commands() {
now - 10
);
_("Send another tab to the desktop client");
await engine.sendCommand(
"displayURI",
["https://foobar.com", engine.localID, "Foo bar!"],
desktopID
);
_("Send another command to the desktop client to wipe tabs engine");
await engine.sendCommand("wipeEngine", ["tabs"], desktopID);
await syncClientsEngine(server);
let desktopPayload = collection.cleartext(desktopID);
@ -1451,8 +1342,8 @@ add_task(async function test_duplicate_remote_commands() {
desktopPayload.commands,
[
{
command: "displayURI",
args: ["https://foobar.com", engine.localID, "Foo bar!"],
command: "wipeEngine",
args: ["tabs"],
},
],
"Should only send the second command to the desktop client"
@ -1488,8 +1379,8 @@ add_task(async function test_upload_after_reboot() {
type: "desktop",
commands: [
{
command: "displayURI",
args: ["https://deviceclink.com", deviceCID, "Device C link"],
command: "wipeEngine",
args: ["history"],
flowID: Utils.makeGUID(),
},
],
@ -1516,12 +1407,8 @@ add_task(async function test_upload_after_reboot() {
ok(engine.isFirstSync);
await syncClientsEngine(server);
_("Send tab to client");
await engine.sendCommand(
"displayURI",
["https://example.com", engine.localID, "Yak Herders Anonymous"],
deviceBID
);
_("Send command to client to wipe tab engine");
await engine.sendCommand("wipeEngine", ["tabs"], deviceBID);
const oldUploadOutgoing = SyncEngine.prototype._uploadOutgoing;
SyncEngine.prototype._uploadOutgoing = async () =>
@ -1533,8 +1420,8 @@ add_task(async function test_upload_after_reboot() {
deviceBPayload.commands,
[
{
command: "displayURI",
args: ["https://deviceclink.com", deviceCID, "Device C link"],
command: "wipeEngine",
args: ["history"],
},
],
"Should be the same because the upload failed"
@ -1565,12 +1452,8 @@ add_task(async function test_upload_after_reboot() {
deviceBPayload.commands,
[
{
command: "displayURI",
args: [
"https://example.com",
engine.localID,
"Yak Herders Anonymous",
],
command: "wipeEngine",
args: ["tabs"],
},
],
"Should only had written our outgoing command"
@ -1608,13 +1491,13 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
type: "desktop",
commands: [
{
command: "displayURI",
args: ["https://deviceblink.com", deviceBID, "Device B link"],
command: "wipeEngine",
args: ["history"],
flowID: Utils.makeGUID(),
},
{
command: "displayURI",
args: ["https://deviceclink.com", deviceCID, "Device C link"],
command: "wipeEngine",
args: ["tabs"],
flowID: Utils.makeGUID(),
},
],
@ -1655,8 +1538,8 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
SyncEngine.prototype._uploadOutgoing = async () =>
engine._onRecordsWritten([], [deviceBID]);
let commandsProcessed = 0;
engine._handleDisplayURIs = uris => {
commandsProcessed = uris.length;
engine.service.wipeClient = _engine => {
commandsProcessed++;
};
await syncClientsEngine(server);
@ -1668,18 +1551,18 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
localRemoteRecord.commands,
[
{
command: "displayURI",
args: ["https://deviceblink.com", deviceBID, "Device B link"],
command: "wipeEngine",
args: ["history"],
},
{
command: "displayURI",
args: ["https://deviceclink.com", deviceCID, "Device C link"],
command: "wipeEngine",
args: ["tabs"],
},
],
"Should be the same because the upload failed"
);
// Another client sends another link
// Another client sends a wipe command
collection.insertRecord(
{
id: engine.localID,
@ -1687,18 +1570,18 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
type: "desktop",
commands: [
{
command: "displayURI",
args: ["https://deviceblink.com", deviceBID, "Device B link"],
command: "wipeEngine",
args: ["history"],
flowID: Utils.makeGUID(),
},
{
command: "displayURI",
args: ["https://deviceclink.com", deviceCID, "Device C link"],
command: "wipeEngine",
args: ["tabs"],
flowID: Utils.makeGUID(),
},
{
command: "displayURI",
args: ["https://deviceclink2.com", deviceCID, "Device C link 2"],
command: "wipeEngine",
args: ["bookmarks"],
flowID: Utils.makeGUID(),
},
],
@ -1714,8 +1597,8 @@ add_task(async function test_keep_cleared_commands_after_reboot() {
await engine.initialize();
commandsProcessed = 0;
engine._handleDisplayURIs = uris => {
commandsProcessed = uris.length;
engine.service.wipeClient = _engine => {
commandsProcessed++;
};
await syncClientsEngine(server);
await engine.processIncomingCommands();
@ -1814,86 +1697,6 @@ 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.");
@ -2099,31 +1902,15 @@ add_task(async function test_duplicate_commands_telemetry() {
await syncClientsEngine(server);
// Make sure deduping works before syncing
await engine.sendURIToClientForDisplay(
"https://example.com",
remoteId,
"Example"
);
await engine.sendURIToClientForDisplay(
"https://example.com",
remoteId,
"Example"
);
await engine.sendCommand("wipeEngine", ["history"], remoteId);
await engine.sendCommand("wipeEngine", ["history"], remoteId);
equal(events.length, 1);
await syncClientsEngine(server);
// And after syncing.
await engine.sendURIToClientForDisplay(
"https://example.com",
remoteId,
"Example"
);
await engine.sendCommand("wipeEngine", ["history"], remoteId);
equal(events.length, 1);
// Ensure we aren't deduping commands to different clients
await engine.sendURIToClientForDisplay(
"https://example.com",
remoteId2,
"Example"
);
await engine.sendCommand("wipeEngine", ["history"], remoteId2);
equal(events.length, 2);
} finally {
Service.recordTelemetryEvent = origRecordTelemetryEvent;
@ -2269,11 +2056,7 @@ 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.sendURIToClientForDisplay(
`https://www.example.com/1/${i}`,
remoteId,
`Page 1.${i}`
);
await engine.sendCommand("wipeEngine", [`history: ${i}`], remoteId);
}
await syncClientsEngine(server);
@ -2288,11 +2071,7 @@ 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.sendURIToClientForDisplay(
`https://www.example.com/2/${i}`,
remoteId,
`Page 2.${i}`
);
await engine.sendCommand("wipeEngine", [`tabs: ${i}`], remoteId);
}
await syncClientsEngine(server);
@ -2313,12 +2092,8 @@ 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, "displayURI");
deepEqual(lastCommand.args, [
"https://www.example.com/2/499",
engine.localID,
"Page 2.499",
]);
equal(lastCommand.command, "wipeEngine");
deepEqual(lastCommand.args, ["tabs: 499"]);
} finally {
maxSizeStub.restore();
await cleanup();