Bug 1316396: Part 4 - Refactor ChildAPIManager and sub-classes. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Thu, 10 Nov 2016 12:29:38 -0800
changeset 348787 d5fa01b12572b777103c15f55ef09281215f917e
parent 348786 1731b48caa40c1753400d6867a6b2fb99dafd60d
child 348788 e2aa5d3af1089e05650634b8f24e675ae99a902f
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1316396
milestone52.0a1
Bug 1316396: Part 4 - Refactor ChildAPIManager and sub-classes. r=aswan MozReview-Commit-ID: LVChZtLxyNT
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionParent.jsm
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -535,52 +535,41 @@ class ProxyAPIImplementation extends Sch
   hasListener(listener) {
     let set = this.childApiManager.listeners.get(this.path);
     return set ? set.has(listener) : false;
   }
 }
 
 let nextId = 1;
 
-// We create one instance of this class for every extension context
-// that needs to use remote APIs. It uses the message manager to
-// communicate with the ParentAPIManager singleton in
-// Extension.jsm. It handles asynchronous function calls as well as
-// event listeners.
-class ChildAPIManager {
+// We create one instance of this class for every extension context that
+// needs to use remote APIs. It uses the message manager to communicate
+// with the ParentAPIManager singleton in ExtensionParent.jsm. It
+// handles asynchronous function calls as well as event listeners.
+class ChildAPIManagerBase {
   constructor(context, messageManager, localApis, contextData) {
     this.context = context;
     this.messageManager = messageManager;
 
     // The root namespace of all locally implemented APIs. If an extension calls
     // an API that does not exist in this object, then the implementation is
     // delegated to the ParentAPIManager.
     this.localApis = localApis;
 
-    let id = String(context.extension.id) + "." + String(context.contextId);
-    this.id = id;
-
-    let data = {childId: id, extensionId: context.extension.id, principal: context.principal};
-    Object.assign(data, contextData);
+    this.id = `${context.extension.id}.${context.contextId}`;
 
     messageManager.addMessageListener("API:RunListener", this);
     messageManager.addMessageListener("API:CallResult", this);
 
     // Map[path -> Set[listener]]
     // path is, e.g., "runtime.onMessage".
     this.listeners = new Map();
 
     // Map[callId -> Deferred]
     this.callPromises = new Map();
-
-    this.createProxyContextInConstructor(data);
-  }
-
-  createProxyContextInConstructor(data) {
-    this.messageManager.sendAsyncMessage("API:CreateProxyContext", data);
   }
 
   receiveMessage({name, data}) {
     if (data.childId != this.id) {
       return;
     }
 
     switch (name) {
@@ -649,21 +638,21 @@ class ChildAPIManager {
    * `removeListener` is used on a listener that was added on another object
    * through `addListener`, then the event is unregistered.
    *
    * @param {string} path The full name of the event, e.g. "tabs.onCreated".
    * @returns {object} An object with the addListener, removeListener and
    *   hasListener methods. See SchemaAPIInterface for documentation.
    */
   getParentEvent(path) {
-    let parsed = /^(.+)\.(on[A-Z][^.]+)$/.exec(path);
-    if (!parsed) {
-      throw new Error("getParentEvent: Invalid event name: " + path);
-    }
-    let [, namespace, name] = parsed;
+    path = path.split(".");
+
+    let name = path.pop();
+    let namespace = path.join(".");
+
     let impl = new ProxyAPIImplementation(namespace, name, this);
     return {
       addListener: (listener, ...args) => impl.addListener(listener, args),
       removeListener: (listener) => impl.removeListener(listener),
       hasListener: (listener) => impl.hasListener(listener),
     };
   }
 
@@ -687,94 +676,101 @@ class ChildAPIManager {
     }
     if (allowedContexts.includes("addon_parent_only")) {
       return false;
     }
     return true;
   }
 
   getImplementation(namespace, name) {
-    let pathObj = this.localApis;
-    if (pathObj) {
-      for (let part of namespace.split(".")) {
-        pathObj = pathObj[part];
-        if (!pathObj) {
-          break;
-        }
-      }
-      if (pathObj && name in pathObj) {
-        return new LocalAPIImplementation(pathObj, name, this.context);
-      }
+    let obj = namespace.split(".").reduce(
+      (object, prop) => object && object[prop],
+      this.localApis);
+
+    if (obj && name in obj) {
+      return new LocalAPIImplementation(obj, name, this.context);
     }
 
     return this.getFallbackImplementation(namespace, name);
   }
 
   getFallbackImplementation(namespace, name) {
     // No local API found, defer implementation to the parent.
     return new ProxyAPIImplementation(namespace, name, this);
   }
 
   hasPermission(permission) {
     return this.context.extension.hasPermission(permission);
   }
 }
 
+class ChildAPIManager extends ChildAPIManagerBase {
+  constructor(context, messageManager, localApis, contextData) {
+    super(context, messageManager, localApis, contextData);
+
+    let params = {
+      childId: this.id,
+      extensionId: context.extension.id,
+      principal: context.principal,
+    };
+    Object.assign(params, contextData);
+
+    this.messageManager.sendAsyncMessage("API:CreateProxyContext", params);
+  }
+}
+
 
 // A class that behaves identical to a ChildAPIManager, except
 // 1) creation of the ProxyContext in the parent is synchronous, and
 // 2) APIs without a local implementation and marked as incompatible with the
 //    out-of-process model fall back to directly invoking the parent methods.
 // TODO(robwu): Remove this when all APIs have migrated.
-class PseudoChildAPIManager extends ChildAPIManager {
-  createProxyContextInConstructor(originalData) {
-    // Create a structured clone to simulate IPC.
-    let data = Object.assign({}, originalData, {principal: null});
-    data = Cu.cloneInto(data, {});
+class PseudoChildAPIManager extends ChildAPIManagerBase {
+  constructor(context, messageManager, localApis, contextData) {
+    super(context, messageManager, localApis, contextData);
+
+    let params = {
+      childId: this.id,
+      extensionId: context.extension.id,
+    };
+    Object.assign(params, contextData);
+
+    // Structured clone the parameters to simulate cross-process messaging.
+    params = Cu.cloneInto(params, {});
     // Principals can be structured cloned by message managers, but not
     // by cloneInto.
-    data.principal = originalData.principal;
+    params.principal = context.principal;
+    params.cloneScope = this.cloneScope;
 
-    this.url = data.url;
+    this.url = params.url;
 
-    let name = "API:CreateProxyContext";
-    // The <browser> that receives messages from `this.messageManager`.
-    let target = this.context.contentWindow
-      .QueryInterface(Ci.nsIInterfaceRequestor)
-      .getInterface(Ci.nsIDocShell)
-      .chromeEventHandler;
-    ParentAPIManager.receiveMessage({name, data, target});
-
-    let proxyContext = ParentAPIManager.proxyContexts.get(this.id);
-
-    // Use an identical cloneScope in the parent as the child to have identical
-    // behavior for proxied vs direct calls. If all APIs are proxied, then the
-    // parent cloneScope does not really matter (because when the message
-    // arrives locally, the object is cloned into the local clone scope).
-    // If all calls are direct, then the parent cloneScope does matter, because
-    // the objects are not cloned again.
-    Object.defineProperty(proxyContext, "cloneScope", {
-      get: () => this.cloneScope,
+    let browserElement = this.context.docShell.chromeEventHandler;
+    ParentAPIManager.receiveMessage({
+      name: "API:CreateProxyContext",
+      data: params,
+      target: browserElement,
     });
 
+    this.parentContext = ParentAPIManager.proxyContexts.get(this.id);
+
     // Synchronously unload the ProxyContext because we synchronously create it.
-    this.context.callOnClose(proxyContext);
+    this.context.callOnClose(this.parentContext);
   }
 
   getFallbackImplementation(namespace, name) {
     // This is gross and should be removed ASAP.
-    let shouldSynchronouslyUseParentAPI = false;
-    // Incompatible APIs are listed here.
-    if (namespace == "webNavigation" || // ChildAPIManager is oblivious to filters.
-        namespace == "webRequest") { // Incompatible by design (synchronous).
-      shouldSynchronouslyUseParentAPI = true;
-    }
-    if (shouldSynchronouslyUseParentAPI) {
-      let proxyContext = ParentAPIManager.proxyContexts.get(this.id);
-      let apiObj = findPathInObject(proxyContext.apiObj, namespace, false);
+    let useDirectParentAPI = (
+      // Incompatible APIs are listed here.
+      namespace == "webNavigation" || // ChildAPIManager is oblivious to filters.
+      namespace == "webRequest" // Incompatible by design (synchronous).
+    );
+
+    if (useDirectParentAPI) {
+      let apiObj = findPathInObject(this.parentContext.apiObj, namespace, false);
+
       if (apiObj && name in apiObj) {
         return new LocalAPIImplementation(apiObj, name, this.context);
       }
       // If we got here, then it means that the JSON schema claimed that the API
       // will be available, but no actual implementation is given.
       // You should either provide an implementation or rewrite the JSON schema.
     }
 
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -286,16 +286,24 @@ class ProxyContextParent extends BaseCon
     this.currentMessageManager = xulBrowser.messageManager;
     this._docShellTracker = new BrowserDocshellFollower(
       xulBrowser, this.onBrowserChange.bind(this));
 
     Object.defineProperty(this, "principal", {
       value: principal, enumerable: true, configurable: true,
     });
 
+    // TODO: Replace this with a Sandbox with our content principal when
+    // we move to separate processes.
+    if (params.cloneScope) {
+      Object.defineProperty(this, "cloneScope", {
+        value: params.cloneScope, enumerable: true, configurable: true,
+      });
+    }
+
     this.listenerProxies = new Map();
 
     apiManager.emit("proxy-context-load", this);
   }
 
   get cloneScope() {
     return this.sandbox;
   }