Bug 1656426 - ensure we re-fetch the profile after racing another client. r=rfkelly

Differential Revision: https://phabricator.services.mozilla.com/D85539

services/fxaccounts/FxAccountsProfile.jsm

@@ -166,9 +166,13 @@ FxAccountsProfile.prototype = {
   // Get the user's profile data, fetching from the network if necessary.
   // Most callers should instead use `getProfile()`; this methods exists to support
   // callers who need to await the underlying network request.
-  async ensureProfile({ staleOk = false } = {}) {
+  async ensureProfile({ staleOk = false, forceFresh = false } = {}) {
+    if (staleOk && forceFresh) {
+      throw new Error("contradictory options specified");
+    }
     const profileCache = await this._getProfileCache();
     if (
+      forceFresh ||
       !profileCache ||
       (Date.now() > this._cachedAt + this.PROFILE_FRESHNESS_THRESHOLD &&
         !staleOk)

services/fxaccounts/FxAccountsTelemetry.jsm

@@ -121,12 +121,18 @@ class FxAccountsTelemetry {
   async _ensureEcosystemAnonId(generatePlaceholder = true) {
     const telemetry = this;
     return this._fxai.withCurrentAccountState(async function(state) {
-      // Fetching a fresh profile should never update the ID, and saving a
-      // network request matters for telemetry, so we are fine with a slightly
-      // stale profile.
-      const profile = await telemetry._fxai.profile.ensureProfile({
-        staleOk: true,
-      });
+      // Fetching a fresh profile should never *change* the ID, but it might
+      // fetch the first value we see, and saving a network request matters for
+      // telemetry, so:
+      // * first time around we are fine with a slightly stale profile - if it
+      //   has an ID, it's a stable ID we can be sure is good.
+      // * But if we didn't have one, so generated a new one, but then raced
+      //   with another client to update it, we *must* fetch a new profile, even
+      //   if our current version is fresh.
+      let options = generatePlaceholder
+        ? { staleOk: true }
+        : { forceFresh: true };
+      const profile = await telemetry._fxai.profile.ensureProfile(options);
       if (profile && profile.hasOwnProperty("ecosystemAnonId")) {
         return profile.ecosystemAnonId;
       }

services/fxaccounts/tests/xpcshell/test_profile.js

@@ -436,12 +436,12 @@ add_task(async function test_ensureProfile() {
       threshold: 1000,
       expectsCachedProfileReturned: true,
       cachedProfile: {
-        uid: `${ACCOUNT_UID}5`,
-        email: `${ACCOUNT_EMAIL}5`,
-        avatar: "myimg5",
+        uid: `${ACCOUNT_UID}8`,
+        email: `${ACCOUNT_EMAIL}8`,
+        avatar: "myimg8",
       },
       fetchAndCacheProfileResolves: false,
-      staleOk: true,
+      options: { staleOk: true },
     },
     // staleOk but no cached profile
     {
@@ -449,11 +449,24 @@ add_task(async function test_ensureProfile() {
       expectsCachedProfileReturned: false,
       cachedProfile: null,
       fetchedProfile: {
-        uid: ACCOUNT_UID,
-        email: ACCOUNT_EMAIL,
-        avatar: "myimg",
+        uid: `${ACCOUNT_UID}9`,
+        email: `${ACCOUNT_EMAIL}9`,
+        avatar: "myimg9",
       },
-      staleOk: true,
+      options: { staleOk: true },
+    },
+    // fresh profile but forceFresh = true
+    {
+      // Note: The threshold for this test case is being set to an arbitrary value that will
+      // be greater than Date.now() so the retrieved cached profile will be deemed recent.
+      threshold: Date.now() + 5000,
+      expectsCachedProfileReturned: false,
+      fetchedProfile: {
+        uid: `${ACCOUNT_UID}10`,
+        email: `${ACCOUNT_EMAIL}10`,
+        avatar: "myimg10",
+      },
+      options: { forceFresh: true },
     },
   ];
 
@@ -472,10 +485,7 @@ add_task(async function test_ensureProfile() {
       );
     profile.PROFILE_FRESHNESS_THRESHOLD = tc.threshold;
 
-    let options = {};
-    if (tc.staleOk || false) {
-      options.staleOk = true;
-    }
+    let options = tc.options || {};
     if (tc.expectsCachedProfileReturned) {
       mockProfile.expects("_fetchAndCacheProfile").never();
       let actualProfile = await profile.ensureProfile(options);

services/fxaccounts/tests/xpcshell/test_telemetry.js

@@ -280,6 +280,7 @@ add_task(async function test_ensureEcosystemAnonId_clientRace() {
 
     mockProfile
       .expects("ensureProfile")
+      .withArgs(sinon.match({ staleOk: true }))
       .once()
       .returns({});
 
@@ -306,6 +307,7 @@ add_task(async function test_ensureEcosystemAnonId_clientRace() {
     if (tc.errorCode === 412) {
       mockProfile
         .expects("ensureProfile")
+        .withArgs(sinon.match({ forceFresh: true }))
         .once()
         .returns({
           ecosystemAnonId: expectedEcosystemAnonId,