Backed out changeset d71c400313a2 (bug 1293132) for failing test_ext_schemas_api_injection.js on Android's Sets test. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Thu, 18 Aug 2016 18:27:34 +0200
changeset 309963 1c6f07b604f671e7a1844e6351e4b7174b58b2ef
parent 309962 fe2f3a5598cca874793c0169a8f3f2bc1bdf1ccb
child 309964 8a70c7469c5025ec9d644c3eb82f60f5982c1edb
push id30575
push userryanvm@gmail.com
push dateFri, 19 Aug 2016 13:46:06 +0000
treeherdermozilla-central@3da4d64410c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1293132
milestone51.0a1
backs outd71c400313a239b1858c73655990b01429fe417a
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
Backed out changeset d71c400313a2 (bug 1293132) for failing test_ext_schemas_api_injection.js on Android's Sets test. r=backout
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/Schemas.jsm
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -658,17 +658,17 @@ GlobalManager = {
         return extension.hasPermission(permission);
       },
 
       callFunction(path, name, args) {
         return findPathInObject(schemaApi, path)[name](...args);
       },
 
       callFunctionNoReturn(path, name, args) {
-        findPathInObject(schemaApi, path)[name](...args);
+        return findPathInObject(schemaApi, path)[name](...args);
       },
 
       callAsyncFunction(path, name, args, callback) {
         // 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.
         if (callback === null) {
           callback = defaultCallback;
@@ -695,20 +695,20 @@ GlobalManager = {
         return findPathInObject(schemaApi, path)[name];
       },
 
       setProperty(path, name, value) {
         findPathInObject(schemaApi, path)[name] = value;
       },
 
       addListener(path, name, listener, args) {
-        findPathInObject(schemaApi, path)[name].addListener.call(null, listener, ...args);
+        return findPathInObject(schemaApi, path)[name].addListener.call(null, listener, ...args);
       },
       removeListener(path, name, listener) {
-        findPathInObject(schemaApi, path)[name].removeListener.call(null, listener);
+        return findPathInObject(schemaApi, path)[name].removeListener.call(null, listener);
       },
       hasListener(path, name, listener) {
         return findPathInObject(schemaApi, path)[name].hasListener.call(null, listener);
       },
     };
     Schemas.inject(dest, schemaWrapper);
   },
 
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -77,62 +77,37 @@ function getValueBaseType(value) {
   } else if (t == "number") {
     if (value % 1 == 0) {
       return "integer";
     }
   }
   return t;
 }
 
