Bug 1257565 - create a pref to phase out use of the XML blocklist, r=aswan
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 07 May 2019 00:55:15 +0000
changeset 472805 f960290161ab3be6793e7eabc27d34573dea9a6e
parent 472804 9ba2aa4a61437e0dd44dc8c5b48d3961b7b8ace1
child 472806 77ec8a41ee7361d9f02511b6992747857931485c
push id35978
push usershindli@mozilla.com
push dateTue, 07 May 2019 09:44:39 +0000
treeherdermozilla-central@7aee5a30dd15 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1257565
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 1257565 - create a pref to phase out use of the XML blocklist, r=aswan Differential Revision: https://phabricator.services.mozilla.com/D29832
toolkit/mozapps/extensions/Blocklist.jsm
toolkit/mozapps/extensions/test/xpcshell/xml-blocklist/test_blocklist_gfx.js
toolkit/mozapps/extensions/test/xpcshell/xml-blocklist/test_overrideblocklist.js
--- a/toolkit/mozapps/extensions/Blocklist.jsm
+++ b/toolkit/mozapps/extensions/Blocklist.jsm
@@ -278,46 +278,28 @@ function getLocale() {
   return Services.locale.requestedLocale;
 }
 
 /* Get the distribution pref values, from defaults only */
 function getDistributionPrefValue(aPrefName) {
   return Services.prefs.getDefaultBranch(null).getCharPref(aPrefName, "default");
 }
 
+let gLoadingWasTriggered = false;
+
 /**
  * Manages the Blocklist. The Blocklist is a representation of the contents of
  * blocklist.xml and allows us to remotely disable / re-enable blocklisted
  * items managed by the Extension Manager with an item's appDisabled property.
  * It also blocklists plugins with data from blocklist.xml.
  */
