Backed out changeset 8f55090db9e6 (bug 1338525) for eslint failure in toolkit/components/extensions/ExtensionUtils.jsm. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Thu, 16 Mar 2017 20:53:09 +0100
changeset 348128 f89deb8fd3cefc493a123f6e47914bf37cb075d4
parent 348127 8f4dfccfd6fec3133e61b54fda977fbaec77951d
child 348129 5d3d03119fa1f7e05642ec1546ff5c711d0824e3
push id88164
push usercbook@mozilla.com
push dateFri, 17 Mar 2017 13:55:35 +0000
treeherdermozilla-inbound@e46c08babe02 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1338525
milestone55.0a1
backs out8f55090db9e6990a651a0d286831397c696a1f70
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
Backed out changeset 8f55090db9e6 (bug 1338525) for eslint failure in toolkit/components/extensions/ExtensionUtils.jsm. r=backout
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/ext-theme.js
toolkit/components/extensions/schemas/theme.json
toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -80,17 +80,16 @@ var {
   apiManager: Management,
 } = ExtensionParent;
 
 const {
   EventEmitter,
   LocaleData,
   StartupCache,
   getUniqueId,
-  validateThemeManifest,
 } = ExtensionUtils;
 
 XPCOMUtils.defineLazyGetter(this, "console", ExtensionUtils.getConsole);
 
 const LOGGER_ID_BASE = "addons.webextension.";
 const UUID_MAP_PREF = "extensions.webextensions.uuids";
 const LEAVE_STORAGE_PREF = "extensions.webextensions.keepStorageOnUninstall";
 const LEAVE_UUID_PREF = "extensions.webextensions.keepUuidOnUninstall";
@@ -425,27 +424,16 @@ this.ExtensionData = class {
 
         logError: error => {
           this.logger.warn(`Loading extension '${this.id}': Reading manifest: ${error}`);
         },
 
         preprocessors: {},
       };
 
