Bug 1559353 - ensure XML and remote settings lists are updated immediately when switching between them, r=leplatrem
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 19 Jun 2019 17:35:17 +0000
changeset 479226 eba0ea8c0a9e2a2eee0c6a013b1006a4b131d14f
parent 479225 40860d14200df2bc1219d2018fa1b56ac178ac26
child 479227 87aac9c06931c3048a09b7f3fc1e3532bb31adf1
push id36174
push useropoprus@mozilla.com
push dateWed, 19 Jun 2019 21:38:13 +0000
treeherdermozilla-central@5b9a3de04646 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersleplatrem
bugs1559353
milestone69.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 1559353 - ensure XML and remote settings lists are updated immediately when switching between them, r=leplatrem Differential Revision: https://phabricator.services.mozilla.com/D35032
toolkit/mozapps/extensions/Blocklist.jsm
toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_switchImplementations.js
toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/xpcshell.ini
--- a/toolkit/mozapps/extensions/Blocklist.jsm
+++ b/toolkit/mozapps/extensions/Blocklist.jsm
@@ -383,16 +383,21 @@ this.GfxBlocklistRS = {
   },
 
   shutdown() {
     if (this._client) {
       this._client.off("sync", this.checkForEntries);
     }
   },
 
+  sync() {
+    this._ensureInitialized();
+    return this._client.sync();
+  },
+
   async checkForEntries() {
     this._ensureInitialized();
     if (!gBlocklistEnabled) {
       return []; // return value expected by tests.
     }
     let entries = await this._client.get();
     // Trim helper (spaces, tabs, no-break spaces..)
     const trim = (s) => (s || "").replace(/(^[\s\uFEFF\xA0]+)|([\s\uFEFF\xA0]+$)/g, "");
@@ -559,17 +564,22 @@ this.PluginBlocklistRS = {
     }
     if (!entry.matchFilename && !entry.matchName && !entry.matchDescription) {
       Cu.reportError(new Error("Nothing to filter plugin item " + entry.blockID + " on"));
       return null;
     }
     return entry;
   },
 
