Bug 1254129 - Read the device name from the clients collection when fetching Synced Tabs. r=markh, a=ritu
authorKit Cambridge <kcambridge@mozilla.com>
Mon, 11 Apr 2016 11:10:40 -0700
changeset 332840 584c4d11538d9b1124c212906ec8e5b7d7254a19
parent 332839 5df87a8821df92414fd626220e10015b68927ae1
child 332841 901b6248af7d34012dc519cef40ec38c034f1c39
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh, ritu
bugs1254129
milestone48.0a2
Bug 1254129 - Read the device name from the clients collection when fetching Synced Tabs. r=markh, a=ritu MozReview-Commit-ID: GS6ynoxpzHP
services/sync/modules/SyncedTabs.jsm
services/sync/modules/engines/clients.js
services/sync/tests/unit/test_syncedtabs.js
--- a/services/sync/modules/SyncedTabs.jsm
+++ b/services/sync/modules/SyncedTabs.jsm
@@ -85,17 +85,17 @@ let SyncedTabsInternal = {
     };
   }),
 
   /* Make a "client" record. Returns a promise for consistency with _makeTab */
   _makeClient: Task.async(function* (client) {
     return {
       id: client.id,
       type: "client",
-      name: client.clientName,
+      name: Weave.Service.clientsEngine.getClientName(client.id),
       icon:  this._getClientIcon(client.id),
       tabs: []
     };
   }),
 
   _tabMatchesFilter(tab, filter) {
     let reFilter = new RegExp(escapeRegExp(filter), "i");
     return tab.url.match(reFilter) || tab.title.match(reFilter);
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -152,16 +152,24 @@ ClientEngine.prototype = {
   set localType(value) {
     Svc.Prefs.set("client.type", value);
   },
 
   remoteClientExists(id) {
     return !!this._store._remoteClients[id];
   },
 
+  getClientName(id) {
+    if (id == this.localID) {
+      return this.localName;
+    }
+    let client = this._store._remoteClients[id];
+    return client ? client.name : "";
+  },
+
   isMobile: function isMobile(id) {
     if (this._store._remoteClients[id])
       return this._store._remoteClients[id].type == DEVICE_TYPE_MOBILE;
     return false;
   },
 
   _syncStartup: function _syncStartup() {
     // Reupload new client record periodically.
--- a/services/sync/tests/unit/test_syncedtabs.js
+++ b/services/sync/tests/unit/test_syncedtabs.js
@@ -28,48 +28,59 @@ MockTabsEngine.prototype = {
 
   getOpenURLs() {
     return new Set();
   },
 }
 
 // A clients engine that doesn't need to be a constructor.
 let MockClientsEngine = {
+  clientSettings: null, // Set in `configureClients`.
+
   isMobile(guid) {
     if (!guid.endsWith("desktop") && !guid.endsWith("mobile")) {
       throw new Error("this module expected guids to end with 'desktop' or 'mobile'");
     }
     return guid.endsWith("mobile");
   },
   remoteClientExists(id) {
-    return !id.startsWith("guid_stale");
+    return this.clientSettings[id] !== false;
+  },
+  getClientName(id) {
+    if (this.clientSettings[id]) {
+      return this.clientSettings[id];
+    }
+    let engine = Weave.Service.engineManager.get("tabs");
+    return engine.clients[id].clientName;
   },
 }
 
 // Configure Sync with our mock tabs engine and force it to become initialized.
 Services.prefs.setCharPref("services.sync.username", "someone@somewhere.com");
 
 Weave.Service.engineManager.unregister("tabs");
 Weave.Service.engineManager.register(MockTabsEngine);
 Weave.Service.clientsEngine = MockClientsEngine;
 
 // Tell the Sync XPCOM service it is initialized.
 let weaveXPCService = Cc["@mozilla.org/weave/service;1"]
                         .getService(Ci.nsISupports)
                         .wrappedJSObject;
 weaveXPCService.ready = true;
 
-function configureClients(clients) {
+function configureClients(clients, clientSettings = {}) {
   // Configure the instance Sync created.
   let engine = Weave.Service.engineManager.get("tabs");
   // each client record is expected to have an id.
   for (let [guid, client] in Iterator(clients)) {
     client.id = guid;
   }
   engine.clients = clients;
+  // Apply clients collection overrides.
+  MockClientsEngine.clientSettings = clientSettings;
   // Send an observer that pretends the engine just finished a sync.
   Services.obs.notifyObservers(null, "weave:engine:sync:finish", "tabs");
 }
 
 // The tests.
 add_task(function* test_noClients() {
   // no clients, can't be tabs.
   yield configureClients({});
@@ -125,23 +136,42 @@ add_task(function* test_staleClientWithT
     guid_stale_desktop: {
       clientName: "My Deleted Laptop",
       tabs: [
       {
         urlHistory: ["https://bar.com/"],
         icon: "https://bar.com/favicon",
       }],
     },
+    guid_stale_name_desktop: {
+      clientName: "My Generic Device",
+      tabs: [
+      {
+        urlHistory: ["https://example.edu/"],
+        icon: "https://example.edu/favicon",
+      }],
+    },
+  }, {
+    guid_stale_mobile: false,
+    guid_stale_desktop: false,
+    // We should always use the device name from the clients collection, instead
+    // of the possibly stale tabs collection.
+    guid_stale_name_desktop: "My Laptop",
   });
   let clients = yield SyncedTabs.getTabClients();
   clients.sort((a, b) => { return a.name.localeCompare(b.name);});
-  equal(clients.length, 2);
+  equal(clients.length, 3);
+  equal(clients[0].name, "My Desktop");
   equal(clients[0].tabs.length, 1);
   equal(clients[0].tabs[0].url, "http://foo.com/");
-  equal(clients[1].tabs.length, 0);
+  equal(clients[1].name, "My Laptop");
+  equal(clients[1].tabs.length, 1);
+  equal(clients[1].tabs[0].url, "https://example.edu/");
+  equal(clients[2].name, "My Phone");
+  equal(clients[2].tabs.length, 0);
 });
 
 add_task(function* test_clientWithTabsIconsDisabled() {
   Services.prefs.setBoolPref("services.sync.syncedTabs.showRemoteIcons", false);
   yield configureClients({
     guid_desktop: {
       clientName: "My Desktop",
       tabs: [