Bug 1601662 - split UITour's 'fxa' request into high and low latency versions r=MattN

Differential Revision: https://phabricator.services.mozilla.com/D59443 --HG-- extra : moz-landing-system : lando
2020-01-10 03:37:02 +00:00 · 2020-01-10 03:37:02 +00:00 · f18437fc58
--- a/browser/components/uitour/UITour-lib.js
+++ b/browser/components/uitour/UITour-lib.js
@ -401,6 +401,7 @@ if (typeof Mozilla == "undefined") {
   * - :js:func:`sync <Mozilla.UITour.Configuration.Sync>`
   *   DEPRECATED, use 'fxa'
   * - :js:func:`fxa <Mozilla.UITour.Configuration.FxA>`
+   * - :js:func:`fxaConnections <Mozilla.UITour.Configuration.FxAConnections>`
   *
   */

@ -465,8 +466,8 @@ if (typeof Mozilla == "undefined") {
   */

  /**
-   * FxA status, including whether FxA is connected, device counts, services
-   * connected to this browser and services externally connected to the account.
+   * FxA local status, including whether FxA is connected and the general
+   * account state.
   * @typedef {Object} Mozilla.UITour.Configuration.FxA
   * @property {Boolean} setup - Whether FxA is setup on this device. If false,
   *    no other properties will exist.
@ -474,6 +475,23 @@ if (typeof Mozilla == "undefined") {
   *    If false, it probably means the account is unverified or the user has
   *    changed their password on another device and needs to update it here.
   *    In that case many other properties will not exist.
+   * @property {Mozilla.UITour.Configuration.BrowserServices} [browserServices] -
+   *    Information about account services attached to this browser, and with
+   *    special support implemented by this browser. You should not expect
+   *    every accountService connected in this browser to get a special entry
+   *    here. Indeed, what services, and in what circumstances they may appear
+   *    here in the future is largely TBD.
+   * @since 71
+   */
+
+  /**
+   * FxA connections status - information about the account which typically
+   * isn't stored locally, so needs to be obtained from the FxA servers. As such,
+   * requesting this information is likely to be high-latency and may return
+   * incomplete data if there is a network or server error.
+   * @typedef {Object} Mozilla.UITour.Configuration.FxAConnections
+   * @property {Boolean} setup - Whether FxA is setup on this device. If false,
+   *    no other properties will exist.
   * @property {Number} [numOtherDevices] - Number of devices connected to this
   *    account, not counting this device.
   * @property {Object.<String, Number>} [numDevicesByType] - A count of devices
@ -486,13 +504,7 @@ if (typeof Mozilla == "undefined") {
   *    browser and should not be confused with devices. For example, if the user
   *    has enabled Monitor or Lockwise on one or more devices - including on
   *    this device - that service will have a single entry here.
-   * @property {Mozilla.UITour.Configuration.BrowserServices} [browserServices] -
-   *    Information about account services attached to this browser, and with
-   *    special support implemented by this browser. You should not expect
-   *    every accountService connected in this browser to get a special entry
-   *    here. Indeed, what services, and in what circumstances they may appear
-   *    here in the future is largely TBD.
-   * @since 71
+   * @since 73
   */

  /**
--- a/browser/components/uitour/UITour.jsm
+++ b/browser/components/uitour/UITour.jsm
@ -1671,6 +1671,9 @@ var UITour = {
      case "fxa":
        this.getFxA(aBrowser, aCallbackID);
        break;
+      case "fxaConnections":
+        this.getFxAConnections(aBrowser, aCallbackID);
+        break;

      // NOTE: 'sync' is deprecated and should be removed in Firefox 73 (because
      // by then, all consumers will have upgraded to use 'fxa' in that version
@ -1727,7 +1730,52 @@ var UITour = {
    }
  },

+  // Get data about the local FxA account. This should be a low-latency request
+  // - everything returned here can be obtained locally without hitting any
+  // remote servers. See also `getFxAConnections()`
  getFxA(aBrowser, aCallbackID) {
+    (async () => {
+      let setup = !!(await fxAccounts.getSignedInUser());
+      let result = { setup };
+      if (!setup) {
+        this.sendPageCallback(aBrowser, aCallbackID, result);
+        return;
+      }
+      // We are signed in so need to build a richer result.
+      // Each of the "browser services" - currently only "sync" is supported
+      result.browserServices = {};
+      let hasSync = Services.prefs.prefHasUserValue("services.sync.username");
+      if (hasSync) {
+        result.browserServices.sync = {
+          // We always include 'setup' for b/w compatibility.
+          setup: true,
+          desktopDevices: Services.prefs.getIntPref(
+            "services.sync.clients.devices.desktop",
+            0
+          ),
+          mobileDevices: Services.prefs.getIntPref(
+            "services.sync.clients.devices.mobile",
+            0
+          ),
+          totalDevices: Services.prefs.getIntPref(
+            "services.sync.numClients",
+            0
+          ),
+        };
+      }
+      // And the account state.
+      result.accountStateOK = await fxAccounts.hasLocalSession();
+      this.sendPageCallback(aBrowser, aCallbackID, result);
+    })().catch(err => {
+      log.error(err);
+      this.sendPageCallback(aBrowser, aCallbackID, {});
+    });
+  },
+
+  // Get data about the FxA account "connections" (ie, other devices, other
+  // apps, etc. Note that this is likely to be a high-latency request - we will
+  // usually hit the FxA servers to obtain this info.
+  getFxAConnections(aBrowser, aCallbackID) {
    (async () => {
      let setup = !!(await fxAccounts.getSignedInUser());
      let result = { setup };
@ -1761,27 +1809,6 @@ var UITour = {
          }, {});
      }

-      // Each of the "browser services" - currently only "sync" is supported
-      result.browserServices = {};
-      let hasSync = Services.prefs.prefHasUserValue("services.sync.username");
-      if (hasSync) {
-        result.browserServices.sync = {
-          // We always include 'setup' for b/w compatibility.
-          setup: true,
-          desktopDevices: Services.prefs.getIntPref(
-            "services.sync.clients.devices.desktop",
-            0
-          ),
-          mobileDevices: Services.prefs.getIntPref(
-            "services.sync.clients.devices.mobile",
-            0
-          ),
-          totalDevices: Services.prefs.getIntPref(
-            "services.sync.numClients",
-            0
-          ),
-        };
-      }
      try {
        // Each of the "account services", which we turn into a map keyed by ID.
        let attachedClients = await fxAccounts.listAttachedOAuthClients();
@ -1799,9 +1826,6 @@ var UITour = {
      } catch (ex) {
        log.warn("Failed to build the attached clients list", ex);
      }
-      // We check the account state last because it's possible any of the above
-      // calls transitioned it from good -> bad.
-      result.accountStateOK = await fxAccounts.hasLocalSession();
      this.sendPageCallback(aBrowser, aCallbackID, result);
    })().catch(err => {
      log.error(err);
--- a/browser/components/uitour/test/browser_fxa_config.js
+++ b/browser/components/uitour/test/browser_fxa_config.js
@ -37,14 +37,12 @@ add_UITour_task(async function test_no_sync_no_devices() {
  sandbox.stub(fxAccounts, "listAttachedOAuthClients").resolves([]);
  sandbox.stub(fxAccounts, "hasLocalSession").resolves(true);

-  let result = await getConfigurationPromise("fxa");
+  let result = await getConfigurationPromise("fxaConnections");
  Assert.deepEqual(result, {
    setup: true,
-    accountStateOK: true,
    numOtherDevices: 0,
    numDevicesByType: {},
    accountServices: {},
-    browserServices: {},
  });
  sandbox.restore();
 });
@ -91,12 +89,10 @@ add_UITour_task(async function test_no_sync_many_devices() {
  sandbox.stub(fxAccounts, "listAttachedOAuthClients").resolves([]);
  sandbox.stub(fxAccounts, "hasLocalSession").resolves(true);

-  let result = await getConfigurationPromise("fxa");
+  let result = await getConfigurationPromise("fxaConnections");
  Assert.deepEqual(result, {
    setup: true,
-    accountStateOK: true,
    accountServices: {},
-    browserServices: {},
    numOtherDevices: 5,
    numDevicesByType: {
      desktop: 2,
@ -108,7 +104,7 @@ add_UITour_task(async function test_no_sync_many_devices() {
  sandbox.restore();
 });

-add_UITour_task(async function test_no_sync_no_cached_devices() {
+add_UITour_task(async function test_fxa_connections_no_cached_devices() {
  const sandbox = sinon.createSandbox();
  sandbox
    .stub(fxAccounts, "getSignedInUser")
@ -140,12 +136,10 @@ add_UITour_task(async function test_no_sync_no_cached_devices() {
  sandbox.stub(fxAccounts, "hasLocalSession").resolves(true);
  let rdlStub = sandbox.stub(fxAccounts.device, "refreshDeviceList").resolves();

-  let result = await getConfigurationPromise("fxa");
+  let result = await getConfigurationPromise("fxaConnections");
  Assert.deepEqual(result, {
    setup: true,
-    accountStateOK: true,
    accountServices: {},
-    browserServices: {},
    numOtherDevices: 1,
    numDevicesByType: {
      mobile: 1,
@ -155,13 +149,12 @@ add_UITour_task(async function test_no_sync_no_cached_devices() {
  sandbox.restore();
 });

-add_UITour_task(async function test_account_clients() {
+add_UITour_task(async function test_account_connections() {
  const sandbox = sinon.createSandbox();
  sandbox
    .stub(fxAccounts, "getSignedInUser")
    .returns({ email: "foo@example.com" });
  sandbox.stub(fxAccounts.device, "recentDeviceList").get(() => []);
-  sandbox.stub(fxAccounts, "hasLocalSession").resolves(false);
  sandbox.stub(fxAccounts, "listAttachedOAuthClients").resolves([
    {
      id: "802d56ef2a9af9fa",
@ -181,9 +174,8 @@ add_UITour_task(async function test_account_clients() {
      lastAccessedDaysAgo: null,
    },
  ]);
-  Assert.deepEqual(await getConfigurationPromise("fxa"), {
+  Assert.deepEqual(await getConfigurationPromise("fxaConnections"), {
    setup: true,
-    accountStateOK: false,
    numOtherDevices: 0,
    numDevicesByType: {},
    accountServices: {
@ -200,7 +192,6 @@ add_UITour_task(async function test_account_clients() {
        lastAccessedWeeksAgo: null,
      },
    },
-    browserServices: {},
  });
  sandbox.restore();
 });
@ -221,9 +212,6 @@ add_UITour_task(async function test_sync() {
  Assert.deepEqual(await getConfigurationPromise("fxa"), {
    setup: true,
    accountStateOK: true,
-    numOtherDevices: 0,
-    numDevicesByType: {},
-    accountServices: {},
    browserServices: {
      sync: {
        setup: true,