Bug 1338409: Part 2 - Lazily parse the schema data for each namespace property, as it is needed. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 09 Feb 2017 18:59:49 -0800
changeset 481593 080d30c8e3b113fa800436782bd1b4f3c3c11617
parent 481592 504e8335ebcfba6bef87aae6a6bbed79f4eb8702
child 545237 0ea75df3610913f21bd45ab93e9840c53a37910b
push id44862
push usermaglione.k@gmail.com
push dateFri, 10 Feb 2017 03:56:08 +0000
reviewersaswan
bugs1338409
milestone54.0a1
Bug 1338409: Part 2 - Lazily parse the schema data for each namespace property, as it is needed. r?aswan MozReview-Commit-ID: FNQJdt6BnjI
mobile/android/components/extensions/test/mochitest/test_ext_tabs_captureVisibleTab.html
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/test/mochitest/test_ext_contentscript_create_iframe.html
--- a/mobile/android/components/extensions/test/mochitest/test_ext_tabs_captureVisibleTab.html
+++ b/mobile/android/components/extensions/test/mochitest/test_ext_tabs_captureVisibleTab.html
@@ -141,18 +141,18 @@ add_task(function* testCaptureVisibleTab
 
 add_task(function* testCaptureVisibleTabPermissions() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "permissions": ["tabs"],
     },
 
     background() {
-      browser.test.assertFalse("captureVisibleTab" in browser.tabs,
-                               'Extension without "<all_tabs>" permission should not have access to captureVisibleTab');
+      browser.test.assertEq(undefined, browser.tabs.captureVisibleTab,
+                            'Extension without "<all_tabs>" permission should not have access to captureVisibleTab');
       browser.test.notifyPass("captureVisibleTabPermissions");
     },
   });
 
   yield extension.startup();
 
   yield extension.awaitFinish("captureVisibleTabPermissions");
 
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -108,16 +108,76 @@ function exportLazyGetter(object, prop, 
     }, object),
 
     set: Cu.exportFunction(value => {
       redefine(value);
     }, object),
   });
 }
 
