Bug 1368383 part 2 - Handle the Sync-not-ready state in page action send to device. r=markh
☠☠ backed out by 807243861494 ☠ ☠
authorEdouard Oger <eoger@fastmail.com>
Thu, 01 Jun 2017 16:52:40 -0400
changeset 410968 4eb7778c4325c8ab17c4576aaf57e952cddbadbe
parent 410967 3ca93081969bdb74ada3974ce36afa7dd327bbb2
child 410969 9526ccbaa1e7cc42b19cad86029c5fa95a48f289
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh
bugs1368383
milestone55.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
Bug 1368383 part 2 - Handle the Sync-not-ready state in page action send to device. r=markh MozReview-Commit-ID: JYPCw3OVGXo
browser/base/content/browser-sync.js
browser/base/content/browser.css
browser/base/content/browser.js
browser/base/content/browser.xul
browser/base/content/test/urlbar/browser_page_action_menu.js
browser/locales/en-US/chrome/browser/browser.dtd
--- a/browser/base/content/browser-sync.js
+++ b/browser/base/content/browser-sync.js
@@ -39,16 +39,20 @@ var gSync = {
       "chrome://weave/locale/sync.properties"
     );
   },
 
   get syncReady() {
     return Cc["@mozilla.org/weave/service;1"].getService().wrappedJSObject.ready;
   },
 
