Bug 1316784: Kill PseudoChildAPIManager. r=aswan
☠☠ backed out by 9381a00d19f8 ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Fri, 11 Nov 2016 15:50:29 -0800
changeset 322343 888815bca14ce217d0be88ec39a79018790ee969
parent 322342 05c206c4311bc4d6e727f89243aea2b5139fca61
child 322344 67858d51f777ad68685a7d0cca0355229fe17716
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewersaswan
bugs1316784
milestone52.0a1
Bug 1316784: Kill PseudoChildAPIManager. r=aswan MozReview-Commit-ID: CIRltzhWWAJ
browser/components/extensions/test/browser/browser_ext_windows_create_tabId.js
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/ExtensionParent.jsm
--- a/browser/components/extensions/test/browser/browser_ext_windows_create_tabId.js
+++ b/browser/components/extensions/test/browser/browser_ext_windows_create_tabId.js
@@ -91,24 +91,29 @@ add_task(function* testWindowCreate() {
       browser.test.log("Try to create a window with an invalid tabId");
       await browser.test.assertRejects(
         browser.windows.create({tabId: 0}),
         /Invalid tab ID: 0/,
         "Create call failed as expected");
 
 
       browser.test.log("Try to create a window with two URLs");
-      [, , window] = await Promise.all([
+      let readyPromise = Promise.all([
         // tabs.onUpdated can be invoked between the call of windows.create and
         // the invocation of its callback/promise, so set up the listeners
         // before creating the window.
         promiseTabUpdated("http://example.com/"),
         promiseTabUpdated("http://example.org/"),
-        browser.windows.create({url: ["http://example.com/", "http://example.org/"]}),
       ]);
+
+      await new Promise(resolve => setTimeout(resolve, 0));
+
+      window = await browser.windows.create({url: ["http://example.com/", "http://example.org/"]});
+      await readyPromise;
+
       browser.test.assertEq(2, window.tabs.length, "2 tabs were opened in new window");
       browser.test.assertEq("about:blank", window.tabs[0].url, "about:blank, page not loaded yet");
       browser.test.assertEq("about:blank", window.tabs[1].url, "about:blank, page not loaded yet");
 
       window = await browser.windows.get(window.id, {populate: true});
 
       browser.test.assertEq(2, window.tabs.length, "2 tabs were opened in new window");
       browser.test.assertEq("http://example.com/", window.tabs[0].url, "Correct URL was loaded in tab 1");
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -17,42 +17,36 @@ this.EXPORTED_SYMBOLS = ["ExtensionChild
 const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cu = Components.utils;
 const Cr = Components.results;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
-XPCOMUtils.defineLazyModuleGetter(this, "ExtensionParent",
-                                  "resource://gre/modules/ExtensionParent.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "MessageChannel",
                                   "resource://gre/modules/MessageChannel.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "NativeApp",
                                   "resource://gre/modules/NativeMessaging.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
                                   "resource://gre/modules/PromiseUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Schemas",
                                   "resource://gre/modules/Schemas.jsm");
 
-XPCOMUtils.defineLazyGetter(this, "ParentAPIManager",
-                            () => ExtensionParent.ParentAPIManager);
-
 const CATEGORY_EXTENSION_SCRIPTS_ADDON = "webextension-scripts-addon";
 
 Cu.import("resource://gre/modules/ExtensionCommon.jsm");
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 const {
   DefaultMap,
   EventManager,
   SingletonEventManager,
   SpreadArgs,
   defineLazyGetter,
-  findPathInObject,
   getInnerWindowID,
   getMessageManager,
   getUniqueId,
   injectAPI,
 } = ExtensionUtils;
 
 const {
   BaseContext,
@@ -537,20 +531,21 @@ class ProxyAPIImplementation extends Sch
     return map.listeners.has(listener);
   }
 }
 
 // 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 {
+class ChildAPIManager {
   constructor(context, messageManager, localApis, contextData) {
     this.context = context;
     this.messageManager = messageManager;
+    this.url = contextData.url;
 
     // 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;
 
     this.id = `${context.extension.id}.${context.contextId}`;
 
@@ -561,16 +556,25 @@ class ChildAPIManagerBase {
 
     this.listeners = new DefaultMap(() => ({
       ids: new Map(),
       listeners: new Map(),
     }));
 
     // Map[callId -> Deferred]
     this.callPromises = new Map();
+
+    let params = {
+      childId: this.id,
+      extensionId: context.extension.id,
+      principal: context.principal,
+    };
+    Object.assign(params, contextData);
+
+    this.messageManager.sendAsyncMessage("API:CreateProxyContext", params);
   }
 
   receiveMessage({name, messageName, data}) {
     if (data.childId != this.id) {
       return;
     }
 
     switch (name || messageName) {
@@ -702,90 +706,16 @@ class ChildAPIManagerBase {
     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 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.
-    params.principal = context.principal;
-    params.cloneScope = this.cloneScope;
-
-    this.url = params.url;
-
-    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(this.parentContext);
-  }
-
-  getFallbackImplementation(namespace, name) {
-    let useDirectParentAPI = (
-      // Incompatible APIs are listed here.
-      false
-    );
-
-    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.
-    }
-
-    return super.getFallbackImplementation(namespace, name);
-  }
-}
-
 class ExtensionPageContextChild extends BaseContext {
   /**
    * This ExtensionPageContextChild represents a privileged addon
    * execution environment that has full access to the WebExtensions
    * APIs (provided that the correct permissions have been requested).
    *
    * This is the child side of the ExtensionPageContextParent class
    * defined in ExtensionParent.jsm.
@@ -896,17 +826,17 @@ defineLazyGetter(ExtensionPageContextChi
 defineLazyGetter(ExtensionPageContextChild.prototype, "childManager", function() {
   let localApis = {};
   apiManager.generateAPIs(this, localApis);
 
   if (this.viewType == "background") {
     apiManager.global.initializeBackgroundPage(this.contentWindow);
   }
 
-  let childManager = new PseudoChildAPIManager(this, this.messageManager, localApis, {
+  let childManager = new ChildAPIManager(this, this.messageManager, localApis, {
     envType: "addon_parent",
     viewType: this.viewType,
     url: this.uri.spec,
     incognito: this.incognito,
   });
 
   this.callOnClose(childManager);
 
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -195,18 +195,16 @@ class BaseContext {
    * @param {object} [options.recipient]
    *
    * @returns {Promise}
    */
   sendMessage(target, messageName, data, options = {}) {
     options.recipient = options.recipient || {};
     options.sender = options.sender || {};
 
-    // TODO(robwu): This should not unconditionally be overwritten once we
-    // support onMessageExternal / onConnectExternal (bugzil.la/1258360).
     options.recipient.extensionId = this.extension.id;
     options.sender.extensionId = this.extension.id;
     options.sender.contextId = this.contextId;
 
     return MessageChannel.sendMessage(target, messageName, data, options);
   }
 
   get lastError() {
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -258,24 +258,16 @@ class ProxyContextParent extends BaseCon
     // message manager object may change when `xulBrowser` swaps docshells, e.g.
     // when a tab is moved to a different window.
     this.messageManagerProxy = new MessageManagerProxy(xulBrowser);
 
     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;
   }