Bug 1312690: Lazily initialize binding implementation objects. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 30 Oct 2016 20:24:12 -0700
changeset 432814 a91bb22b40fe35baa8088d229d41052d81321a0d
parent 432813 95afa0fbed8c8dd1d9f40a3123392be2a60a872e
child 432815 79b8c08a8879f6ab0516222506e94f7bea4efe92
push id34435
push usermaglione.k@gmail.com
push dateWed, 02 Nov 2016 20:58:46 +0000
reviewersaswan
bugs1312690
milestone52.0a1
Bug 1312690: Lazily initialize binding implementation objects. r?aswan MozReview-Commit-ID: E8K6In6JtWO
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/test/xpcshell/test_ext_schemas_allowed_contexts.js
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -600,23 +600,22 @@ class Entry {
   }
 
   /**
    * 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.
    *
-   * @param {SchemaAPIInterface} apiImpl The implementation of the API.
    * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
    * @param {string} name The method name, e.g. "get".
    * @param {object} dest The object where `path`.`name` should be stored.
    * @param {InjectionContext} context
    */
-  inject(apiImpl, path, name, dest, context) {
+  inject(path, name, dest, context) {
   }
 }
 
 // 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
@@ -930,17 +929,17 @@ class StringType extends Type {
 
     return r;
   }
 
   checkBaseType(baseType) {
     return baseType == "string";
   }
 