+  get isSignedIn() {
+    return UIState.get().status == UIState.STATUS_SIGNED_IN;
+  },
+
   get remoteClients() {
     return Weave.Service.clientsEngine.remoteClients
            .sort((a, b) => a.name.localeCompare(b.name));
   },
 
   _generateNodeGetters(usePhoton) {
     let prefix = usePhoton ? "appMenu-" : "PanelUI-";
     for (let k of ["Status", "Avatar", "Label", "Container"]) {
@@ -292,50 +296,50 @@ var gSync = {
     for (let i = devicesPopup.childNodes.length - 1; i >= 0; --i) {
       let child = devicesPopup.childNodes[i];
       if (child.classList.contains("sync-menuitem")) {
         child.remove();
       }
     }
 
     const fragment = document.createDocumentFragment();
+    if (this.syncReady) {
+      const onTargetDeviceCommand = (event) => {
+        let clients = event.target.getAttribute("clientId") ?
+          [event.target.getAttribute("clientId")] :
+          this.remoteClients.map(client => client.id);
 
-    const onTargetDeviceCommand = (event) => {
-      let clients = event.target.getAttribute("clientId") ?
-        [event.target.getAttribute("clientId")] :
-        this.remoteClients.map(client => client.id);
-
-      clients.forEach(clientId => this.sendTabToDevice(url, clientId, title));
-      gPageActionButton.panel.hidePopup();
-    }
+        clients.forEach(clientId => this.sendTabToDevice(url, clientId, title));
+        gPageActionButton.panel.hidePopup();
+      }
 
-    function addTargetDevice(clientId, name, clientType) {
-      const targetDevice = createDeviceNodeFn(clientId, name, clientType);
-      targetDevice.addEventListener("command", onTargetDeviceCommand, true);
-      targetDevice.classList.add("sync-menuitem", "sendtab-target");
-      targetDevice.setAttribute("clientId", clientId);
-      targetDevice.setAttribute("clientType", clientType);
-      targetDevice.setAttribute("label", name);
-      fragment.appendChild(targetDevice);
-    }
+      function addTargetDevice(clientId, name, clientType) {
+        const targetDevice = createDeviceNodeFn(clientId, name, clientType);
+        targetDevice.addEventListener("command", onTargetDeviceCommand, true);
+        targetDevice.classList.add("sync-menuitem", "sendtab-target");
+        targetDevice.setAttribute("clientId", clientId);
+        targetDevice.setAttribute("clientType", clientType);
+        targetDevice.setAttribute("label", name);
+        fragment.appendChild(targetDevice);
+      }
 
-    const clients = this.remoteClients;
-    for (let client of clients) {
-      addTargetDevice(client.id, client.name, client.type);
-    }
+      const clients = this.remoteClients;
+      for (let client of clients) {
+        addTargetDevice(client.id, client.name, client.type);
+      }
 
-    // "All devices" menu item
-    if (clients.length > 1) {
-      const separator = createDeviceNodeFn();
-      separator.classList.add("sync-menuitem");
-      fragment.appendChild(separator);
-      const allDevicesLabel = this.fxaStrings.GetStringFromName("sendTabToAllDevices.menuitem");
-      addTargetDevice("", allDevicesLabel, "");
+      // "All devices" menu item
+      if (clients.length > 1) {
+        const separator = createDeviceNodeFn();
+        separator.classList.add("sync-menuitem");
+        fragment.appendChild(separator);
+        const allDevicesLabel = this.fxaStrings.GetStringFromName("sendTabToAllDevices.menuitem");
+        addTargetDevice("", allDevicesLabel, "");
+      }
     }
-
     devicesPopup.appendChild(fragment);
   },
 
   isSendableURI(aURISpec) {
     if (!aURISpec) {
       return false;
     }
     // Disallow sending tabs with more than 65535 characters.
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -1320,15 +1320,15 @@ toolbarpaletteitem[place="palette"][hidd
 
 .dragfeedback-tab {
   -moz-appearance: none;
   opacity: 0.65;
   -moz-window-shadow: none;
 }
 
 /* Page action menu */
-#page-action-sendToDeviceView-body[signedin] > #page-action-sendToDevice-fxa-button,
-#page-action-sendToDeviceView-body:not([signedin]) > #page-action-no-devices-button,
-#page-action-sendToDeviceView-body[hasdevices] > #page-action-no-devices-button {
+#page-action-sendToDeviceView-body:not([state="notsignedin"]) > #page-action-sendToDevice-fxa-button,
+#page-action-sendToDeviceView-body:not([state="nodevice"]) > #page-action-no-devices-button,
+#page-action-sendToDeviceView-body:not([state="notready"]) > #page-action-sync-not-ready-button {
   display: none;
 }
 
 %include theme-vars.inc.css
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -7833,46 +7833,64 @@ var gPageActionButton = {
   },
 
   emailLink() {
     this.panel.hidePopup();
     MailIntegration.sendLinkForBrowser(gBrowser.selectedBrowser);
   },
 
   showSendToDeviceView(subviewButton) {
+    this.setupSendToDeviceView();
+    PanelUI.showSubView("page-action-sendToDeviceView", subviewButton);
+  },
+
+  setupSendToDeviceView() {
     let browser = gBrowser.selectedBrowser;
     let url = browser.currentURI.spec;
     let title = browser.contentTitle;
     let body = this.sendToDeviceBody;
 
+    // This is on top because it also clears the device list between state changes.
     gSync.populateSendTabToDevicesMenu(body, url, title, (clientId, name, clientType) => {
       if (!name) {
         return document.createElement("toolbarseparator");
       }
       let item = document.createElement("toolbarbutton");
       item.classList.add("page-action-sendToDevice-device", "subviewbutton");
       if (clientId) {
         item.classList.add("subviewbutton-iconic");
       }
       return item;
     });
 
-    if (gSync.remoteClients.length) {
-      body.setAttribute("hasdevices", "true");
-    } else {
-      body.removeAttribute("hasdevices");
-    }
-
-    if (UIState.get().status == UIState.STATUS_SIGNED_IN) {
-      body.setAttribute("signedin", "true");
-    } else {
-      body.removeAttribute("signedin");
-    }
-
-    PanelUI.showSubView("page-action-sendToDeviceView", subviewButton);
+    if (!gSync.isSignedIn) {
+      // Could be unconfigured or unverified
+      body.setAttribute("state", "notsignedin");
+      return;
+    }
+
+    // In the first ~10 sec after startup, Sync may not be loaded and the list
+    // of devices will be empty.
+    if (!gSync.syncReady) {
+      body.setAttribute("state", "notready");
+      // Force a background Sync
+      Services.tm.dispatchToMainThread(() => {
+        Weave.Service.sync([]);  // [] = clients engine only
+        if (!window.closed && gSync.syncReady) {
+          this.setupSendToDeviceView();
+        }
+      });
+      return;
+    }
+    if (!gSync.remoteClients.length) {
+      body.setAttribute("state", "nodevice");
+      return;
+    }
+
+    body.setAttribute("state", "signedin");
   },
 
   fxaButtonClicked() {
     this.panel.hidePopup();
     gSync.openPrefs();
   },
 };
 
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -489,16 +489,20 @@
                            class="subviewbutton subviewbutton-iconic"
                            label="&syncBrand.fxAccount.label;"
                            shortcut="&sendToDevice.fxaRequired.label;"
                            oncommand="gPageActionButton.fxaButtonClicked();"/>
             <toolbarbutton id="page-action-no-devices-button"
                            class="subviewbutton"
                            label="&sendToDevice.noDevices.label;"
                            disabled="true"/>
+            <toolbarbutton id="page-action-sync-not-ready-button"
+                           class="subviewbutton"
+                           label="&sendToDevice.syncNotReady.label;"
+                           disabled="true"/>
           </vbox>
         </panelview>
       </photonpanelmultiview>
     </panel>
 
     <!-- Bookmarks and history tooltip -->
     <tooltip id="bhTooltip"/>
 
--- a/browser/base/content/test/urlbar/browser_page_action_menu.js
+++ b/browser/base/content/test/urlbar/browser_page_action_menu.js
@@ -1,12 +1,18 @@
 "use strict";
 
 let gPanel = document.getElementById("page-action-panel");
 
+const mockRemoteClients = [
+  { id: "0", name: "foo", type: "mobile" },
+  { id: "1", name: "bar", type: "desktop" },
+  { id: "2", name: "baz", type: "mobile" },
+];
+
 add_task(async function bookmark() {
   // Open a unique page.
   let url = "http://example.com/browser_page_action_menu";
   await BrowserTestUtils.withNewTab(url, async () => {
     // Open the panel.
     await promisePanelOpen();
 
     // The bookmark button should read "Bookmark This Page" and not be starred.
@@ -123,16 +129,122 @@ add_task(async function sendToDevice_non
       document.getElementById("page-action-send-to-device-button");
     Assert.ok(sendToDeviceButton.disabled);
     let hiddenPromise = promisePanelHidden();
     gPanel.hidePopup();
     await hiddenPromise;
   });
 });
 
+add_task(async function sendToDevice_syncNotReady() {
+  // Open a tab that's sendable.
+  await BrowserTestUtils.withNewTab("http://example.com/", async () => {
+    let syncReadyMock = mockReturn(gSync, "syncReady", false);
+    let signedInMock = mockReturn(gSync, "isSignedIn", true);
+
+    let remoteClientsMock;
+    let origSync = Weave.Service.sync;
+    Weave.Service.sync = () => {
+      mockReturn(gSync, "syncReady", true);
+      remoteClientsMock = mockReturn(gSync, "remoteClients", mockRemoteClients);
+    };
+
+    let origSetupSendToDeviceView = gPageActionButton.setupSendToDeviceView;
+    gPageActionButton.setupSendToDeviceView = () => {
+      this.numCall++ || (this.numCall = 1);
+      origSetupSendToDeviceView.call(gPageActionButton);
+      testSendTabToDeviceMenu(this.numCall);
+    }
+
+    let cleanUp = () => {
+      Weave.Service.sync = origSync;
+      gPageActionButton.setupSendToDeviceView = origSetupSendToDeviceView;
+      signedInMock.restore();
+      syncReadyMock.restore();
+      remoteClientsMock.restore();
+    };
+    registerCleanupFunction(cleanUp);
+
+    // Open the panel.
+    await promisePanelOpen();
+    let sendToDeviceButton =
+      document.getElementById("page-action-send-to-device-button");
+    Assert.ok(!sendToDeviceButton.disabled);
+
+    // Click Send to Device.
+    let viewPromise = promiseViewShown();
+    EventUtils.synthesizeMouseAtCenter(sendToDeviceButton, {});
+    let view = await viewPromise;
+    Assert.equal(view.id, "page-action-sendToDeviceView");
+
+    function testSendTabToDeviceMenu(numCall) {
+      if (numCall == 1) {
+        // The Fxa button should be shown.
+        checkSendToDeviceItems([
+          {
+            id: "page-action-sendToDevice-fxa-button",
+            display: "none",
+          },
+          {
+            id: "page-action-no-devices-button",
+            display: "none",
+            disabled: true,
+          },
+          {
+            id: "page-action-sync-not-ready-button",
+            disabled: true,
+          },
+        ]);
+      } else if (numCall == 2) {
+        // The devices should be shown in the subview.
+        let expectedItems = [
+          {
+            id: "page-action-sendToDevice-fxa-button",
+            display: "none",
+          },
+          {
+            id: "page-action-no-devices-button",
+            display: "none",
+            disabled: true,
+          },
+          {
+            id: "page-action-sync-not-ready-button",
+            display: "none",
+            disabled: true,
+          },
+        ];
+        for (let client of mockRemoteClients) {
+          expectedItems.push({
+            attrs: {
+              clientId: client.id,
+              label: client.name,
+              clientType: client.type,
+            },
+          });
+        }
+        expectedItems.push(
+          null,
+          {
+            label: "All Devices",
+          }
+        );
+        checkSendToDeviceItems(expectedItems);
+      } else {
+        ok(false, "This should never happen");
+      }
+    }
+
+    // Done, hide the panel.
+    let hiddenPromise = promisePanelHidden();
+    gPanel.hidePopup();
+    await hiddenPromise;
+    cleanUp();
+  });
+});
+
 add_task(async function sendToDevice_notSignedIn() {
   // Open a tab that's sendable.
   await BrowserTestUtils.withNewTab("http://example.com/", async () => {
     await promiseSyncReady();
 
     // Open the panel.
     await promisePanelOpen();
     let sendToDeviceButton =
@@ -150,16 +262,21 @@ add_task(async function sendToDevice_not
       {
         id: "page-action-sendToDevice-fxa-button",
       },
       {
         id: "page-action-no-devices-button",
         display: "none",
         disabled: true,
       },
+      {
+        id: "page-action-sync-not-ready-button",
+        display: "none",
+        disabled: true,
+      },
     ]);
 
     // Click the Fxa button.
     let body = view.firstChild;
     let fxaButton = body.childNodes[0];
     Assert.equal(fxaButton.id, "page-action-sendToDevice-fxa-button");
     let prefsTabPromise = BrowserTestUtils.waitForNewTab(gBrowser);
     let hiddenPromise = promisePanelHidden();
@@ -201,16 +318,21 @@ add_task(async function sendToDevice_noD
       {
         id: "page-action-sendToDevice-fxa-button",
         display: "none",
       },
       {
         id: "page-action-no-devices-button",
         disabled: true,
       },
+      {
+        id: "page-action-sync-not-ready-button",
+        display: "none",
+        disabled: true,
+      },
     ]);
 
     // Done, hide the panel.
     let hiddenPromise = promisePanelHidden();
     gPanel.hidePopup();
     await hiddenPromise;
 
     await UIState.reset();
@@ -219,30 +341,19 @@ add_task(async function sendToDevice_noD
 
 add_task(async function sendToDevice_devices() {
   // Open a tab that's sendable.
   await BrowserTestUtils.withNewTab("http://example.com/", async () => {
     await promiseSyncReady();
     UIState._internal._state = { status: UIState.STATUS_SIGNED_IN };
 
     // Set up mock remote clients.
-    let mockRemoteClients = [
-      { id: "0", name: "foo", type: "mobile" },
-      { id: "1", name: "bar", type: "desktop" },
-      { id: "2", name: "baz", type: "mobile" },
-    ];
-    let originalGetter =
-      Object.getOwnPropertyDescriptor(gSync, "remoteClients").get;
-    Object.defineProperty(gSync, "remoteClients", {
-      get() { return mockRemoteClients; }
-    });
+    let remoteClientsMock = mockReturn(gSync, "remoteClients", mockRemoteClients);
     let cleanUp = () => {
-      Object.defineProperty(gSync, "remoteClients", {
-        get: originalGetter
-      });
+      remoteClientsMock.restore();
     };
     registerCleanupFunction(cleanUp);
 
     // Open the panel.
     await promisePanelOpen();
     let sendToDeviceButton =
       document.getElementById("page-action-send-to-device-button");
     Assert.ok(!sendToDeviceButton.disabled);
@@ -259,16 +370,21 @@ add_task(async function sendToDevice_dev
         id: "page-action-sendToDevice-fxa-button",
         display: "none",
       },
       {
         id: "page-action-no-devices-button",
         display: "none",
         disabled: true,
       },
+      {
+        id: "page-action-sync-not-ready-button",
+        display: "none",
+        disabled: true,
+      },
     ];
     for (let client of mockRemoteClients) {
       expectedItems.push({
         attrs: {
           clientId: client.id,
           label: client.name,
           clientType: client.type,
         },
@@ -376,8 +492,32 @@ function checkSendToDeviceItems(expected
     if ("attrs" in expected) {
       for (let name in expected.attrs) {
         Assert.ok(actual.hasAttribute(name));
         Assert.equal(actual.getAttribute(name), expected.attrs[name]);
       }
     }
   }
 }
+
+// Copied from test/sync/head.js (see bug 1369855)
+function mockReturn(obj, symbol, fixture) {
+  let getter = Object.getOwnPropertyDescriptor(obj, symbol).get;
+  if (getter) {
+    Object.defineProperty(obj, symbol, {
+      get() { return fixture; }
+    });
+    return {
+      restore() {
+        Object.defineProperty(obj, symbol, {
+          get: getter
+        });
+      }
+    }
+  }
+  let func = obj[symbol];
+  obj[symbol] = () => fixture;
+  return {
+    restore() {
+      obj[symbol] = func;
+    }
+  }
+}
--- a/browser/locales/en-US/chrome/browser/browser.dtd
+++ b/browser/locales/en-US/chrome/browser/browser.dtd
@@ -958,8 +958,9 @@ you can use these alternative items. Oth
 <!ENTITY updateRestart.panelUI.label2 "Restart to update &brandShorterName;">
 
 <!ENTITY pageActionButton.tooltip "Page actions">
 
 <!ENTITY sendToDevice.label "Send to Deviceā€¦">
 <!ENTITY sendToDevice.viewTitle "Send to Device">
 <!ENTITY sendToDevice.fxaRequired.label "Required">
 <!ENTITY sendToDevice.noDevices.label "No Devices Available">
+<!ENTITY sendToDevice.syncNotReady.label "Syncing Devicesā€¦">