Bug 1315575: Part 3 - Convert ImageData objects at the binding layer, and remove unnecessary content bindings. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Sun, 06 Nov 2016 17:35:07 -0800
changeset 321560 6fb5f161f09c4a75984b053c022fe36a101a7bcc
parent 321559 f29c03c0682adebf120110a654d22eb5130d7bca
child 321561 5bc308f6ac4c5f428399d975cfadf4039d44827f
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewersaswan
bugs1315575
milestone52.0a1
Bug 1315575: Part 3 - Convert ImageData objects at the binding layer, and remove unnecessary content bindings. r=aswan MozReview-Commit-ID: CjqXRiFcMWp
browser/components/extensions/ext-browserAction.js
browser/components/extensions/ext-c-browserAction.js
browser/components/extensions/ext-c-pageAction.js
browser/components/extensions/ext-pageAction.js
browser/components/extensions/extensions-browser.manifest
browser/components/extensions/jar.mn
browser/components/extensions/schemas/browser_action.json
browser/components/extensions/schemas/page_action.json
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/schemas/manifest.json
--- a/browser/components/extensions/ext-browserAction.js
+++ b/browser/components/extensions/ext-browserAction.js
@@ -466,19 +466,16 @@ extensions.registerSchemaAPI("browserAct
 
         let title = BrowserAction.for(extension).getProperty(tab, "title");
         return Promise.resolve(title);
       },
 
       setIcon: function(details) {
         let tab = details.tabId !== null ? TabManager.getTab(details.tabId, context) : null;
 
-        // Note: the caller in the child process has already normalized
-        // `details` to not contain an `imageData` property, so the icon can
-        // safely be normalized here without errors.
         let icon = IconDetails.normalize(details, extension, context);
         BrowserAction.for(extension).setProperty(tab, "icon", icon);
       },
 
       setBadgeText: function(details) {
         let tab = details.tabId !== null ? TabManager.getTab(details.tabId, context) : null;
 
         BrowserAction.for(extension).setProperty(tab, "badgeText", details.text);
deleted file mode 100644
--- a/browser/components/extensions/ext-c-browserAction.js
+++ /dev/null
@@ -1,26 +0,0 @@
-/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim: set sts=2 sw=2 et tw=80: */
-"use strict";
-
-Cu.import("resource://gre/modules/ExtensionUtils.jsm");
-var {
-  IconDetails,
-} = ExtensionUtils;
-
-extensions.registerSchemaAPI("browserAction", "addon_child", context => {
-  return {
-    browserAction: {
-      setIcon: function(details) {
-        // This needs to run in the addon process because normalization requires
-        // the use of <canvas>.
-        let normalizedDetails = {
-          tabId: details.tabId,
-          path: IconDetails.normalize(details, context.extension, context),
-        };
-        return context.childManager.callParentAsyncFunction("browserAction.setIcon", [
-          normalizedDetails,
-        ]);
-      },
-    },
-  };
-});
deleted file mode 100644
--- a/browser/components/extensions/ext-c-pageAction.js
+++ /dev/null
@@ -1,26 +0,0 @@
-/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim: set sts=2 sw=2 et tw=80: */
-"use strict";
-
-Cu.import("resource://gre/modules/ExtensionUtils.jsm");
-var {
-  IconDetails,
-} = ExtensionUtils;
-
-extensions.registerSchemaAPI("pageAction", "addon_child", context => {
-  return {
-    pageAction: {
-      setIcon: function(details) {
-        // This needs to run in the addon process because normalization requires
-        // the use of <canvas>.
-        let normalizedDetails = {
-          tabId: details.tabId,
-          path: IconDetails.normalize(details, context.extension, context),
-        };
-        return context.childManager.callParentAsyncFunction("pageAction.setIcon", [
-          normalizedDetails,
-        ]);
-      },
-    },
-  };
-});
--- a/browser/components/extensions/ext-pageAction.js
+++ b/browser/components/extensions/ext-pageAction.js
@@ -255,19 +255,16 @@ extensions.registerSchemaAPI("pageAction
 
         let title = PageAction.for(extension).getProperty(tab, "title");
         return Promise.resolve(title);
       },
 
       setIcon(details) {
         let tab = TabManager.getTab(details.tabId, context);
 
-        // Note: the caller in the child process has already normalized
-        // `details` to not contain an `imageData` property, so the icon can
-        // safely be normalized here without errors.
         let icon = IconDetails.normalize(details, extension, context);
         PageAction.for(extension).setProperty(tab, "icon", icon);
       },
 
       setPopup(details) {
         let tab = TabManager.getTab(details.tabId, context);
 
         // Note: Chrome resolves arguments to setIcon relative to the calling
--- a/browser/components/extensions/extensions-browser.manifest
+++ b/browser/components/extensions/extensions-browser.manifest
@@ -7,19 +7,17 @@ category webextension-scripts desktop-ru
 category webextension-scripts history chrome://browser/content/ext-history.js
 category webextension-scripts pageAction chrome://browser/content/ext-pageAction.js
 category webextension-scripts sessions chrome://browser/content/ext-sessions.js
 category webextension-scripts tabs chrome://browser/content/ext-tabs.js
 category webextension-scripts utils chrome://browser/content/ext-utils.js
 category webextension-scripts windows chrome://browser/content/ext-windows.js
 
 # scripts that must run in the same process as addon code.
-category webextension-scripts-addon browserAction chrome://browser/content/ext-c-browserAction.js
 category webextension-scripts-addon contextMenus chrome://browser/content/ext-c-contextMenus.js
-category webextension-scripts-addon pageAction chrome://browser/content/ext-c-pageAction.js
 category webextension-scripts-addon tabs chrome://browser/content/ext-c-tabs.js
 
 # schemas
 category webextension-schemas bookmarks chrome://browser/content/schemas/bookmarks.json
 category webextension-schemas browser_action chrome://browser/content/schemas/browser_action.json
 category webextension-schemas commands chrome://browser/content/schemas/commands.json
 category webextension-schemas context_menus chrome://browser/content/schemas/context_menus.json
 category webextension-schemas context_menus_internal chrome://browser/content/schemas/context_menus_internal.json
--- a/browser/components/extensions/jar.mn
+++ b/browser/components/extensions/jar.mn
@@ -18,12 +18,10 @@ browser.jar:
     content/browser/ext-contextMenus.js
     content/browser/ext-desktop-runtime.js
     content/browser/ext-history.js
     content/browser/ext-pageAction.js
     content/browser/ext-sessions.js
     content/browser/ext-tabs.js
     content/browser/ext-utils.js
     content/browser/ext-windows.js
-    content/browser/ext-c-browserAction.js
     content/browser/ext-c-contextMenus.js
-    content/browser/ext-c-pageAction.js
     content/browser/ext-c-tabs.js
--- a/browser/components/extensions/schemas/browser_action.json
+++ b/browser/components/extensions/schemas/browser_action.json
@@ -55,16 +55,17 @@
         "minItems": 4,
         "maxItems": 4
       },
       {
         "id": "ImageDataType",
         "type": "object",
         "isInstanceOf": "ImageData",
         "additionalProperties": { "type": "any" },
+        "postprocess": "convertImageDataToURL",
         "description": "Pixel data for an image. Must be an ImageData object (for example, from a <code>canvas</code> element)."
       }
     ],
     "functions": [
       {
         "name": "setTitle",
         "type": "function",
         "description": "Sets the title of the browser action. This shows up in the tooltip.",
--- a/browser/components/extensions/schemas/page_action.json
+++ b/browser/components/extensions/schemas/page_action.json
@@ -44,16 +44,17 @@
     "description": "Use the <code>browser.pageAction</code> API to put icons inside the address bar. Page actions represent actions that can be taken on the current page, but that aren't applicable to all pages.",
     "permissions": ["manifest:page_action"],
     "types": [
       {
         "id": "ImageDataType",
         "type": "object",
         "isInstanceOf": "ImageData",
         "additionalProperties": { "type": "any" },
+        "postprocess": "convertImageDataToURL",
         "description": "Pixel data for an image. Must be an ImageData object (for example, from a <code>canvas</code> element)."
       }
     ],
     "functions": [
       {
         "name": "show",
         "type": "function",
         "async": "callback",
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -525,30 +525,25 @@ let IconDetails = {
   // function simply logs a warning message.
   normalize(details, extension, context = null) {
     let result = {};
 
     try {
       if (details.imageData) {
         let imageData = details.imageData;
 
-        // The global might actually be from Schema.jsm, which
-        // normalizes most of our arguments. In that case it won't have
-        // an ImageData property. But Schema.jsm doesn't normalize
-        // actual ImageData objects, so they will come from a global
-        // with the right property.
-        if (instanceOf(imageData, "ImageData")) {
+        if (typeof imageData == "string") {
           imageData = {"19": imageData};
         }
 
         for (let size of Object.keys(imageData)) {
           if (!INTEGER.test(size)) {
             throw new Error(`Invalid icon size ${size}, must be an integer`);
           }
-          result[size] = this.convertImageDataToDataURL(imageData[size], context);
+          result[size] = imageData[size];
         }
       }
 
       if (details.path) {
         let path = details.path;
         if (typeof path != "object") {
           path = {"19": path};
         }
@@ -638,26 +633,16 @@ let IconDetails = {
 
         ctx.drawImage(this, 0, 0, this.width, this.height, dx, dy, dWidth, dHeight);
         resolve(canvas.toDataURL("image/png"));
       };
       image.onerror = reject;
       image.src = imageURL;
     });
   },
-
-  convertImageDataToDataURL(imageData, context) {
-    let document = context.contentWindow.document;
-    let canvas = document.createElementNS("http://www.w3.org/1999/xhtml", "canvas");
-    canvas.width = imageData.width;
-    canvas.height = imageData.height;
-    canvas.getContext("2d").putImageData(imageData, 0, 0);
-
-    return canvas.toDataURL("image/png");
-  },
 };
 
 const LISTENERS = Symbol("listeners");
 
 class EventEmitter {
   constructor() {
     this[LISTENERS] = new Map();
   }
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -115,16 +115,28 @@ function exportLazyGetter(object, prop, 
     }, object),
 
     set: Cu.exportFunction(value => {
       redefine(value);
     }, object),
   });
 }
 
+const POSTPROCESSORS = {
+  convertImageDataToURL(imageData, context) {
+    let document = context.cloneScope.document;
+    let canvas = document.createElementNS("http://www.w3.org/1999/xhtml", "canvas");
+    canvas.width = imageData.width;
+    canvas.height = imageData.height;
+    canvas.getContext("2d").putImageData(imageData, 0, 0);
+
+    return canvas.toDataURL("image/png");
+  },
+};
+
 // Parses a regular expression, with support for the Python extended
 // syntax that allows setting flags by including the string (?im)
 function parsePattern(pattern) {
   let flags = "";
   let match = /^\(\?([im]*)\)(.*)/.exec(pattern);
   if (match) {
     [, flags, pattern] = match;
   }
@@ -178,16 +190,17 @@ class Context {
     this.params = params;
 
     this.path = [];
     this.preprocessors = {
       localize(value, context) {
         return value;
       },
     };
+    this.postprocessors = POSTPROCESSORS;
     this.isChromeCompat = false;
 
     this.currentChoices = new Set();
     this.choicePathIndex = 0;
 
     for (let method of overridableMethods) {
       if (method in params) {
         this[method] = params[method].bind(params);
@@ -535,16 +548,24 @@ class Entry {
      * @property {string} [preprocessor]
      * If set to a string value, and a preprocessor of the same is
      * defined in the validation context, it will be applied to this
      * value prior to any normalization.
      */
     this.preprocessor = schema.preprocess || null;
 
     /**
+     * @property {string} [postprocessor]
+     * If set to a string value, and a postprocessor of the same is
+     * defined in the validation context, it will be applied to this
+     * value after any normalization.
+     */
+    this.postprocessor = schema.postprocess || null;
+
+    /**
      * @property {Array<string>} allowedContexts A list of allowed contexts
      * to consider before generating the API.
      * These are not parsed by the schema, but passed to `shouldInject`.
      */
     this.allowedContexts = schema.allowedContexts || [];
   }
 
   /**
@@ -558,16 +579,33 @@ class Entry {
   preprocess(value, context) {
     if (this.preprocessor) {
       return context.preprocessors[this.preprocessor](value, context);
     }
     return value;
   }
 
   /**
+   * Postprocess the given result with the postprocessor declared in
+   * `postprocessor`.
+   *
+   * @param {object} result
+   * @param {Context} context
+   * @returns {object}
+   */
+  postprocess(result, context) {
+    if (result.error || !this.postprocessor) {
+      return result;
+    }
+
+    let value = context.postprocessors[this.postprocessor](result.value, context);
+    return {value};
+  }
+
+  /**
    * Logs a deprecation warning for this entry, based on the value of
    * its `deprecated` property.
    *
    * @param {Context} context
    * @param {value} [value]
    */
   logDeprecation(context, value = null) {
     let message = "This property is deprecated";
@@ -618,17 +656,17 @@ class Entry {
 // schema or else to any type object used throughout the schema.
 class Type extends Entry {
   /**
    * @property {Array<string>} EXTRA_PROPERTIES
    *        An array of extra properties which may be present for
    *        schemas of this type.
    */
   static get EXTRA_PROPERTIES() {
-    return ["description", "deprecated", "preprocess", "allowedContexts"];
+    return ["description", "deprecated", "preprocess", "postprocess", "allowedContexts"];
   }
 
   /**
    * Parses the given schema object and returns an instance of this
    * class which corresponds to its properties.
    *
    * @param {object} schema
    *        A JSON schema object which corresponds to a definition of
@@ -717,17 +755,17 @@ class Type extends Entry {
                          choice);
   }
 }
 
 // Type that allows any value.
 class AnyType extends Type {
   normalize(value, context) {
     this.checkDeprecated(context, value);
-    return {value};
+    return this.postprocess({value}, context);
   }
 
   checkBaseType(baseType) {
     return true;
   }
 }
 
 // An untagged union type.
@@ -891,17 +929,17 @@ class StringType extends Type {
     let r = this.normalizeBase("string", value, context);
     if (r.error) {
       return r;
     }
     value = r.value;
 
     if (this.enumeration) {
       if (this.enumeration.includes(value)) {
-        return {value};
+        return this.postprocess({value}, context);
       }
 
       let choices = this.enumeration.map(JSON.stringify).join(", ");
 
       return context.error(`Invalid enumeration value ${JSON.stringify(value)}`,
                            `be one of [${choices}]`);
     }
 
@@ -1131,17 +1169,17 @@ class ObjectType extends Type {
 
         if (!instanceOf(value, this.isInstanceOf)) {
           return context.error(`Object must be an instance of ${this.isInstanceOf}`,
                                `be an instance of ${this.isInstanceOf}`);
         }
 
         // This is kind of a hack, but we can't normalize things that
         // aren't JSON, so we just return them.
-        return {value};
+        return this.postprocess({value}, context);
       }
 
       let properties = this.extractProperties(value, context);
       let remainingProps = new Set(Object.keys(properties));
 
       let result = {};
       for (let prop of Object.keys(this.properties)) {
         this.checkProperty(context, prop, this.properties[prop], result,
@@ -1170,17 +1208,17 @@ class ObjectType extends Type {
         return context.error(`Unexpected property "${[...remainingProps]}"`,
                              `not contain an unexpected "${[...remainingProps]}" property`);
       } else if (remainingProps.size) {
         let props = [...remainingProps].sort().join(", ");
         return context.error(`Unexpected properties: ${props}`,
                              `not contain the unexpected properties [${props}]`);
       }
 
-      return {value: result};
+      return this.postprocess({value: result}, context);
     } catch (e) {
       if (e.error) {
         return e;
       }
       throw e;
     }
   }
 }
@@ -1262,17 +1300,17 @@ class IntegerType extends Type {
       return context.error(`Integer ${value} is too small (must be at least ${this.minimum})`,
                            `be at least ${this.minimum}`);
     }
     if (value > this.maximum) {
       return context.error(`Integer ${value} is too big (must be at most ${this.maximum})`,
                            `be no greater than ${this.maximum}`);
     }
 
-    return r;
+    return this.postprocess(r, context);
   }
 
   checkBaseType(baseType) {
     return baseType == "integer";
   }
 }
 
 class BooleanType extends Type {
@@ -1326,17 +1364,17 @@ class ArrayType extends Type {
                            `have at least ${this.minItems} items`);
     }
 
     if (result.length > this.maxItems) {
       return context.error(`Array requires at most ${this.maxItems} items; you have ${result.length}`,
                            `have at most ${this.maxItems} items`);
     }
 
-    return {value: result};
+    return this.postprocess({value: result}, context);
   }
 
   checkBaseType(baseType) {
     return baseType == "array";
   }
 }
 
 class FunctionType extends Type {
--- a/toolkit/components/extensions/schemas/manifest.json
+++ b/toolkit/components/extensions/schemas/manifest.json
@@ -344,30 +344,30 @@
         ]
       },
       {
         "id": "IconImageData",
         "choices": [
           {
             "type": "object",
             "patternProperties": {
-              "^[1-9]\\d*$": {
-                "type": "object",
-                "isInstanceOf": "ImageData"
-              }
+              "^[1-9]\\d*$": { "$ref": "ImageData" }
             },
             "additionalProperties": false
           },
-          {
-            "type": "object",
-            "isInstanceOf": "ImageData"
-          }
+          { "$ref": "ImageData" }
         ]
       },
       {
+        "id": "ImageData",
+        "type": "object",
+        "isInstanceOf": "ImageData",
+        "postprocess": "convertImageDataToURL"
+      },
+      {
         "id": "UnrecognizedProperty",
         "type": "any",
         "deprecated": "An unexpected property was found in the WebExtension manifest."
       },
       {
         "id": "PersistentBackgroundProperty",
         "type": "boolean",
         "deprecated": "Event pages are not currently supported. This will run as a persistent background page."