Bug 1368383 part 2 - Handle the Sync-not-ready state in page action send to device. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Thu, 01 Jun 2017 16:52:40 -0400
changeset 588489 b808b1bc6d5f3dfea898ab388f74df4d46a6e65d
parent 588488 c0f5b285c5b69c84ff5cb2423d018ab686705d3c
child 631586 dc529414faabe838f7ddfe39b2166d3aba823c24
push id62052
push userbmo:eoger@fastmail.com
push dateFri, 02 Jun 2017 20:59:56 +0000
reviewersmarkh
bugs1368383
milestone55.0a1
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
@@ -1329,15 +1329,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
@@ -7838,52 +7838,79 @@ var gPageActionButton = {
   },
 
   emailLink() {
     this.panel.hidePopup();
     MailIntegration.sendLinkForBrowser(gBrowser.selectedBrowser);
   },
 
   showSendToDeviceView(subviewButton) {
+    if (!this._initialized) {
+      Services.obs.addObserver(this, "weave:engine:sync:finish", true);
+      this._initialized = true;
+    }
+    this.setupSendToDeviceView();
+    PanelUI.showSubView("page-action-sendToDeviceView", subviewButton);
+  },
+
+  observe(subject, topic, data) {
+    if (window.closed || topic != "weave:engine:sync:finish" || data != "clients") {
+      return;
+    }
+    this.setupSendToDeviceView();
+  },
+
+  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;
+    }
+
+    if (!gSync.syncReady) {
+      body.setAttribute("state", "notready");
+      // Force a background Sync
+      Services.tm.dispatchToMainThread(() => {
+        Weave.Service.sync([]);  // [] = clients engine only
+      });
+      return;
+    }
+    if (!gSync.remoteClients.length) {
+      body.setAttribute("state", "nodevice");
+      return;
+    }
+
+    body.setAttribute("state", "signedin");
   },
 
   fxaButtonClicked() {
     this.panel.hidePopup();
     gSync.openPrefs();
   },
+
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
+                                         Ci.nsISupportsWeakReference])
 };
 
 /**
  * Fired on the "marionette-remote-control" system notification,
  * indicating if the browser session is under remote control.
  */
 const gRemoteControl = {
   observe(subject, topic, data) {
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -490,16 +490,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
@@ -123,16 +123,133 @@ 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 origSync = Weave.Service.sync;
+    let canContinue = PromiseUtils.defer();
+    Weave.Service.sync = () => {
+      setTimeout(async () => {
+        await canContinue.promise;
+        mockReturn(gSync, "syncReady", true);
+        Services.obs.notifyObservers(null, "weave:engine:sync:finish", "clients")
+      }, 50);
+    };
+    let signedInMock = mockReturn(gSync, "isSignedIn", true);
+    let syncReadyMock = mockReturn(gSync, "syncReady", false);
+    // 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 remoteClientsMock = mockReturn(gSync, "remoteClients", mockRemoteClients);
+
+    let origSetupSendToDeviceView = gPageActionButton.setupSendToDeviceView;
+    let setupSendToDeviceView2ndCall = new Promise(resolve => {
+      let numCalls = 0;
+      gPageActionButton.setupSendToDeviceView = () => {
+        numCalls++;
+        origSetupSendToDeviceView.call(gPageActionButton);
+        if (numCalls == 2) {
+          resolve();
+        }
+      }
+    });
+
+    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");
+
+    // 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,
+      },
+    ]);
+
+    canContinue.resolve();
+    await setupSendToDeviceView2ndCall;
+
+    // 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);
+
+    // 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 +267,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 +323,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();
@@ -259,16 +386,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 +508,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…">