-  inject(apiImpl, path, name, dest, context) {
+  inject(path, name, dest, context) {
     if (this.enumeration) {
       exportLazyGetter(dest, name, () => {
         let obj = Cu.createObjectIn(dest);
         for (let e of this.enumeration) {
           obj[e.toUpperCase()] = e;
         }
         return obj;
       });
@@ -1409,17 +1408,17 @@ 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(apiImpl, path, name, dest, context) {
+  inject(path, name, dest, context) {
     dest[name] = this.value;
   }
 }
 
 // Represents a "property" defined in a schema namespace that is not a
 // constant.
 class TypeProperty extends Entry {
   constructor(schema, namespaceName, name, type, writable) {
@@ -1429,21 +1428,23 @@ class TypeProperty extends Entry {
     this.type = type;
     this.writable = writable;
   }
 
   throwError(context, msg) {
     throw context.makeError(`${msg} for ${this.namespaceName}.${this.name}.`);
   }
 
-  inject(apiImpl, path, name, dest, context) {
+  inject(path, name, dest, context) {
     if (this.unsupported) {
       return;
     }
 
+    let apiImpl = context.getImplementation(path.join("."), name);
+
     let getStub = () => {
       this.checkDeprecated(context);
       return apiImpl.getProperty();
     };
 
     let desc = {
       configurable: false,
       enumerable: true,
@@ -1482,17 +1483,17 @@ class SubModuleProperty extends Entry {
   constructor(schema, name, namespaceName, reference, properties) {
     super(schema);
     this.name = name;
     this.namespaceName = namespaceName;
     this.reference = reference;
     this.properties = properties;
   }
 
-  inject(apiImpl, path, name, dest, context) {
+  inject(path, name, dest, context) {
     exportLazyGetter(dest, name, () => {
       let obj = Cu.createObjectIn(dest);
 
       let ns = Schemas.namespaces.get(this.namespaceName);
       let type = ns.get(this.reference);
       if (!type && this.reference.includes(".")) {
         let [namespaceName, ref] = this.reference.split(".");
         ns = Schemas.namespaces.get(namespaceName);
@@ -1503,18 +1504,17 @@ class SubModuleProperty extends Entry {
       }
 
       let functions = type.functions;
       for (let fun of functions) {
         let subpath = path.concat(name);
         let namespace = subpath.join(".");
         let allowedContexts = fun.allowedContexts.length ? fun.allowedContexts : ns.defaultContexts;
         if (context.shouldInject(namespace, fun.name, allowedContexts)) {
-          let apiImpl = context.getImplementation(namespace, fun.name);
-          fun.inject(apiImpl, subpath, fun.name, obj, context);
+          fun.inject(subpath, fun.name, obj, context);
         }
       }
 
       // TODO: Inject this.properties.
 
       return obj;
     });
   }
@@ -1617,26 +1617,28 @@ class FunctionEntry extends CallEntry {
     this.unsupported = unsupported;
     this.returns = returns;
     this.permissions = permissions;
 
     this.isAsync = type.isAsync;
     this.hasAsyncCallback = type.hasAsyncCallback;
   }
 
-  inject(apiImpl, path, name, dest, context) {
+  inject(path, name, dest, context) {
     if (this.unsupported) {
       return;
     }
 
     if (this.permissions && !this.permissions.some(perm => context.hasPermission(perm))) {
       return;
     }
 
     exportLazyGetter(dest, name, () => {
+      let apiImpl = context.getImplementation(path.join("."), 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();
@@ -1679,26 +1681,28 @@ 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(apiImpl, path, name, dest, context) {
+  inject(path, name, dest, context) {
     if (this.unsupported) {
       return;
     }
 
     if (this.permissions && !this.permissions.some(perm => context.hasPermission(perm))) {
       return;
     }
 
     exportLazyGetter(dest, name, () => {
+      let apiImpl = context.getImplementation(path.join("."), name);
+
       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);
@@ -2016,18 +2020,17 @@ this.Schemas = {
 
       for (let [name, entry] of ns) {
         let allowedContexts = entry.allowedContexts;
         if (!allowedContexts.length) {
           allowedContexts = ns.defaultContexts;
         }
 
         if (context.shouldInject(ns.name, name, allowedContexts)) {
-          let apiImpl = context.getImplementation(ns.name, name);
-          entry.inject(apiImpl, [ns.name], name, obj, context);
+          entry.inject([ns.name], name, obj, context);
         }
       }
 
       // Remove the namespace object if it is empty
       if (Object.keys(obj).length) {
         return obj;
       }
     };
--- a/toolkit/components/extensions/test/xpcshell/test_ext_schemas_allowed_contexts.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_schemas_allowed_contexts.js
@@ -68,33 +68,42 @@ let schemaJson = [
 ];
 add_task(function* testRestrictions() {
   let url = "data:," + JSON.stringify(schemaJson);
   yield Schemas.load(url);
   let results = {};
   let localWrapper = {
     shouldInject(ns, name, allowedContexts) {
       name = name === null ? ns : ns + "." + name;
-      results[name] = allowedContexts.join();
+      results[name] = allowedContexts.join(",");
       return true;
     },
     getImplementation() {
       // The actual implementation is not significant for this test.
       // Let's take this opportunity to see if schema generation is free of
       // exceptions even when somehow getImplementation does not return an
       // implementation.
     },
   };
 
   let root = {};
   Schemas.inject(root, localWrapper);
 
   function verify(path, expected) {
+    let obj = root;
+    for (let thing of path.split(".")) {
+      try {
+        obj = obj[thing];
+      } catch (e) {
+        // Blech.
+      }
+    }
+
     let result = results[path];
-    do_check_eq(result, expected);
+    equal(result, expected);
   }
 
   verify("noAllowedContexts", "");
   verify("noAllowedContexts.prop1", "");
   verify("noAllowedContexts.prop2", "test_zero,test_one");
   verify("noAllowedContexts.prop3", "");
   verify("noAllowedContexts.prop4", "numeric_one");
 
@@ -124,15 +133,15 @@ add_task(function* testRestrictions() {
   // Note: test_nine inherits allowed contexts from the namespace, not from
   // submodule. There is no "defaultContexts" for submodule types to not
   // complicate things.
   verify("with_submodule.prop1.noAllowedContexts", "test_nine");
   verify("with_submodule.prop1.allowedContexts", "test_ten");
 
   // This is a constant, so it does not matter that getImplementation does not
   // return an implementation since the API injector should take care of it.
-  do_check_eq(root.noAllowedContexts.prop3, 1);
+  equal(root.noAllowedContexts.prop3, 1);
 
   Assert.throws(() => root.noAllowedContexts.prop1,
                 /undefined/,
                 "Should throw when the implementation is absent.");
 });