-      if (this.manifest.theme) {
-        let invalidProps = validateThemeManifest(Object.getOwnPropertyNames(this.manifest));
-
-        if (invalidProps.length) {
-          let message = `Themes defined in the manifest may only contain static resources. ` +
-            `If you would like to use additional properties, please use the "theme" permission instead. ` +
-            `(the invalid properties found are: ${invalidProps})`;
-          this.manifestError(message);
-        }
-      }
-
       if (this.localeData) {
         context.preprocessors.localize = (value, context) => this.localize(value);
       }
 
       let normalized = Schemas.normalize(this.manifest, "manifest.WebExtensionManifest", context);
       if (normalized.error) {
         this.manifestError(normalized.error);
       } else {
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -856,33 +856,16 @@ function watchExtensionProxyContextLoad(
 
   extension.on("extension-proxy-context-load", listener);
 
   return () => {
     extension.off("extension-proxy-context-load", listener);
   };
 }
 
-// Used to cache the list of WebExtensionManifest properties defined in the BASE_SCHEMA.
-let gBaseManifestProperties = null;
-
 const ExtensionParent = {
   GlobalManager,
   HiddenExtensionPage,
   ParentAPIManager,
   apiManager,
-  get baseManifestProperties() {
-    if (gBaseManifestProperties) {
-      return gBaseManifestProperties;
-    }
-
-    let types = Schemas.schemaJSON.get(BASE_SCHEMA)[0].types;
-    let manifest = types.find(type => type.id === "WebExtensionManifest");
-    if (!manifest) {
-      throw new Error("Unable to find base manifest properties");
-    }
-
-    gBaseManifestProperties = Object.getOwnPropertyNames(manifest.properties);
-    return gBaseManifestProperties;
-  },
   promiseExtensionViewLoaded,
   watchExtensionProxyContextLoad,
 };
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -56,49 +56,16 @@ XPCOMUtils.defineLazyGetter(this, "conso
 
 let nextId = 0;
 XPCOMUtils.defineLazyGetter(this, "uniqueProcessID", () => Services.appinfo.uniqueProcessID);
 
 function getUniqueId() {
   return `${nextId++}-${uniqueProcessID}`;
 }
 
-// The list of properties that themes are allowed to contain.
-XPCOMUtils.defineLazyGetter(this, "gAllowedThemeProperties", () => {
-  Cu.import("resource://gre/modules/ExtensionParent.jsm");
-  let propertiesInBaseManifest = ExtensionParent.baseManifestProperties;
-
-  // The properties found in the base manifest contain all of the properties that
-  // themes are allowed to have. However, the list also contains several properties
-  // that aren't allowed, so we need to filter them out first before the list can
-  // be used to validate themes.
-  return propertiesInBaseManifest.filter(prop => {
-    const propertiesToRemove = ["background", "content_scripts", "permissions"];
-    return !propertiesToRemove.includes(prop);
-  });
-});
-
-/**
- * Validates a theme to ensure it only contains static resources.
- *
- * @param {Array<string> manifestProperties} The list of top-level keys found in the
- *    the extension's manifest.
- * @returns {Array<string>} A list of invalid properties or an empty list
- *    if none are found.
- */
-function validateThemeManifest(manifestProperties) {
-  let invalidProps = [];
-  for (let propName of manifestProperties) {
-    if (propName != "theme" && !gAllowedThemeProperties.includes(propName)) {
-      invalidProps.push(propName);
-    }
-  }
-  return invalidProps;
-}
-
 let StartupCache = {
   DB_NAME: "ExtensionStartupCache",
 
   SCHEMA_VERSION: 1,
 
   STORE_NAMES: Object.freeze(["locales", "manifests", "schemas"]),
 
   dbPromise: null,
@@ -1342,17 +1309,16 @@ this.ExtensionUtils = {
   promiseDocumentReady,
   promiseEvent,
   promiseObserved,
   runSafe,
   runSafeSync,
   runSafeSyncWithoutClone,
   runSafeWithoutClone,
   stylesheetMap,
-  validateThemeManifest,
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,
   ExtensionError,
   IconDetails,
   LimitedSet,
   LocaleData,
   MessageManagerProxy,
--- a/toolkit/components/extensions/ext-theme.js
+++ b/toolkit/components/extensions/ext-theme.js
@@ -2,20 +2,16 @@
 
 Cu.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
                                   "resource://gre/modules/Preferences.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LightweightThemeManager",
                                   "resource://gre/modules/LightweightThemeManager.jsm");
 
-XPCOMUtils.defineLazyGetter(this, "gThemesEnabled", () => {
-  return Preferences.get("extensions.webextensions.themes.enabled");
-});
-
 // WeakMap[Extension -> Theme]
 let themeMap = new WeakMap();
 
 const ICONS = Preferences.get("extensions.webextensions.themes.icons.buttons", "").split(",");
 
 /** Class representing a theme. */
 class Theme {
   /**
@@ -153,17 +149,17 @@ class Theme {
     Services.obs.notifyObservers(null,
       "lightweight-theme-styling-update",
       JSON.stringify(lwtStyles));
   }
 }
 
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("manifest_theme", (type, directive, extension, manifest) => {
-  if (!gThemesEnabled) {
+  if (!Preferences.get("extensions.webextensions.themes.enabled")) {
     // Return early if themes are disabled.
     return;
   }
 
   let theme = new Theme(extension.baseURI);
   theme.load(manifest.theme);
   themeMap.set(extension, theme);
 });
@@ -180,27 +176,20 @@ extensions.on("shutdown", (type, extensi
 });
 /* eslint-enable mozilla/balanced-listeners */
 
 extensions.registerSchemaAPI("theme", "addon_parent", context => {
   let {extension} = context;
   return {
     theme: {
       update(details) {
-        if (!gThemesEnabled) {
-          // Return early if themes are disabled.
-          return;
-        }
-
         let theme = themeMap.get(extension);
 
-        // Themes which use `update` cannot have a theme defined
-        // in the manifest. Therefore, we need to initialize the
-        // theme the first time `update` is called.
+        // We won't have a theme if theme's aren't enabled.
         if (!theme) {
-          themeMap.set(extension, new Theme(extension.baseURI));
+          return;
         }
 
         theme.load(details);
       },
     },
   };
 });
--- a/toolkit/components/extensions/schemas/theme.json
+++ b/toolkit/components/extensions/schemas/theme.json
@@ -2,25 +2,16 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 [
   {
     "namespace": "manifest",
     "types": [
       {
-        "$extend": "Permission",
-        "choices": [{
-          "type": "string",
-          "enum": [
-            "theme"
-          ]
-        }]
-      },
-      {
         "id": "ThemeType",
         "type": "object",
         "properties": {
           "images": {
             "type": "object",
             "optional": true,
             "properties": {
               "headerURL": {
@@ -206,17 +197,17 @@
           }
         }
       }
     ]
   },
   {
     "namespace": "theme",
     "description": "The theme API allows customizing of visual elements of the browser.",
-    "permissions": ["theme"],
+    "permissions": ["manifest:theme"],
     "functions": [
       {
         "name": "update",
         "type": "function",
         "async": true,
         "description": "Make complete or partial updates to the theme. Resolves when the update has completed.",
         "parameters": [
           {
--- a/toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js
+++ b/toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js
@@ -35,44 +35,40 @@ add_task(function* setup() {
   yield SpecialPowers.pushPrefEnv({
     set: [["extensions.webextensions.themes.enabled", true]],
   });
 });
 
 add_task(function* test_dynamic_theme_updates() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
-      permissions: ["theme"],
+      "theme": {
+        "images": {
+          "headerURL": BACKGROUND_1,
+        },
+        "colors": {
+          "accentcolor": ACCENT_COLOR_1,
+          "textcolor": TEXT_COLOR_1,
+        },
+      },
     },
     background() {
       browser.test.onMessage.addListener((msg, details) => {
         if (msg != "update-theme") {
           browser.test.fail("expected 'update-theme' message");
         }
 
         browser.theme.update(details);
         browser.test.sendMessage("theme-updated");
       });
     },
   });
 
   yield extension.startup();
 
-  extension.sendMessage("update-theme", {
-    "images": {
-      "headerURL": BACKGROUND_1,
-    },
-    "colors": {
-      "accentcolor": ACCENT_COLOR_1,
-      "textcolor": TEXT_COLOR_1,
-    },
-  });
-
-  yield extension.awaitMessage("theme-updated");
-
   validateTheme(BACKGROUND_1, ACCENT_COLOR_1, TEXT_COLOR_1);
 
   extension.sendMessage("update-theme", {
     "images": {
       "headerURL": BACKGROUND_2,
     },
     "colors": {
       "accentcolor": ACCENT_COLOR_2,