-var Blocklist = {
+var BlocklistXML = {
   _init() {
-    Services.obs.addObserver(this, "xpcom-shutdown");
-
-    gLoggingEnabled = Services.prefs.getBoolPref(PREF_EM_LOGGING_ENABLED, false);
-    gBlocklistEnabled = Services.prefs.getBoolPref(PREF_BLOCKLIST_ENABLED, true);
-    gBlocklistLevel = Math.min(Services.prefs.getIntPref(PREF_BLOCKLIST_LEVEL, DEFAULT_LEVEL),
-                               MAX_BLOCK_LEVEL);
-    Services.prefs.addObserver("extensions.blocklist.", this);
-    Services.prefs.addObserver(PREF_EM_LOGGING_ENABLED, this);
-
-    // Instantiate Remote Settings clients for blocklists.
-    // Their initialization right here serves two purposes:
-    // - Make sure they are instantiated (it's cheap) in order to be included in the synchronization process;
-    // - Pave the way for Bug 1257565 which will leverage remote settings instead of the XML file
-    //   to manage the blocklists state.
-    BlocklistClients.initialize();
-
-    // If the stub blocklist service deferred any queries because we
-    // weren't loaded yet, execute them now.
-    for (let entry of Services.blocklist.pluginQueries.splice(0)) {
-      entry.resolve(this.getPluginBlocklistState(entry.plugin,
-                                                 entry.appVersion,
-                                                 entry.toolkitVersion));
+    if (gLoadingWasTriggered) {
+      this.loadBlocklistAsync();
     }
   },
 
   STATE_NOT_BLOCKED: Ci.nsIBlocklistService.STATE_NOT_BLOCKED,
   STATE_SOFTBLOCKED: Ci.nsIBlocklistService.STATE_SOFTBLOCKED,
   STATE_BLOCKED: Ci.nsIBlocklistService.STATE_BLOCKED,
   STATE_OUTDATED: Ci.nsIBlocklistService.STATE_OUTDATED,
   STATE_VULNERABLE_UPDATE_AVAILABLE: Ci.nsIBlocklistService.STATE_VULNERABLE_UPDATE_AVAILABLE,
@@ -345,62 +327,33 @@ var Blocklist = {
 
   get profileBlocklistPath() {
     let path = OS.Path.join(OS.Constants.Path.profileDir, FILE_BLOCKLIST);
     Object.defineProperty(this, "profileBlocklistPath", {value: path});
     return path;
   },
 
   shutdown() {
-    Services.obs.removeObserver(this, "xpcom-shutdown");
-    Services.prefs.removeObserver("extensions.blocklist.", this);
-    Services.prefs.removeObserver(PREF_EM_LOGGING_ENABLED, this);
   },
 
-  observe(aSubject, aTopic, aData) {
-    switch (aTopic) {
-    case "xpcom-shutdown":
-      this.shutdown();
-      break;
-    case "profile-after-change":
-      // We're only called here on non-Desktop-Firefox, and use this opportunity to try to
-      // load the blocklist asynchronously. On desktop Firefox, we load the list from
-      // nsBrowserGlue after sessionstore-windows-restored.
-      this.loadBlocklistAsync();
-      break;
-    case "nsPref:changed":
-      switch (aData) {
-        case PREF_EM_LOGGING_ENABLED:
-          gLoggingEnabled = Services.prefs.getBoolPref(PREF_EM_LOGGING_ENABLED, false);
-          break;
-        case PREF_BLOCKLIST_ENABLED:
-          gBlocklistEnabled = Services.prefs.getBoolPref(PREF_BLOCKLIST_ENABLED, true);
-          // This is a bit messy. Especially in tests, but in principle also by users toggling
-          // this preference repeatedly, plugin loads could race with each other if we don't
-          // enforce that they are applied sequentially.
-          // So we only update once the previous `_blocklistUpdated` call finishes running.
-          let lastUpdate = this._lastUpdate || undefined;
-          let newUpdate = this._lastUpdate = (async () => {
-            await lastUpdate;
-            this._clear();
-            await this.loadBlocklistAsync();
-            await this._blocklistUpdated(null, null);
-            if (newUpdate == this._lastUpdate) {
-              delete this._lastUpdate;
-            }
-          })().catch(Cu.reportError);
-          break;
-        case PREF_BLOCKLIST_LEVEL:
-          gBlocklistLevel = Math.min(Services.prefs.getIntPref(PREF_BLOCKLIST_LEVEL, DEFAULT_LEVEL),
-                                     MAX_BLOCK_LEVEL);
-          this._blocklistUpdated(null, null);
-          break;
+  _onBlocklistEnabledToggle() {
+    // This is a bit messy. Especially in tests, but in principle also by users toggling
+    // this preference repeatedly, plugin loads could race with each other if we don't
+    // enforce that they are applied sequentially.
+    // So we only update once the previous `_blocklistUpdated` call finishes running.
+    let lastUpdate = this._lastUpdate || undefined;
+    let newUpdate = this._lastUpdate = (async () => {
+      await lastUpdate;
+      this._clear();
+      await this.loadBlocklistAsync();
+      await this._blocklistUpdated(null, null);
+      if (newUpdate == this._lastUpdate) {
+        delete this._lastUpdate;
       }
-      break;
-    }
+    })().catch(Cu.reportError);
   },
 
   /**
    * Determine the blocklist state of an add-on
    * @param {Addon} addon
    *        The addon item to be checked.
    * @param {string?} appVersion
    *        The version of the application we are checking in the blocklist.
@@ -1611,9 +1564,141 @@ BlocklistItemData.prototype = {
     // getAttribute returns null if the attribute is not present.
     let minVersion = versionRangeElement.getAttribute("minVersion");
     let maxVersion = versionRangeElement.getAttribute("maxVersion");
 
     return { minVersion, maxVersion };
   },
 };
 
+let BlocklistRS = {
+  _init() {
+    // ignore.
+  },
+  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.
+  },
+
+  loadBlocklistAsync() {
+    // Ignore, but ensure that if we start the other service after this, we
+    // initialize it straight away.
+    gLoadingWasTriggered = true;
+  },
+
+  getPluginBlocklistState() {
+  },
+
+  getPluginBlockURL() {
+  },
+
+  getAddonBlocklistState() {
+  },
+
+  getAddonBlocklistEntry() {
+  },
+
+  _blocklistUpdated(oldAddons, oldPlugins) {
+  },
+
+  initializeClients() {
+    BlocklistClients.initialize();
+  },
+};
+
+const kSharedAPIs = [
+  "notify",
+  "loadBlocklistAsync",
+  "getPluginBlockURL",
+  "getPluginBlocklistState",
+  "getAddonBlocklistState",
+  "getAddonBlocklistEntry",
+  "_blocklistUpdated",
+];
+let Blocklist = {
+  _init() {
+    Services.obs.addObserver(this, "xpcom-shutdown");
+    gLoggingEnabled = Services.prefs.getBoolPref(PREF_EM_LOGGING_ENABLED, false);
+    gBlocklistEnabled = Services.prefs.getBoolPref(PREF_BLOCKLIST_ENABLED, true);
+    gBlocklistLevel = Math.min(Services.prefs.getIntPref(PREF_BLOCKLIST_LEVEL, DEFAULT_LEVEL),
+                               MAX_BLOCK_LEVEL);
+    Services.prefs.addObserver("extensions.blocklist.", this);
+    Services.prefs.addObserver(PREF_EM_LOGGING_ENABLED, this);
+
+    // Instantiate Remote Settings clients for blocklists.
+    // Their initialization right here serves two purposes:
+    // - Make sure they are instantiated (it's cheap) in order to be included in the synchronization process;
+    // - Ensure that onecrl and other consumers in there are loaded.
+    // Ideally, this should happen only when BlocklistRS is initialized.
+    BlocklistRS.initializeClients();
+    // Define forwarding functions:
+    for (let k of kSharedAPIs) {
+      this[k] = (...args) => this._impl[k](...args);
+    }
+
+    this.onUpdateImplementation();
+
+    // If the stub blocklist service deferred any queries because we
+    // weren't loaded yet, execute them now.
+    for (let entry of Services.blocklist.pluginQueries.splice(0)) {
+      entry.resolve(this.getPluginBlocklistState(entry.plugin,
+                                                 entry.appVersion,
+                                                 entry.toolkitVersion));
+    }
+  },
+  // the `isLoaded` property needs forwarding too:
+  get isLoaded() {
+    return this._impl.isLoaded;
+  },
+
+  onUpdateImplementation() {
+    this._impl = this.useXML ? BlocklistXML : BlocklistRS;
+    this._impl._init();
+  },
+
+  shutdown() {
+    Services.obs.removeObserver(this, "xpcom-shutdown");
+    Services.prefs.removeObserver("extensions.blocklist.", this);
+    Services.prefs.removeObserver(PREF_EM_LOGGING_ENABLED, this);
+  },
+
+  observe(subject, topic, prefName) {
+    switch (topic) {
+      case "xpcom-shutdown":
+        this.shutdown();
+        break;
+      case "profile-after-change":
+        // We're only called here on non-Desktop-Firefox, and use this opportunity to try to
+        // load the blocklist asynchronously. On desktop Firefox, we load the list from
+        // nsBrowserGlue after sessionstore-windows-restored.
+        this.loadBlocklistAsync();
+        break;
+      case "nsPref:changed":
+        switch (prefName) {
+          case PREF_EM_LOGGING_ENABLED:
+            gLoggingEnabled = Services.prefs.getBoolPref(PREF_EM_LOGGING_ENABLED, false);
+            break;
+          case PREF_BLOCKLIST_ENABLED:
+            gBlocklistEnabled = Services.prefs.getBoolPref(PREF_BLOCKLIST_ENABLED, true);
+            // Historically, this only does something if we're using the XML blocklist,
+            // so check:
+            if (this._impl == BlocklistXML) {
+              this._impl._onBlocklistEnabledToggle();
+            }
+            break;
+          case PREF_BLOCKLIST_LEVEL:
+            gBlocklistLevel = Math.min(Services.prefs.getIntPref(PREF_BLOCKLIST_LEVEL, DEFAULT_LEVEL),
+                                       MAX_BLOCK_LEVEL);
+            this._blocklistUpdated(null, null);
+            break;
+        }
+        break;
+    }
+  },
+};
+
+XPCOMUtils.defineLazyPreferenceGetter(
+  Blocklist, "useXML", "extensions.blocklist.useXML", true,
+  () => Blocklist.onUpdateImplementation());
+
 Blocklist._init();
--- a/toolkit/mozapps/extensions/test/xpcshell/xml-blocklist/test_blocklist_gfx.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/xml-blocklist/test_blocklist_gfx.js
@@ -13,20 +13,21 @@ const SAMPLE_GFX_RECORD = {
   "devices": ["0x0a6c", "geforce"],
   "featureStatus": "BLOCKED_DRIVER_VERSION",
   "last_modified": 1458035931837,
   "os": "WINNT 6.1",
   "id": "3f947f16-37c2-4e96-d356-78b26363729b",
   "versionRange": {"minVersion": 0, "maxVersion": "*"},
 };
 
+let jsmInternalObj = ChromeUtils.import("resource://gre/modules/Blocklist.jsm", null);
 
 function getBlocklist() {
-  Blocklist._clear();
-  return Blocklist;
+  jsmInternalObj.BlocklistXML._clear();
+  return jsmInternalObj.BlocklistXML;
 }
 
 async function updateBlocklistWithInput(input) {
   let blocklist = getBlocklist();
   let promiseObserved = TestUtils.topicObserved(EVENT_NAME);
   blocklist._loadBlocklistFromXML(gParser.parseFromString(input, "text/xml"));
   let [, received] = await promiseObserved;
   return [blocklist, received];
--- a/toolkit/mozapps/extensions/test/xpcshell/xml-blocklist/test_overrideblocklist.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/xml-blocklist/test_overrideblocklist.js
@@ -40,20 +40,22 @@ function clearBlocklists() {
   if (blocklist.exists())
     blocklist.remove(true);
 
   blocklist = FileUtils.getFile(KEY_PROFILEDIR, [FILE_BLOCKLIST]);
   if (blocklist.exists())
     blocklist.remove(true);
 }
 
+let jsmInternalObj = ChromeUtils.import("resource://gre/modules/Blocklist.jsm", null);
+
 async function reloadBlocklist() {
   Services.prefs.setBoolPref(PREF_BLOCKLIST_ENABLED, false);
   Services.prefs.setBoolPref(PREF_BLOCKLIST_ENABLED, true);
-  await Blocklist._lastUpdate;
+  await jsmInternalObj.BlocklistXML._lastUpdate;
 }
 
 function copyToApp(file) {
   file.clone().copyTo(gAppDir, FILE_BLOCKLIST);
 }
 
 function copyToProfile(file) {
   file = file.clone();