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

--HG--
extra : moz-landing-system : lando

toolkit/mozapps/extensions/Blocklist.jsm

@@ -388,6 +388,11 @@ this.GfxBlocklistRS = {
     }
   },
 
+  sync() {
+    this._ensureInitialized();
+    return this._client.sync();
+  },
+
   async checkForEntries() {
     this._ensureInitialized();
     if (!gBlocklistEnabled) {
@@ -564,7 +569,12 @@ this.PluginBlocklistRS = {
     return entry;
   },
 
-  async ensureInitialized() {
+  sync() {
+    this.ensureInitialized();
+    return this._client.sync();
+  },
+
+  ensureInitialized() {
     if (!gBlocklistEnabled || this._initialized) {
       return;
     }
@@ -587,7 +597,7 @@ this.PluginBlocklistRS = {
 
   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);
@@ -883,7 +893,7 @@ this.PluginBlocklistRS = {
  */
 this.ExtensionBlocklistRS = {
   async _ensureEntries() {
-    await this.ensureInitialized();
+    this.ensureInitialized();
     if (!this._entries && gBlocklistEnabled) {
       await this._updateEntries();
     }
@@ -940,7 +950,12 @@ this.ExtensionBlocklistRS = {
     return entry;
   },
 
-  async ensureInitialized() {
+  sync() {
+    this.ensureInitialized();
+    return this._client.sync();
+  },
+
+  ensureInitialized() {
     if (!gBlocklistEnabled || this._initialized) {
       return;
     }
@@ -2528,6 +2543,12 @@ let BlocklistRS = {
     // 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();
@@ -2599,9 +2620,23 @@ let Blocklist = {
     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() {
@@ -2650,6 +2685,6 @@ let Blocklist = {
 
 XPCOMUtils.defineLazyPreferenceGetter(
   Blocklist, "useXML", "extensions.blocklist.useXML", true,
-  () => Blocklist.onUpdateImplementation());
+  () => Blocklist.onUpdateImplementation(true));
 
 Blocklist._init();

toolkit/mozapps/extensions/internal/AddonTestUtils.jsm

@@ -770,7 +770,7 @@ var AddonTestUtils = {
           item.last_modified = Date.now();
         }
       }
-      await blocklistObj.ensureInitialized();
+      blocklistObj.ensureInitialized();
       let collection = await blocklistObj._client.openCollection();
       await collection.clear();
       await collection.loadDump(newData);

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.");
+});

toolkit/mozapps/extensions/test/xpcshell/rs-blocklist/xpcshell.ini

@@ -57,3 +57,4 @@ skip-if = true
 fail-if = os == "android"
 [test_pluginInfoURL.js]
 [test_softblocked.js]
+[test_switchImplementations.js]