Bug 1454378 - cache and inline things, avoid duplicate attribute or property requests, to make blocklist faster, r?florian draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 13 Jun 2018 17:06:49 -0700
changeset 807834 10eb9961b2f45ae488ff24921cb3350b5ba658a0
parent 807833 3c747d2bb4219892a91ea1fc7d116708e33c0a42
child 807835 307da375c9af61a79353d15f19c2219e4afca0ae
push id113224
push userbmo:gijskruitbosch+bugs@gmail.com
push dateFri, 15 Jun 2018 20:35:10 +0000
reviewersflorian
bugs1454378
milestone62.0a1
Bug 1454378 - cache and inline things, avoid duplicate attribute or property requests, to make blocklist faster, r?florian MozReview-Commit-ID: BwBhZr6sqx2
toolkit/mozapps/extensions/Blocklist.jsm
toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js
--- a/toolkit/mozapps/extensions/Blocklist.jsm
+++ b/toolkit/mozapps/extensions/Blocklist.jsm
@@ -176,16 +176,24 @@ XPCOMUtils.defineLazyGetter(this, "gApp"
     // Not all applications implement nsIXULAppInfo (e.g. xpcshell doesn't).
     if (!(ex instanceof Components.Exception) ||
         ex.result != Cr.NS_NOINTERFACE)
       throw ex;
   }
   return appinfo;
 });
 