-// Methods of Context that are used by Schemas.normalize. These methods can be
-// overridden at the construction of Context.
-const CONTEXT_FOR_VALIDATION = [
-  "checkLoadURL",
-  "hasPermission",
-  "logError",
-];
-
-// Methods of Context that are used by Schemas.inject.
-// Callers of Schemas.inject should implement all of these methods.
-const CONTEXT_FOR_INJECTION = [
-  ...CONTEXT_FOR_VALIDATION,
-  "callFunction",
-  "callFunctionNoReturn",
-  "callAsyncFunction",
-  "getProperty",
-  "setProperty",
-
-  "addListener",
-  "hasListener",
-  "removeListener",
-];
-
-/**
- * A context for schema validation and error reporting. This class is only used
- * internally within Schemas.
- */
 class Context {
-  /**
-   * @param {object} params Provides the implementation of this class.
-   * @param {Array<string>} overridableMethods
-   */
-  constructor(params, overridableMethods = CONTEXT_FOR_VALIDATION) {
+  constructor(params) {
     this.params = params;
 
     this.path = [];
     this.preprocessors = {
       localize(value, context) {
         return value;
       },
     };
 
     this.currentChoices = new Set();
     this.choicePathIndex = 0;
 
-    for (let method of overridableMethods) {
+    let methods = ["addListener", "callFunction",
+                   "callFunctionNoReturn", "callAsyncFunction",
+                   "hasPermission",
+                   "hasListener", "removeListener",
+                   "getProperty", "setProperty",
+                   "checkLoadURL", "logError"];
+    for (let method of methods) {
       if (method in params) {
         this[method] = params[method].bind(params);
       }
     }
 
     let props = ["preprocessors"];
     for (let prop of props) {
       if (prop in params) {
@@ -157,22 +132,16 @@ class Context {
   get url() {
     return this.params.url;
   }
 
   get principal() {
     return this.params.principal || Services.scriptSecurityManager.createNullPrincipal({});
   }
 
-  /**
-   * Checks whether `url` may be loaded by the extension in this context.
-   *
-   * @param {string} url The URL that the extension wished to load.
-   * @returns {boolean} Whether the context may load `url`.
-   */
   checkLoadURL(url) {
     let ssm = Services.scriptSecurityManager;
     try {
       ssm.checkLoadURIStrWithPrincipal(this.principal, url,
                                        ssm.DISALLOW_INHERIT_PRINCIPAL);
     } catch (e) {
       return false;
     }
@@ -324,133 +293,16 @@ class Context {
     try {
       return callback();
     } finally {
       this.path.pop();
     }
   }
 }
 
-/**
- * Holds methods that run the actual implementation of the extension APIs. These
- * methods are only called if the extension API invocation matches the signature
- * as defined in the schema. Otherwise an error is reported to the context.
- */
-class InjectionContext extends Context {
-  constructor(params) {
-    super(params, CONTEXT_FOR_INJECTION);
-  }
-
-  /**
-   * Calls function `path`.`name` and returns its return value.
-   *
-   * @abstract
-   * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {string} name The method name, e.g. "get".
-   * @param {Array} args The parameters for the function.
-   * @returns {*} The return value of the invoked function.
-   */
-  callFunction(path, name, args) {
-    throw new Error("Not implemented");
-  }
-
-  /**
-   * Calls function `path`.`name` and ignores its return value.
-   *
-   * @abstract
-   * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {string} name The method name, e.g. "get".
-   * @param {Array} args The parameters for the function.
-   */
-  callFunctionNoReturn(path, name, args) {
-    throw new Error("Not implemented");
-  }
-
-  /**
-   * Call function `path`.`name` that completes asynchronously.
-   *
-   * @abstract
-   * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {string} name The method name, e.g. "get".
-   * @param {Array} args The parameters for the function.
-   * @param {function(*)} [callback] The callback to be called when the function
-   *     completes.
-   * @returns {Promise|undefined} Must be void if `callback` is set, and a
-   *     promise otherwise. The promise is resolved when the function completes.
-   */
-  callAsyncFunction(path, name, args, callback) {
-    throw new Error("Not implemented");
-  }
-
-  /**
-   * Retrieves the value of property `path`.`name`.
-   *
-   * @abstract
-   * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {string} name The property name.
-   * @returns {*} The value of the property.
-   */
-  getProperty(path, name) {
-    throw new Error("Not implemented");
-  }
-
-  /**
-   * Assigns the value of property `path`.`name`.
-   *
-   * @abstract
-   * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {string} name The property name.
-   * @param {string} value The new value of the property.
-   */
-  setProperty(path, name, value) {
-    throw new Error("Not implemented");
-  }
-
-  /**
-   * Registers `listener` for event `path`.`name`.
-   *
-   * @abstract
-   * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {string} name The event name, e.g. "onChanged"
-   * @param {function} listener The callback to be called when the event fires.
-   * @param {Array} args Extra parameters for EventManager.addListener.
-   * @see EventManager.addListener
-   */
-  addListener(path, name, listener, args) {
-    throw new Error("Not implemented");
-  }
-
-  /**
-   * Checks whether `listener` is listening to event `path`.`name`.
-   *
-   * @abstract
-   * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {string} name The event name, e.g. "onChanged"
-   * @param {function} listener The event listener.
-   * @returns {boolean} Whether `listener` was added to event `path`.`name`.
-   * @see EventManager.hasListener
-   */
-  hasListener(path, name, listener) {
-    throw new Error("Not implemented");
-  }
-
-  /**
-   * Unregisters `listener` from event `path`.`name`.
-   *
-   * @abstract
-   * @param {Array<string>} path The API path, e.g. `["storage", "local"]`.
-   * @param {string} name The event name, e.g. "onChanged"
-   * @param {function} listener The event listener.
-   * @see EventManager.removeListener
-   */
-  removeListener(path, name, listener) {
-    throw new Error("Not implemented");
-  }
-}
-
 
 /**
  * The methods in this singleton represent the "format" specifier for
  * JSON Schema string types.
  *
  * Each method either returns a normalized version of the original
  * value, or throws an error if the value is not valid for the given
  * format.
@@ -601,27 +453,21 @@ class Entry {
    * @param {value} [value]
    */
   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.
-   *
-   * @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
-   */
+  // 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. It's an object with properties
+  // callFunction, addListener, removeListener, and hasListener.
   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 {
   // Takes a value, checks that it has the correct type, and returns a
@@ -1193,28 +1039,28 @@ class SubModuleProperty extends Entry {
   constructor(name, namespaceName, reference, properties) {
     super();
     this.name = name;
     this.namespaceName = namespaceName;
     this.reference = reference;
     this.properties = properties;
   }
 
-  inject(path, name, dest, context) {
+  inject(path, name, dest, wrapperFuncs) {
     let obj = Cu.createObjectIn(dest, {defineAs: name});
 
     let ns = Schemas.namespaces.get(this.namespaceName);
     let type = ns.get(this.reference);
     if (!type || !(type instanceof SubModuleType)) {
       throw new Error(`Internal error: ${this.namespaceName}.${this.reference} is not a sub-module`);
     }
 
     let functions = type.functions;
     for (let fun of functions) {
-      fun.inject(path.concat(name), fun.name, obj, context);
+      fun.inject(path.concat(name), fun.name, obj, wrapperFuncs);
     }
 
     // TODO: Inject this.properties.
   }
 }
 
 // This class is a base class for FunctionEntrys and Events. It takes
 // care of validating parameter lists (i.e., handling of optional
@@ -1378,22 +1224,22 @@ class Event extends CallEntry {
 
     if (this.permissions && !this.permissions.some(perm => context.hasPermission(perm))) {
       return;
     }
 
     let addStub = (listener, ...args) => {
       listener = this.checkListener(listener, context);
       let actuals = this.checkParameters(args, context);
-      context.addListener(this.path, name, listener, actuals);
+      return context.addListener(this.path, name, listener, actuals);
     };
 
     let removeStub = (listener) => {
       listener = this.checkListener(listener, context);
-      context.removeListener(this.path, name, listener);
+      return context.removeListener(this.path, name, listener);
     };
 
     let hasStub = (listener) => {
       listener = this.checkListener(listener, context);
       return context.hasListener(this.path, name, listener);
     };
 
     let obj = Cu.createObjectIn(dest, {defineAs: name});
@@ -1799,26 +1645,18 @@ this.Schemas = {
     let data = Services.ppmm.initialProcessData;
     data["Extension:Schemas"] = this.schemaJSON;
 
     Services.ppmm.broadcastAsyncMessage("Schema:Delete", {url});
 
     this.flushSchemas();
   },
 
-  /**
-   * Inject registered extension APIs into `dest`.
-   *
-   * @param {object} dest The root namespace for the APIs.
-   *     This object is usually exposed to extensions as "chrome" or "browser".
-   * @param {object} wrapperFuncs An implementation of the InjectionContext
-   *     interface, which runs the actual functionality of the generated API.
-   */
   inject(dest, wrapperFuncs) {
-    let context = new InjectionContext(wrapperFuncs);
+    let context = new Context(wrapperFuncs);
 
     for (let [namespace, ns] of this.namespaces) {
       if (ns.permissions && !ns.permissions.some(perm => context.hasPermission(perm))) {
         continue;
       }
 
       let obj = Cu.createObjectIn(dest, {defineAs: namespace});
       for (let [name, entry] of ns) {
@@ -1853,25 +1691,16 @@ this.Schemas = {
         }
 
         // Copy the apiObj as the final nested level.
         currentObj[nsLevels.pop()] = apiObj;
       }
     }
   },
 
-  /**
-   * Normalize `obj` according to the loaded schema for `typeName`.
-   *
-   * @param {object} obj The object to normalize against the schema.
-   * @param {string} typeName The name in the format namespace.propertyname
-   * @param {object} context An implementation of Context. Any validation errors
-   *     are reported to the given context.
-   * @returns {object} The normalized object.
-   */
   normalize(obj, typeName, context) {
     let [namespaceName, prop] = typeName.split(".");
     let ns = this.namespaces.get(namespaceName);
     let type = ns.get(prop);
 
     return type.normalize(obj, new Context(context));
   },
 };