-  async ensureInitialized() {
+  sync() {
+    this.ensureInitialized();
+    return this._client.sync();
+  },
+
+  ensureInitialized() {
     if (!gBlocklistEnabled || this._initialized) {
       return;
     }
     this._initialized = true;
     this._client = RemoteSettings(Services.prefs.getCharPref(PREF_BLOCKLIST_PLUGINS_COLLECTION), {
       bucketNamePref: PREF_BLOCKLIST_BUCKET,
       lastCheckTimePref: PREF_BLOCKLIST_PLUGINS_CHECKED_SECONDS,
       signerName: Services.prefs.getCharPref(PREF_BLOCKLIST_PLUGINS_SIGNER),
@@ -582,17 +592,17 @@ this.PluginBlocklistRS = {
   shutdown() {
     if (this._client) {
       this._client.off("sync", this._onUpdate);
     }
   },
 
   async _onUpdate() {
     let oldEntries = this._entries || [];
-    await this.ensureInitialized();
+    this.ensureInitialized();
     await this._updateEntries();
     const pluginHost = Cc["@mozilla.org/plugin/host;1"].
                          getService(Ci.nsIPluginHost);
     const plugins = pluginHost.getPluginTags();
 
     let blockedItems = [];
 
     for (let plugin of plugins) {
@@ -878,17 +888,17 @@ this.PluginBlocklistRS = {
  *   "id": "<unique guid>",
  *   "last_modified": 1480349215672,
  * }
  *
  * Note: we assign to the global to allow tests to reach the object directly.
  */
 this.ExtensionBlocklistRS = {
   async _ensureEntries() {
-    await this.ensureInitialized();
+    this.ensureInitialized();
     if (!this._entries && gBlocklistEnabled) {
       await this._updateEntries();
     }
   },
 
   async _updateEntries() {
     if (!gBlocklistEnabled) {
       this._entries = [];
@@ -935,17 +945,22 @@ this.ExtensionBlocklistRS = {
     // Need something to filter on - at least a guid or name (either could be a regex):
     if (!entry.guid && !entry.name) {
       Cu.reportError(new Error("Nothing to filter add-on item " + entry.blockID + " on"));
       return null;
     }
     return entry;
   },
 
-  async ensureInitialized() {
+  sync() {
+    this.ensureInitialized();
+    return this._client.sync();
+  },
+
+  ensureInitialized() {
     if (!gBlocklistEnabled || this._initialized) {
       return;
     }
     this._initialized = true;
     this._client = RemoteSettings(Services.prefs.getCharPref(PREF_BLOCKLIST_ADDONS_COLLECTION), {
       bucketNamePref: PREF_BLOCKLIST_BUCKET,
       lastCheckTimePref: PREF_BLOCKLIST_ADDONS_CHECKED_SECONDS,
       signerName: Services.prefs.getCharPref(PREF_BLOCKLIST_ADDONS_SIGNER),
@@ -2523,16 +2538,22 @@ let BlocklistRS = {
   },
   isLoaded: true,
 
   notify() {
     // ignore. We might miss a timer notification once if the XML impl. is disabled
     // when the timer fires and subsequently gets enabled. That seems OK.
   },
 
+  forceUpdate() {
+    for (let blocklist of [GfxBlocklistRS, ExtensionBlocklistRS, PluginBlocklistRS]) {
+      blocklist.sync().catch(Cu.reportError);
+    }
+  },
+
   loadBlocklistAsync() {
     // Need to ensure we notify gfx of new stuff.
     GfxBlocklistRS.checkForEntries();
     ExtensionBlocklistRS.ensureInitialized();
     PluginBlocklistRS.ensureInitialized();
     // Also ensure that if we start the other service after this, we
     // initialize it straight away.
     gLoadingWasTriggered = true;
@@ -2594,19 +2615,33 @@ let Blocklist = {
                                                  entry.toolkitVersion));
     }
   },
   // the `isLoaded` property needs forwarding too:
   get isLoaded() {
     return this._impl.isLoaded;
   },
 
-  onUpdateImplementation() {
+  onUpdateImplementation(shouldCheckForUpdates = false) {
     this._impl = this.useXML ? BlocklistXML : BlocklistRS;
     this._impl._init();
+    if (shouldCheckForUpdates) {
+      if (this.useXML) {
+        // In theory, we should be able to use the "last update" pref to figure out
+        // when we last fetched updates and avoid fetching it if we switched
+        // this on and off within a day. However, the pref is updated by the
+        // update timer code, but the request is made in our code - and a request is
+        // not made if we're not using the XML blocklist, despite the pref being
+        // updated. In other words, the pref being updated is no guarantee that we
+        // actually updated the list that day. So just unconditionally update it:
+        this._impl.notify();
+      } else {
+        this._impl.forceUpdate();
+      }
+    }
   },
 
   shutdown() {
     this._impl.shutdown();
     Services.obs.removeObserver(this, "xpcom-shutdown");
     Services.prefs.removeObserver("extensions.blocklist.", this);
     Services.prefs.removeObserver(PREF_EM_LOGGING_ENABLED, this);
   },
@@ -2645,11 +2680,11 @@ let Blocklist = {
         }
         break;
     }
   },
 };
 
 XPCOMUtils.defineLazyPreferenceGetter(
   Blocklist, "useXML", "extensions.blocklist.useXML", true,
-  () => Blocklist.onUpdateImplementation());
+  () => Blocklist.onUpdateImplementation(true));
 
 Blocklist._init();
--- a/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
+++ b/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
@@ -765,17 +765,17 @@ var AddonTestUtils = {
       for (let item of newData) {
         if (!item.id) {
           item.id = uuidGen.generateUUID().number.slice(1, -1);
         }
         if (!item.last_modified) {
           item.last_modified = Date.now();
         }
       }
-      await blocklistObj.ensureInitialized();
+      blocklistObj.ensureInitialized();
       let collection = await blocklistObj._client.openCollection();
       await collection.clear();
       await collection.loadDump(newData);
       // We manually call _onUpdate... which is evil, but at the moment kinto doesn't have
       // a better abstraction unless you want to mock your own http server to do the update.
       await blocklistObj._onUpdate();
     }
   },
new file mode 100644
--- /dev/null
+++ b/toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/test_switchImplementations.js
@@ -0,0 +1,40 @@
+/* Any copyright is dedicated to the Public Domain.
+http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const PREF_BLOCKLIST_URL              = "extensions.blocklist.url";
+
+add_task(async function test_switch_blocklist_implementations() {
+  createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1");
+  // In this folder, `useXML` is already set, and set to false, ie we're using remote settings.
+  Services.prefs.setCharPref(PREF_BLOCKLIST_URL, "http://localhost/blocklist.xml");
+  let {Blocklist} = ChromeUtils.import("resource://gre/modules/Blocklist.jsm");
+  await Blocklist.loadBlocklistAsync();
+  // Observe request:
+  let sentRequest = TestUtils.topicObserved("http-on-modify-request", function check(subject, data) {
+    let httpChannel = subject.QueryInterface(Ci.nsIHttpChannel);
+    info("Got request for: " + httpChannel.URI.spec);
+    return httpChannel.URI.spec.startsWith("http://localhost/blocklist.xml");
+  });
+  // Switch to XML:
+  Services.prefs.setBoolPref("extensions.blocklist.useXML", true);
+  // Check we're updating:
+  await sentRequest;
+  ok(true, "We got an update request for the XML list when switching");
+
+  // Now do the same for the remote settings list:
+  let {Utils} = ChromeUtils.import("resource://services-settings/Utils.jsm");
+  // Mock the 'get latest changes' endpoint so we don't need a network request for it:
+  Utils.fetchLatestChanges = () => Promise.resolve({changes: [{last_modified: Date.now()}]});
+  let requestPromises = ["addons", "plugins", "gfx"].map(slug => {
+    return TestUtils.topicObserved("http-on-modify-request", function check(subject, data) {
+      let httpChannel = subject.QueryInterface(Ci.nsIHttpChannel);
+      info("Got request for: " + httpChannel.URI.spec);
+      return httpChannel.URI.spec.includes("buckets/blocklists/collections/" + slug);
+    });
+  });
+  Services.prefs.setBoolPref("extensions.blocklist.useXML", false);
+  await Promise.all(requestPromises);
+  ok(true, "We got update requests for all the remote settings lists when switching.");
+});
--- a/toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/xpcshell.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/xpcshell.ini
@@ -52,8 +52,9 @@ requesttimeoutfactor = 2
 [test_gfxBlacklist_prefs.js]
 # Bug 1248787 - consistently fails
 skip-if = true
 [test_pluginBlocklistCtp.js]
 # Bug 676992: test consistently fails on Android
 fail-if = os == "android"
 [test_pluginInfoURL.js]
 [test_softblocked.js]
+[test_switchImplementations.js]