+XPCOMUtils.defineLazyGetter(this, "gAppID", function() {
+  return gApp.ID;
+});
+
+XPCOMUtils.defineLazyGetter(this, "gAppVersion", function() {
+  return gApp.version;
+});
+
 XPCOMUtils.defineLazyGetter(this, "gABI", function() {
   let abi = null;
   try {
     abi = gApp.XPCOMABI;
   } catch (e) {
     LOG("BlockList Global gABI: XPCOM ABI unknown.");
   }
   return abi;
@@ -243,24 +251,26 @@ function restartApp() {
  * xpcomabi attribute.
  *
  * @param {Element} blocklistElement
  *        The blocklist element from an XML blocklist.
  * @returns {bool}
  *        Whether the entry matches the current OS.
  */
 function matchesOSABI(blocklistElement) {
-  if (blocklistElement.hasAttribute("os")) {
-    var choices = blocklistElement.getAttribute("os").split(",");
+  let os = blocklistElement.getAttribute("os");
+  if (os) {
+    let choices = os.split(",");
     if (choices.length > 0 && !choices.includes(gApp.OS))
       return false;
   }
 
-  if (blocklistElement.hasAttribute("xpcomabi")) {
-    choices = blocklistElement.getAttribute("xpcomabi").split(",");
+  let xpcomabi = blocklistElement.getAttribute("xpcomabi");
+  if (xpcomabi) {
+    let choices = xpcomabi.split(",");
     if (choices.length > 0 && !choices.includes(gApp.XPCOMABI))
       return false;
   }
 
   return true;
 }
 
 /**
@@ -275,32 +285,16 @@ function getLocale() {
 }
 
 /* Get the distribution pref values, from defaults only */
 function getDistributionPrefValue(aPrefName) {
   return Services.prefs.getDefaultBranch(null).getCharPref(aPrefName, "default");
 }
 
 /**
- * Parse a string representation of a regular expression. Needed because we
- * use the /pattern/flags form (because it's detectable), which is only
- * supported as a literal in JS.
- *
- * @param {string} aStr
- *         String representation of regexp
- * @return {RegExp} instance
- */
-function parseRegExp(aStr) {
-  let lastSlash = aStr.lastIndexOf("/");
-  let pattern = aStr.slice(1, lastSlash);
-  let flags = aStr.slice(lastSlash + 1);
-  return new RegExp(pattern, flags);
-}
-
-/**
  * 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 = {
   _init() {
     Services.obs.addObserver(this, "xpcom-shutdown");
@@ -435,21 +429,21 @@ var Blocklist = {
    *          properties indicating the block state and URL, if there is
    *          a matching blocklist entry, or null otherwise.
    */
   _getAddonBlocklistEntry(addon, addonEntries, appVersion, toolkitVersion) {
     if (!gBlocklistEnabled)
       return null;
 
     // Not all applications implement nsIXULAppInfo (e.g. xpcshell doesn't).
-    if (!appVersion && !gApp.version)
+    if (!appVersion && !gAppVersion)
       return null;
 
     if (!appVersion)
-      appVersion = gApp.version;
+      appVersion = gAppVersion;
     if (!toolkitVersion)
       toolkitVersion = gApp.platformVersion;
 
     var blItem = this._findMatchingAddonEntry(addonEntries, addon);
     if (!blItem)
       return null;
 
     for (let currentblItem of blItem.versions) {
@@ -530,17 +524,17 @@ var Blocklist = {
 
   _findMatchingAddonEntry(aAddonEntries, aAddon) {
     if (!aAddon)
       return null;
     // Returns true if the params object passes the constraints set by entry.
     // (For every non-null property in entry, the same key must exist in
     // params and value must be the same)
     function checkEntry(entry, params) {
-      for (let [key, value] of entry) {
+      for (let [key, value] of Object.entries(entry)) {
         if (value === null || value === undefined)
           continue;
         if (params[key]) {
           if (value instanceof RegExp) {
             if (!value.test(params[key])) {
               return false;
             }
           } else if (value !== params[key]) {
@@ -606,24 +600,24 @@ var Blocklist = {
       }
     }
 
     if (pingCountVersion < 1)
       pingCountVersion = 1;
     if (pingCountTotal < 1)
       pingCountTotal = 1;
 
-    dsURI = dsURI.replace(/%APP_ID%/g, gApp.ID);
+    dsURI = dsURI.replace(/%APP_ID%/g, gAppID);
     // Not all applications implement nsIXULAppInfo (e.g. xpcshell doesn't).
-    if (gApp.version)
-      dsURI = dsURI.replace(/%APP_VERSION%/g, gApp.version);
+    if (gAppVersion)
+      dsURI = dsURI.replace(/%APP_VERSION%/g, gAppVersion);
     dsURI = dsURI.replace(/%PRODUCT%/g, gApp.name);
     // Not all applications implement nsIXULAppInfo (e.g. xpcshell doesn't).
-    if (gApp.version)
-      dsURI = dsURI.replace(/%VERSION%/g, gApp.version);
+    if (gAppVersion)
+      dsURI = dsURI.replace(/%VERSION%/g, gAppVersion);
     dsURI = dsURI.replace(/%BUILD_ID%/g, gApp.appBuildID);
     dsURI = dsURI.replace(/%BUILD_TARGET%/g, gApp.OS + "_" + gABI);
     dsURI = dsURI.replace(/%OS_VERSION%/g, gOSVersion);
     dsURI = dsURI.replace(/%LOCALE%/g, getLocale());
     dsURI = dsURI.replace(/%CHANNEL%/g, UpdateUtils.UpdateChannel);
     dsURI = dsURI.replace(/%PLATFORM_VERSION%/g, gApp.platformVersion);
     dsURI = dsURI.replace(/%DISTRIBUTION%/g,
                       getDistributionPrefValue(PREF_APP_DISTRIBUTION));
@@ -919,43 +913,50 @@ var Blocklist = {
   _handleEmItemNode(blocklistElement, result) {
     if (!matchesOSABI(blocklistElement))
       return;
 
     let blockEntry = {
       versions: [],
       prefs: [],
       blockID: null,
-      attributes: new Map()
+      attributes: {},
       // Atleast one of EXTENSION_BLOCK_FILTERS must get added to attributes
     };
 
     // Any filter starting with '/' is interpreted as a regex. So if an attribute
     // starts with a '/' it must be checked via a regex.
     function regExpCheck(attr) {
-      return attr.startsWith("/") ? parseRegExp(attr) : attr;
+      if (attr.startsWith("/")) {
+        let lastSlash = attr.lastIndexOf("/");
+        let pattern = attr.slice(1, lastSlash);
+        let flags = attr.slice(lastSlash + 1);
+        return new RegExp(pattern, flags);
+      }
+      return attr;
     }
 
     for (let filter of EXTENSION_BLOCK_FILTERS) {
       let attr = blocklistElement.getAttribute(filter);
       if (attr)
-        blockEntry.attributes.set(filter, regExpCheck(attr));
+        blockEntry.attributes[filter] = regExpCheck(attr);
     }
 
     var children = blocklistElement.children;
 
     for (let childElement of children) {
-      if (childElement.localName === "prefs") {
+      let localName = childElement.localName;
+      if (localName == "prefs" && childElement.hasChildNodes) {
         let prefElements = childElement.children;
         for (let prefElement of prefElements) {
           if (prefElement.localName !== "pref")
             continue;
           blockEntry.prefs.push(prefElement.textContent);
         }
-      } else if (childElement.localName === "versionRange") {
+      } else if (localName == "versionRange") {
         blockEntry.versions.push(new BlocklistItemData(childElement));
       }
     }
     // if only the extension ID is specified block all versions of the
     // extension for the current application.
     if (blockEntry.versions.length == 0)
       blockEntry.versions.push(new BlocklistItemData(null));
 
@@ -972,29 +973,33 @@ var Blocklist = {
     var blockEntry = {
       matches: {},
       versions: [],
       blockID: null,
       infoURL: null,
     };
     var hasMatch = false;
     for (let childElement of children) {
-      if (childElement.localName == "match") {
-        var name = childElement.getAttribute("name");
-        var exp = childElement.getAttribute("exp");
-        try {
-          blockEntry.matches[name] = new RegExp(exp, "m");
-          hasMatch = true;
-        } catch (e) {
-          // Ignore invalid regular expressions
-        }
-      } else if (childElement.localName == "versionRange") {
-        blockEntry.versions.push(new BlocklistItemData(childElement));
-      } else if (childElement.localName == "infoURL") {
-        blockEntry.infoURL = childElement.textContent;
+      switch (childElement.localName) {
+        case "match":
+          var name = childElement.getAttribute("name");
+          var exp = childElement.getAttribute("exp");
+          try {
+            blockEntry.matches[name] = new RegExp(exp, "m");
+            hasMatch = true;
+          } catch (e) {
+            // Ignore invalid regular expressions
+          }
+          break;
+        case "versionRange":
+          blockEntry.versions.push(new BlocklistItemData(childElement));
+          break;
+        case "infoURL":
+          blockEntry.infoURL = childElement.textContent;
+          break;
       }
     }
     // Plugin entries require *something* to match to an actual plugin
     if (!hasMatch)
       return;
     // Add a default versionRange if there wasn't one specified
     if (blockEntry.versions.length == 0)
       blockEntry.versions.push(new BlocklistItemData(null));
@@ -1092,21 +1097,21 @@ var Blocklist = {
    *        {entry: blocklistEntry, version: blocklistEntryVersion},
    *        or null if there is no matching entry.
    */
   _getPluginBlocklistEntry(plugin, pluginEntries, appVersion, toolkitVersion) {
     if (!gBlocklistEnabled)
       return null;
 
     // Not all applications implement nsIXULAppInfo (e.g. xpcshell doesn't).
-    if (!appVersion && !gApp.version)
+    if (!appVersion && !gAppVersion)
       return Ci.nsIBlocklistService.STATE_NOT_BLOCKED;
 
     if (!appVersion)
-      appVersion = gApp.version;
+      appVersion = gAppVersion;
     if (!toolkitVersion)
       toolkitVersion = gApp.platformVersion;
 
     const pluginProperties = {
       description: plugin.description,
       filename: plugin.filename,
       name: plugin.name,
       version: plugin.version,
@@ -1412,45 +1417,45 @@ var Blocklist = {
       blocklistWindow.addEventListener("unload", blocklistUnloadHandler);
   },
 };
 
 /*
  * Helper for constructing a blocklist.
  */
 function BlocklistItemData(versionRangeElement) {
-  var versionRange = this.getBlocklistVersionRange(versionRangeElement);
-  this.minVersion = versionRange.minVersion;
-  this.maxVersion = versionRange.maxVersion;
-  if (versionRangeElement && versionRangeElement.hasAttribute("severity"))
-    this.severity = versionRangeElement.getAttribute("severity");
-  else
-    this.severity = DEFAULT_SEVERITY;
-  if (versionRangeElement && versionRangeElement.hasAttribute("vulnerabilitystatus")) {
-    this.vulnerabilityStatus = versionRangeElement.getAttribute("vulnerabilitystatus");
-  } else {
-    this.vulnerabilityStatus = VULNERABILITYSTATUS_NONE;
-  }
-  this.targetApps = { };
-  var found = false;
-
+  this.targetApps = {};
+  let foundTarget = false;
+  this.severity = DEFAULT_SEVERITY;
+  this.vulnerabilityStatus = VULNERABILITYSTATUS_NONE;
   if (versionRangeElement) {
+    let versionRange = this.getBlocklistVersionRange(versionRangeElement);
+    this.minVersion = versionRange.minVersion;
+    this.maxVersion = versionRange.maxVersion;
+    if (versionRangeElement.hasAttribute("severity"))
+      this.severity = versionRangeElement.getAttribute("severity");
+    if (versionRangeElement.hasAttribute("vulnerabilitystatus")) {
+      this.vulnerabilityStatus = versionRangeElement.getAttribute("vulnerabilitystatus");
+    }
     for (let targetAppElement of versionRangeElement.children) {
       if (targetAppElement.localName != "targetApplication")
         continue;
-      found = true;
+      foundTarget = true;
       // default to the current application if id is not provided.
-      var appID = targetAppElement.hasAttribute("id") ? targetAppElement.getAttribute("id") : gApp.ID;
+      let appID = targetAppElement.id || gAppID;
       this.targetApps[appID] = this.getBlocklistAppVersions(targetAppElement);
     }
+  } else {
+    this.minVersion = this.maxVersion = null;
   }
+
   // Default to all versions of the current application when no targetApplication
   // elements were found
-  if (!found)
-    this.targetApps[gApp.ID] = this.getBlocklistAppVersions(null);
+  if (!foundTarget)
+    this.targetApps[gAppID] = [{minVersion: null, maxVersion: null}];
 }
 
 BlocklistItemData.prototype = {
   /**
    * Tests if a version of an item is included in the version range and target
    * application information represented by this BlocklistItemData using the
    * provided application and toolkit versions.
    * @param {string} version
@@ -1469,17 +1474,17 @@ BlocklistItemData.prototype = {
     if (!version && (this.minVersion || this.maxVersion))
       return false;
 
     // Check if the item version matches
     if (!this.matchesRange(version, this.minVersion, this.maxVersion))
       return false;
 
     // Check if the application version matches
-    if (this.matchesTargetRange(gApp.ID, appVersion))
+    if (this.matchesTargetRange(gAppID, appVersion))
       return true;
 
     // Check if the toolkit version matches
     return this.matchesTargetRange(TOOLKIT_ID, toolkitVersion);
   },
 
   /**
    * Checks if a version is higher than or equal to the minVersion (if provided)
@@ -1544,40 +1549,34 @@ BlocklistItemData.prototype = {
         if (versionRangeElement.localName != "versionRange")
           continue;
         appVersions.push(this.getBlocklistVersionRange(versionRangeElement));
       }
     }
     // return minVersion = null and maxVersion = null if no specific versionRange
     // elements were found
     if (appVersions.length == 0)
-      appVersions.push(this.getBlocklistVersionRange(null));
+      appVersions.push({minVersion: null, maxVersion: null});
     return appVersions;
   },
 
   /**
    * Retrieves a version range (e.g. minVersion and maxVersion) for a blocklist
    * versionRange element.
    *
    * @param {Element} versionRangeElement
    *        The versionRange blocklist element.
    *
    * @returns {Object}
    *        A JS object with the following properties:
    *          "minVersion"  The minimum version in a version range (default = null).
    *          "maxVersion"  The maximum version in a version range (default = null).
    */
   getBlocklistVersionRange(versionRangeElement) {
-    var minVersion = null;
-    var maxVersion = null;
-    if (!versionRangeElement)
-      return { minVersion, maxVersion };
-
-    if (versionRangeElement.hasAttribute("minVersion"))
-      minVersion = versionRangeElement.getAttribute("minVersion");
-    if (versionRangeElement.hasAttribute("maxVersion"))
-      maxVersion = versionRangeElement.getAttribute("maxVersion");
+    // getAttribute returns null if the attribute is not present.
+    let minVersion = versionRangeElement.getAttribute("minVersion");
+    let maxVersion = versionRangeElement.getAttribute("maxVersion");
 
     return { minVersion, maxVersion };
   }
 };
 
 Blocklist._init();
--- a/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
+++ b/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
@@ -738,18 +738,23 @@ var AddonTestUtils = {
    *        If provided, the application version is changed to this string
    *        before the AddonManager is started.
    */
   async promiseStartupManager(newVersion) {
     if (this.addonIntegrationService)
       throw new Error("Attempting to startup manager that was already started.");
 
 
-    if (newVersion)
+    if (newVersion) {
       this.appInfo.version = newVersion;
+      if (Cu.isModuleLoaded("resource://gre/modules/Blocklist.jsm")) {
+        let bsPassBlocklist = ChromeUtils.import("resource://gre/modules/Blocklist.jsm", {});
+        Object.defineProperty(bsPassBlocklist, "gAppVersion", {value: newVersion});
+      }
+    }
 
     let XPIScope = ChromeUtils.import("resource://gre/modules/addons/XPIProvider.jsm", null);
     XPIScope.AsyncShutdown = MockAsyncShutdown;
 
     this.addonIntegrationService = Cc["@mozilla.org/addons/integration;1"]
           .getService(Ci.nsIObserver);
 
     this.addonIntegrationService.observe(null, "addons-startup", null);
--- a/toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js
@@ -786,16 +786,18 @@ add_task(async function run_app_update_s
   check_addon(r, "1.0", false, false, Ci.nsIBlocklistService.STATE_NOT_BLOCKED);
 });
 
 add_task(async function update_schema_2() {
   await promiseShutdownManager();
 
   await changeXPIDBVersion(100);
   gAppInfo.version = "2";
+  let bsPassBlocklist = ChromeUtils.import("resource://gre/modules/Blocklist.jsm", {});
+  Object.defineProperty(bsPassBlocklist, "gAppVersion", {value: "2"});
   await promiseStartupManager();
 
   let [s1, s2, s3, s4, h, r] = await promiseAddonsByIDs(ADDON_IDS);
 
   check_addon(s1, "1.0", true, true, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
   check_addon(s2, "1.0", true, true, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
   check_addon(s3, "1.0", true, true, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
   check_addon(s4, "1.0", true, false, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
@@ -810,16 +812,18 @@ add_task(async function update_schema_2(
 });
 
 add_task(async function update_schema_3() {
   await promiseRestartManager();
 
   await promiseShutdownManager();
   await changeXPIDBVersion(100);
   gAppInfo.version = "2.5";
+  let bsPassBlocklist = ChromeUtils.import("resource://gre/modules/Blocklist.jsm", {});
+  Object.defineProperty(bsPassBlocklist, "gAppVersion", {value: "2.5"});
   await promiseStartupManager();
 
   let [s1, s2, s3, s4, h, r] = await promiseAddonsByIDs(ADDON_IDS);
 
   check_addon(s1, "1.0", true, true, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
   check_addon(s2, "1.0", true, false, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
   check_addon(s3, "1.0", false, false, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
   check_addon(s4, "1.0", true, false, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
@@ -843,16 +847,18 @@ add_task(async function update_schema_4(
   check_addon(r, "1.0", false, false, Ci.nsIBlocklistService.STATE_BLOCKED);
 });
 
 add_task(async function update_schema_5() {
   await promiseShutdownManager();
 
   await changeXPIDBVersion(100);
   gAppInfo.version = "1";
+  let bsPassBlocklist = ChromeUtils.import("resource://gre/modules/Blocklist.jsm", {});
+  Object.defineProperty(bsPassBlocklist, "gAppVersion", {value: "1"});
   await promiseStartupManager();
 
   let [s1, s2, s3, s4, h, r] = await promiseAddonsByIDs(ADDON_IDS);
 
   check_addon(s1, "1.0", false, false, Ci.nsIBlocklistService.STATE_NOT_BLOCKED);
   check_addon(s2, "1.0", true, false, Ci.nsIBlocklistService.STATE_NOT_BLOCKED);
   check_addon(s3, "1.0", false, false, Ci.nsIBlocklistService.STATE_NOT_BLOCKED);
   check_addon(s4, "1.0", true, false, Ci.nsIBlocklistService.STATE_NOT_BLOCKED);