Bug 1338525 - Add schema validation for webextension themes r=mikedeboer,mossop

MozReview-Commit-ID: 3QjDrTeMKH0

--HG--
extra : rebase_source : e487b3a12d3645de5f846305ae7b99532d15dcc5

toolkit/components/extensions/Extension.jsm

@@ -85,6 +85,7 @@ const {
   LocaleData,
   StartupCache,
   getUniqueId,
+  validateThemeManifest,
 } = ExtensionUtils;
 
 XPCOMUtils.defineLazyGetter(this, "console", ExtensionUtils.getConsole);
@@ -429,6 +430,17 @@ this.ExtensionData = class {
         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);
       }

toolkit/components/extensions/ExtensionParent.jsm

@@ -861,11 +861,28 @@ function watchExtensionProxyContextLoad({extension, viewType, browser}, onExtens
   };
 }
 
+// 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,
 };

toolkit/components/extensions/ExtensionUtils.jsm

@@ -61,6 +61,39 @@ 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",
 
@@ -1314,6 +1347,7 @@ this.ExtensionUtils = {
   runSafeSyncWithoutClone,
   runSafeWithoutClone,
   stylesheetMap,
+  validateThemeManifest,
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,

toolkit/components/extensions/ext-theme.js

@@ -7,6 +7,10 @@ XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
 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();
 
@@ -154,7 +158,7 @@ class Theme {
 
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("manifest_theme", (type, directive, extension, manifest) => {
-  if (!Preferences.get("extensions.webextensions.themes.enabled")) {
+  if (!gThemesEnabled) {
     // Return early if themes are disabled.
     return;
   }
@@ -181,11 +185,18 @@ extensions.registerSchemaAPI("theme", "addon_parent", context => {
   return {
     theme: {
       update(details) {
+        if (!gThemesEnabled) {
+          // Return early if themes are disabled.
+          return;
+        }
+
         let theme = themeMap.get(extension);
 
-        // We won't have a theme if theme's aren't enabled.
+        // 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.
         if (!theme) {
-          return;
+          themeMap.set(extension, new Theme(extension.baseURI));
         }
 
         theme.load(details);

toolkit/components/extensions/schemas/theme.json

@@ -6,6 +6,15 @@
   {
     "namespace": "manifest",
     "types": [
+      {
+        "$extend": "Permission",
+        "choices": [{
+          "type": "string",
+          "enum": [
+            "theme"
+          ]
+        }]
+      },
       {
         "id": "ThemeType",
         "type": "object",
@@ -202,7 +211,7 @@
   {
     "namespace": "theme",
     "description": "The theme API allows customizing of visual elements of the browser.",
-    "permissions": ["manifest:theme"],
+    "permissions": ["theme"],
     "functions": [
       {
         "name": "update",

toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js

@@ -40,15 +40,7 @@ add_task(function* setup() {
 add_task(function* test_dynamic_theme_updates() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
-      "theme": {
-        "images": {
-          "headerURL": BACKGROUND_1,
-        },
-        "colors": {
-          "accentcolor": ACCENT_COLOR_1,
-          "textcolor": TEXT_COLOR_1,
-        },
-      },
+      permissions: ["theme"],
     },
     background() {
       browser.test.onMessage.addListener((msg, details) => {
@@ -64,6 +56,18 @@ add_task(function* test_dynamic_theme_updates() {
 
   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", {