+/**
+ * Defines a lazily-instantiated property descriptor on the given
+ * object. Any security wrappers are waived on the object before the
+ * property is defined.
+ *
+ * The given getter function is guaranteed to be called only once, even
+ * if the target scope retrieves the wrapped getter from the property
+ * descriptor and calls it directly.
+ *
+ * @param {object} object
+ *        The object on which to define the getter.
+ * @param {string|Symbol} prop
+ *        The property name for which to define the getter.
+ * @param {function} getter
+ *        The function to call in order to generate the final property
+ *        descriptor object. This will be called, and the property
+ *        descriptor installed on the object, the first time the
+ *        property is written or read. The function may return
+ *        undefined, which will cause the property to be deleted.
+ */
+function exportLazyProperty(object, prop, getter) {
+  object = Cu.waiveXrays(object);
+
+  let redefine = obj => {
+    let desc = getter.call(obj);
+    if (desc === undefined) {
+      delete object[prop];
+    } else {
+      let defaults = {
+        configurable: true,
+        enumerable: true,
+      };
+
+      if (!desc.set && !desc.get) {
+        defaults.writable = true;
+      }
+
+      Object.defineProperty(object, prop,
+                            Object.assign(defaults, desc));
+    }
+
+    getter = null;
+  };
+
+  Object.defineProperty(object, prop, {
+    enumerable: true,
+    configurable: true,
+
+    get: Cu.exportFunction(function() {
+      redefine(this);
+      return object[prop];
+    }, object),
+
+    set: Cu.exportFunction(function(value) {
+      redefine(this);
+      object[prop] = 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);
 
@@ -626,26 +686,26 @@ class Entry {
    */
   checkDeprecated(context, value = null) {
     if (this.deprecated) {
       this.logDeprecation(context, value);
     }
   }
 
   /**
-   * Injects JS values for the entry into the extension API
-   * namespace. The default implementation is to do nothing.
-   * `context` is used to call the actual implementation
-   * of a given function or event.
+   * Returns a property descriptor for use when injecting this entry
+   * into an API object.
    *
    * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {object} dest The object where `path`.`name` should be stored.
    * @param {InjectionContext} context
+   *
+   * @returns {object|undefined}
    */
-  inject(path, dest, context) {
+  getDescriptor(path, context) {
+    return undefined;
   }
 }
 
 // Corresponds either to a type declared in the "types" section of the
 // schema or else to any type object used throughout the schema.
 class Type extends Entry {
   /**
    * @property {Array<string>} EXTRA_PROPERTIES
@@ -966,25 +1026,25 @@ class StringType extends Type {
 
     return r;
   }
 
   checkBaseType(baseType) {
     return baseType == "string";
   }
 
-  inject(path, dest, context) {
+  getDescriptor(path, context) {
     if (this.enumeration) {
-      exportLazyGetter(dest, this.name, () => {
-        let obj = Cu.createObjectIn(dest);
-        for (let e of this.enumeration) {
-          obj[e.toUpperCase()] = e;
-        }
-        return obj;
-      });
+      let obj = Cu.createObjectIn(context.cloneScope);
+
+      for (let e of this.enumeration) {
+        obj[e.toUpperCase()] = e;
+      }
+
+      return {value: obj};
     }
   }
 }
 
 let FunctionEntry;
 let SubModuleType;
 
 class ObjectType extends Type {
@@ -1462,18 +1522,18 @@ class FunctionType extends Type {
 // particular value. Essentially this is a constant.
 class ValueProperty extends Entry {
   constructor(schema, name, value) {
     super(schema);
     this.name = name;
     this.value = value;
   }
 
-  inject(path, dest, context) {
-    dest[this.name] = this.value;
+  getDescriptor(path, context) {
+    return {value: this.value};
   }
 }
 
 // Represents a "property" defined in a schema namespace that is not a
 // constant.
 class TypeProperty extends Entry {
   constructor(schema, path, name, type, writable) {
     super(schema);
@@ -1482,49 +1542,46 @@ class TypeProperty extends Entry {
     this.type = type;
     this.writable = writable;
   }
 
   throwError(context, msg) {
     throw context.makeError(`${msg} for ${this.path.join(".")}.${this.name}.`);
   }
 
-  inject(path, dest, context) {
+  getDescriptor(path, context) {
     if (this.unsupported) {
       return;
     }
 
     let apiImpl = context.getImplementation(path.join("."), this.name);
 
     let getStub = () => {
       this.checkDeprecated(context);
       return apiImpl.getProperty();
     };
 
     let desc = {
-      configurable: false,
-      enumerable: true,
-
-      get: Cu.exportFunction(getStub, dest),
+      get: Cu.exportFunction(getStub, context.cloneScope),
     };
 
     if (this.writable) {
       let setStub = (value) => {
         let normalized = this.type.normalize(value, context);
         if (normalized.error) {
           this.throwError(context, normalized.error);
         }
 
         apiImpl.setProperty(normalized.value);
       };
 
-      desc.set = Cu.exportFunction(setStub, dest);
+      desc.set = Cu.exportFunction(setStub, context.cloneScope);
     }
 
-    Object.defineProperty(dest, this.name, desc);
+    return desc;
   }
 }
 
 class SubModuleProperty extends Entry {
   // A SubModuleProperty represents a tree of objects and properties
   // to expose to an extension. Currently we support only a limited
   // form of sub-module properties, where "$ref" points to a
   // SubModuleType containing a list of functions and "properties" is
@@ -1538,49 +1595,48 @@ class SubModuleProperty extends Entry {
     super(schema);
     this.name = name;
     this.path = path;
     this.namespaceName = path.join(".");
     this.reference = reference;
     this.properties = properties;
   }
 
-  inject(path, dest, context) {
-    exportLazyGetter(dest, this.name, () => {
-      let obj = Cu.createObjectIn(dest);
+  getDescriptor(path, context) {
+    let obj = Cu.createObjectIn(context.cloneScope);
 
-      let ns = Schemas.getNamespace(this.namespaceName);
-      let type = ns.get(this.reference);
-      if (!type && this.reference.includes(".")) {
-        let [namespaceName, ref] = this.reference.split(".");
-        ns = Schemas.getNamespace(namespaceName);
-        type = ns.get(ref);
-      }
+    let ns = Schemas.getNamespace(this.namespaceName);
+    let type = ns.get(this.reference);
+    if (!type && this.reference.includes(".")) {
+      let [namespaceName, ref] = this.reference.split(".");
+      ns = Schemas.getNamespace(namespaceName);
+      type = ns.get(ref);
+    }
 
-      if (DEBUG) {
-        if (!type || !(type instanceof SubModuleType)) {
-          throw new Error(`Internal error: ${this.namespaceName}.${this.reference} ` +
-                          `is not a sub-module`);
-        }
+    if (DEBUG) {
+      if (!type || !(type instanceof SubModuleType)) {
+        throw new Error(`Internal error: ${this.namespaceName}.${this.reference} ` +
+                        `is not a sub-module`);
       }
-      let subpath = [path, this.name];
-      let namespace = subpath.join(".");
+    }
+    let subpath = [path, this.name];
+    let namespace = subpath.join(".");
 
-      let functions = type.functions;
-      for (let fun of functions) {
-        let allowedContexts = fun.allowedContexts.length ? fun.allowedContexts : ns.defaultContexts;
-        if (context.shouldInject(namespace, fun.name, allowedContexts)) {
-          fun.inject(subpath, obj, context);
-        }
+    let functions = type.functions;
+    for (let fun of functions) {
+      let allowedContexts = fun.allowedContexts.length ? fun.allowedContexts : ns.defaultContexts;
+      if (context.shouldInject(namespace, fun.name, allowedContexts)) {
+        exportLazyProperty(obj, fun.name,
+                           () => fun.getDescriptor(subpath, context));
       }
-
-      // TODO: Inject this.properties.
+    }
 
-      return obj;
-    });
+    // TODO: Inject this.properties.
+
+    return {value: obj};
   }
 }
 
 // This class is a base class for FunctionEntrys and Events. It takes
 // care of validating parameter lists (i.e., handling of optional
 // parameters and parameter type checking).
 class CallEntry extends Entry {
   constructor(schema, path, name, parameters, allowAmbiguousOptionalArguments) {
@@ -1688,60 +1744,59 @@ FunctionEntry = class FunctionEntry exte
     this.unsupported = unsupported;
     this.returns = returns;
     this.permissions = permissions;
 
     this.isAsync = type.isAsync;
     this.hasAsyncCallback = type.hasAsyncCallback;
   }
 
-  inject(path, dest, context) {
+  getDescriptor(path, context) {
     if (this.unsupported) {
       return;
     }
 
     if (this.permissions && !this.permissions.some(perm => context.hasPermission(perm))) {
       return;
     }
 
-    exportLazyGetter(dest, this.name, () => {
-      let apiImpl = context.getImplementation(path.join("."), this.name);
+    let apiImpl = context.getImplementation(path.join("."), this.name);
 
-      let stub;
-      if (this.isAsync) {
-        stub = (...args) => {
-          this.checkDeprecated(context);
-          let actuals = this.checkParameters(args, context);
-          let callback = null;
-          if (this.hasAsyncCallback) {
-            callback = actuals.pop();
-          }
-          if (callback === null && context.isChromeCompat) {
-            // We pass an empty stub function as a default callback for
-            // the `chrome` API, so promise objects are not returned,
-            // and lastError values are reported immediately.
-            callback = () => {};
-          }
-          return apiImpl.callAsyncFunction(actuals, callback);
-        };
-      } else if (!this.returns) {
-        stub = (...args) => {
-          this.checkDeprecated(context);
-          let actuals = this.checkParameters(args, context);
-          return apiImpl.callFunctionNoReturn(actuals);
-        };
-      } else {
-        stub = (...args) => {
-          this.checkDeprecated(context);
-          let actuals = this.checkParameters(args, context);
-          return apiImpl.callFunction(actuals);
-        };
-      }
-      return Cu.exportFunction(stub, dest);
-    });
+    let stub;
+    if (this.isAsync) {
+      stub = (...args) => {
+        this.checkDeprecated(context);
+        let actuals = this.checkParameters(args, context);
+        let callback = null;
+        if (this.hasAsyncCallback) {
+          callback = actuals.pop();
+        }
+        if (callback === null && context.isChromeCompat) {
+          // We pass an empty stub function as a default callback for
+          // the `chrome` API, so promise objects are not returned,
+          // and lastError values are reported immediately.
+          callback = () => {};
+        }
+        return apiImpl.callAsyncFunction(actuals, callback);
+      };
+    } else if (!this.returns) {
+      stub = (...args) => {
+        this.checkDeprecated(context);
+        let actuals = this.checkParameters(args, context);
+        return apiImpl.callFunctionNoReturn(actuals);
+      };
+    } else {
+      stub = (...args) => {
+        this.checkDeprecated(context);
+        let actuals = this.checkParameters(args, context);
+        return apiImpl.callFunction(actuals);
+      };
+    }
+
+    return {value: Cu.exportFunction(stub, context.cloneScope)};
   }
 };
 
 // Represents an "event" defined in a schema namespace.
 class Event extends CallEntry {
   static parseSchema(event, path) {
     let extraParameters = Array.from(event.extraParameters || [], param => ({
       type: Schemas.parseSchema(param, path, ["name", "optional", "default"]),
@@ -1771,52 +1826,50 @@ class Event extends CallEntry {
   checkListener(listener, context) {
     let r = this.type.normalize(listener, context);
     if (r.error) {
       this.throwError(context, "Invalid listener");
     }
     return r.value;
   }
 
-  inject(path, dest, context) {
+  getDescriptor(path, context) {
     if (this.unsupported) {
       return;
     }
 
     if (this.permissions && !this.permissions.some(perm => context.hasPermission(perm))) {
       return;
     }
 
-    exportLazyGetter(dest, this.name, () => {
-      let apiImpl = context.getImplementation(path.join("."), this.name);
+    let apiImpl = context.getImplementation(path.join("."), this.name);
 
-      let addStub = (listener, ...args) => {
-        listener = this.checkListener(listener, context);
-        let actuals = this.checkParameters(args, context);
-        apiImpl.addListener(listener, actuals);
-      };
+    let addStub = (listener, ...args) => {
+      listener = this.checkListener(listener, context);
+      let actuals = this.checkParameters(args, context);
+      apiImpl.addListener(listener, actuals);
+    };
 
-      let removeStub = (listener) => {
-        listener = this.checkListener(listener, context);
-        apiImpl.removeListener(listener);
-      };
+    let removeStub = (listener) => {
+      listener = this.checkListener(listener, context);
+      apiImpl.removeListener(listener);
+    };
 
-      let hasStub = (listener) => {
-        listener = this.checkListener(listener, context);
-        return apiImpl.hasListener(listener);
-      };
-
-      let obj = Cu.createObjectIn(dest);
+    let hasStub = (listener) => {
+      listener = this.checkListener(listener, context);
+      return apiImpl.hasListener(listener);
+    };
 
-      Cu.exportFunction(addStub, obj, {defineAs: "addListener"});
-      Cu.exportFunction(removeStub, obj, {defineAs: "removeListener"});
-      Cu.exportFunction(hasStub, obj, {defineAs: "hasListener"});
+    let obj = Cu.createObjectIn(context.cloneScope);
 
-      return obj;
-    });
+    Cu.exportFunction(addStub, obj, {defineAs: "addListener"});
+    Cu.exportFunction(removeStub, obj, {defineAs: "removeListener"});
+    Cu.exportFunction(hasStub, obj, {defineAs: "hasListener"});
+
+    return {value: obj};
   }
 }
 
 const TYPES = Object.freeze(Object.assign(Object.create(null), {
   any: AnyType,
   array: ArrayType,
   boolean: BooleanType,
   function: FunctionType,
@@ -2002,43 +2055,45 @@ class Namespace extends Map {
    * Injects the properties of this namespace into the given object.
    *
    * @param {object} dest
    *        The object into which to inject the namespace properties.
    * @param {InjectionContext} context
    *        The injection context with which to inject the properties.
    */
   injectInto(dest, context) {
-    for (let [name, entry] of this.entries()) {
-      if (entry.permissions && !entry.permissions.some(perm => context.hasPermission(perm))) {
-        continue;
-      }
+    for (let name of this.keys()) {
+      exportLazyProperty(dest, name, () => {
+        let entry = this.get(name);
+
+        if (entry.permissions && !entry.permissions.some(perm => context.hasPermission(perm))) {
+          return;
+        }
 
-      let allowedContexts = entry.allowedContexts;
-      if (!allowedContexts.length) {
-        allowedContexts = this.defaultContexts;
-      }
+        let allowedContexts = entry.allowedContexts;
+        if (!allowedContexts.length) {
+          allowedContexts = this.defaultContexts;
+        }
 
-      if (context.shouldInject(this.path.join("."), name, allowedContexts)) {
-        entry.inject(this.path, dest, context);
-      }
+        if (context.shouldInject(this.path.join("."), name, allowedContexts)) {
+          return entry.getDescriptor(this.path, context);
+        }
+      });
     }
   }
 
-  inject(path, dest, context) {
-    exportLazyGetter(dest, this.name, () => {
-      let obj = Cu.createObjectIn(dest);
+  getDescriptor(path, context) {
+    let obj = Cu.createObjectIn(context.cloneScope);
 
-      this.injectInto(obj, context);
+    this.injectInto(obj, context);
 
-      // Only inject the namespace object if it isn't empty.
-      if (Object.keys(obj).length) {
-        return obj;
-      }
-    });
+    // Only inject the namespace object if it isn't empty.
+    if (Object.keys(obj).length) {
+      return {value: obj};
+    }
   }
 
   keys() {
     this.init();
     return super.keys();
   }
 
   * entries() {
--- a/toolkit/components/extensions/test/mochitest/test_ext_contentscript_create_iframe.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_contentscript_create_iframe.html
@@ -66,17 +66,17 @@ add_task(function* test_contentscript_cr
       } catch (exception) {
         return String(exception);
       }
     };
 
     let testGetManifest = window.GET_MANIFEST();
 
     let manifest = browser.runtime.getManifest();
-    let availableAPIs = Object.keys(browser);
+    let availableAPIs = Object.keys(browser).filter(key => browser[key]);
 
     browser.runtime.sendMessage({
       name: "content-script-iframe-loaded",
       availableAPIs,
       manifest,
       testGetManifest,
     });
   }