Bug 1541428 - Improve invalidation of polling when adding new Remote Settings client r=glasserc
authorMathieu Leplatre <mathieu@mozilla.com>
Tue, 09 Apr 2019 17:43:47 +0000
changeset 468606 e05177437976417d180e1173ef99e3c17079ca34
parent 468605 5c5765ae3e648c812c09e86b76ca59d297363cb2
child 468607 8cbdae26bf7b3727f7a108fb2af8e5012d07b3d7
push id35843
push usernbeleuzu@mozilla.com
push dateTue, 09 Apr 2019 22:08:13 +0000
treeherdermozilla-central@a31032a16330 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglasserc
bugs1541428
milestone68.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 1541428 - Improve invalidation of polling when adding new Remote Settings client r=glasserc Improve invalidation of polling when adding new Remote Settings client Differential Revision: https://phabricator.services.mozilla.com/D26690
services/settings/remote-settings.js
services/settings/test/unit/test_remote_settings_poll.js
--- a/services/settings/remote-settings.js
+++ b/services/settings/remote-settings.js
@@ -76,16 +76,17 @@ async function jexlFilterFunc(entry, env
     Cu.reportError(e);
   }
   return result ? entry : null;
 }
 
 
 function remoteSettingsFunction() {
   const _clients = new Map();
+  let _invalidatePolling = false;
 
   // If not explicitly specified, use the default signer.
   const defaultSigner = gPrefs.getCharPref(PREF_SETTINGS_DEFAULT_SIGNER);
   const defaultOptions = {
     bucketNamePref: PREF_SETTINGS_DEFAULT_BUCKET,
     signerName: defaultSigner,
     filterFunc: jexlFilterFunc,
   };
@@ -101,17 +102,17 @@ function remoteSettingsFunction() {
     // Get or instantiate a remote settings client.
     if (!_clients.has(collectionName)) {
       // Register a new client!
       const c = new RemoteSettingsClient(collectionName, { ...defaultOptions, ...options });
       // Store instance for later call.
       _clients.set(collectionName, c);
       // Invalidate the polling status, since we want the new collection to
       // be taken into account.
-      gPrefs.clearUserPref(PREF_SETTINGS_LAST_ETAG);
+      _invalidatePolling = true;
     }
     return _clients.get(collectionName);
   };
 
   Object.defineProperty(remoteSettings, "pollingEndpoint", {
     get() {
       return gServerURL + gChangesPath;
     },
@@ -174,17 +175,19 @@ function remoteSettingsFunction() {
         throw new Error(`Server is asking clients to back off; retry in ${Math.ceil(remainingMilliseconds / 1000)}s.`);
       } else {
         gPrefs.clearUserPref(PREF_SETTINGS_SERVER_BACKOFF);
       }
     }
 
     Services.obs.notifyObservers(null, "remote-settings:changes-poll-start", JSON.stringify({ expectedTimestamp }));
 
-    const lastEtag = gPrefs.getCharPref(PREF_SETTINGS_LAST_ETAG, "");
+    // Do we have the latest version already?
+    // Every time we register a new client, we have to fetch the whole list again.
+    const lastEtag = _invalidatePolling ? "" : gPrefs.getCharPref(PREF_SETTINGS_LAST_ETAG, "");
 
     let pollResult;
     try {
       pollResult = await Utils.fetchLatestChanges(remoteSettings.pollingEndpoint, { expectedTimestamp, lastEtag });
     } catch (e) {
       // Report polling error to Uptake Telemetry.
       let reportStatus;
       if (/JSON\.parse/.test(e.message)) {
@@ -252,16 +255,20 @@ function remoteSettingsFunction() {
         Services.prefs.setIntPref(client.lastCheckTimePref, checkedServerTimeInSeconds);
       } catch (e) {
         if (!firstError) {
           firstError = e;
           firstError.details = change;
         }
       }
     }
+
+    // Polling is done.
+    _invalidatePolling = false;
+
     // Report total synchronization duration to Telemetry.
     const durationMilliseconds = new Date() - startedAt;
     const syncTelemetryArgs = { source: TELEMETRY_SOURCE_SYNC, duration: durationMilliseconds, trigger };
 
     if (firstError) {
       // Report the global synchronization failure. Individual uptake reports will also have been sent for each collection.
       await UptakeTelemetry.report(TELEMETRY_COMPONENT, UptakeTelemetry.STATUS.SYNC_ERROR, syncTelemetryArgs);
       // Rethrow the first observed error
--- a/services/settings/test/unit/test_remote_settings_poll.js
+++ b/services/settings/test/unit/test_remote_settings_poll.js
@@ -755,17 +755,17 @@ add_task(async function test_syncs_clien
   // The `example` has a dump, and should cause a network error because the test
   // does not setup the server to receive the requests of `maybeSync()`.
   Assert.ok(/HTTP 404/.test(error.message), "server will return 404 on sync");
   Assert.equal(error.details.collection, "example");
 });
 add_task(clear_state);
 
 
-add_task(async function test_adding_client_resets_last_etag() {
+add_task(async function test_adding_client_resets_polling() {
   function serve200or304(request, response) {
     const entries = [{
       id: "aa71e6cc-9f37-447a-b6e0-c025e8eabd03",
       last_modified: 42,
       host: "localhost",
       bucket: "main",
       collection: "a-collection",
     }];
@@ -795,9 +795,14 @@ add_task(async function test_adding_clie
   const c = RemoteSettings("a-collection");
   c.maybeSync = () => { maybeSyncCalled = true; };
 
   // Poll again.
   await RemoteSettings.pollChanges();
 
   // The new client was called, even if the server data didn't change.
   Assert.ok(maybeSyncCalled);
+
+  // Poll again. This time maybeSync() won't be called.
+  maybeSyncCalled = false;
+  await RemoteSettings.pollChanges();
+  Assert.ok(!maybeSyncCalled);
 });