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 366634 6fb5f161f09c4a75984b053c022fe36a101a7bcc
parent 366633 f29c03c0682adebf120110a654d22eb5130d7bca
child 366635 5bc308f6ac4c5f428399d975cfadf4039d44827f
push id1369
push userjlorenzo@mozilla.com
push dateMon, 27 Feb 2017 14:59:41 +0000
treeherdermozilla-release@d75a1dba431f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1315575
milestone52.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 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."