Bug 1429091 - Use global message manager in AsyncMessageChannel. r?maja_zf draft
authorAndreas Tolfsen <ato@sny.no>
Tue, 09 Jan 2018 15:53:46 +0000
changeset 719298 afa392b9c0f1c1a62b2eb256cebc3fb2dd39251b
parent 719297 fe77befa5210fbd680660abe617145bc821f98c1
child 745750 2aba6f2cfa0edab88f48f17c59b9a93adc80e0fd
push id95208
push userbmo:ato@sny.no
push dateThu, 11 Jan 2018 21:13:16 +0000
reviewersmaja_zf
bugs1429091
milestone59.0a1
Bug 1429091 - Use global message manager in AsyncMessageChannel. r?maja_zf Whenever we make proxied IPC calls to the content frame's message manager, we do so over the global message manager. AsyncMessageChannel takes a closure that returns the current message manager from GeckoDriver#mm, but this is unnecessary because it always holds a global message channel. By not having to pass in a closure returning a message manager to AsyncMessageChannel we losen the tight coupling a little bit. Future patches will further reduce the tight coupling of browserFn. MozReview-Commit-ID: EU0pkxA7lab
testing/marionette/driver.js
testing/marionette/listener.js
testing/marionette/proxy.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -145,18 +145,18 @@ this.GeckoDriver = function(appId, serve
   this.context = Context.Content;
 
   this.sandboxes = new Sandboxes(() => this.getCurrentWindow());
   this.legacyactions = new legacyaction.Chain();
 
   this.capabilities = new session.Capabilities();
 
   this.mm = globalMessageManager;
-  this.listener = proxy.toListener(() => this.mm, this.sendAsync.bind(this),
-      () => this.curBrowser);
+  this.listener = proxy.toListener(
+      this.sendAsync.bind(this), () => this.curBrowser);
 
   // points to an alert instance if a modal dialog is present
   this.dialog = null;
   this.dialogHandler = this.globalModalDialogHandler.bind(this);
 };
 
 Object.defineProperty(GeckoDriver.prototype, "a11yChecks", {
   get() {
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -622,19 +622,17 @@ function deregister() {
  *     Unique identifier of the request.
  * @param {AsyncContentSender.ResponseType} type
  *     Type of response.
  * @param {*} [Object] data
  *     JSON serialisable object to accompany the message.  Defaults to
  *     an empty dictionary.
  */
 function sendToServer(uuid, data = undefined) {
-  let channel = new proxy.AsyncMessageChannel(
-      () => this,
-      sendAsyncMessage.bind(this));
+  let channel = new proxy.AsyncMessageChannel(sendAsyncMessage.bind(this));
   channel.reply(uuid, data);
 }
 
 /**
  * Send asynchronous reply with value to chrome.
  *
  * @param {Object} obj
  *     JSON serialisable object of arbitrary type and complexity.
--- a/testing/marionette/proxy.js
+++ b/testing/marionette/proxy.js
@@ -16,18 +16,21 @@ const {
 } = Cu.import("chrome://marionette/content/error.js", {});
 Cu.import("chrome://marionette/content/evaluate.js");
 Cu.import("chrome://marionette/content/modal.js");
 
 this.EXPORTED_SYMBOLS = ["proxy"];
 
 XPCOMUtils.defineLazyServiceGetter(
     this, "uuidgen", "@mozilla.org/uuid-generator;1", "nsIUUIDGenerator");
+XPCOMUtils.defineLazyServiceGetter(
+    this, "globalMessageManager", "@mozilla.org/globalmessagemanager;1",
+    "nsIMessageBroadcaster");
 
-const logger = Log.repository.getLogger("Marionette");
+const log = Log.repository.getLogger("Marionette");
 
 // Proxy handler that traps requests to get a property.  Will prioritise
 // properties that exist on the object's own prototype.
 const ownPriorityGetterTrap = {
   get: (obj, prop) => {
     if (obj.hasOwnProperty(prop)) {
       return obj[prop];
     }
@@ -45,58 +48,52 @@ this.proxy = {};
  * Calls to this object will be proxied via the message manager to a
  * content frame script, and responses are returend as promises.
  *
  * The argument sequence is serialised and passed as an array, unless it
  * consists of a single object type that isn't null, in which case it's
  * passed literally.  The latter specialisation is temporary to achieve
  * backwards compatibility with listener.js.
  *
- * @param {function(): (nsIMessageSender|nsIMessageBroadcaster)} mmFn
- *     Closure function returning the current message manager.
  * @param {function(string, Object, number)} sendAsyncFn
  *     Callback for sending async messages.
+ * @param {function(): browser.Context} browserFn
+ *     Closure that returns the current browsing context.
  */
-proxy.toListener = function(mmFn, sendAsyncFn, browserFn) {
-  let sender = new proxy.AsyncMessageChannel(
-      mmFn, sendAsyncFn, browserFn);
+proxy.toListener = function(sendAsyncFn, browserFn) {
+  let sender = new proxy.AsyncMessageChannel(sendAsyncFn, browserFn);
   return new Proxy(sender, ownPriorityGetterTrap);
 };
 
 /**
  * Provides a transparent interface between chrome- and content space.
  *
  * The AsyncMessageChannel is an abstraction of the message manager
  * IPC architecture allowing calls to be made to any registered message
  * listener in Marionette.  The <code>#send(...)</code> method
  * returns a promise that gets resolved when the message handler calls
  * <code>.reply(...)</code>.
  */
 proxy.AsyncMessageChannel = class {
-  constructor(mmFn, sendAsyncFn, browserFn) {
-    this.mmFn_ = mmFn;
+  constructor(sendAsyncFn, browserFn) {
     this.sendAsync = sendAsyncFn;
     this.browserFn_ = browserFn;
 
     // TODO(ato): Bug 1242595
     this.activeMessageId = null;
 
     this.listeners_ = new Map();
     this.dialogueObserver_ = null;
     this.closeHandler = null;
   }
 
   get browser() {
     return this.browserFn_();
   }
 
-  get mm() {
-    return this.mmFn_();
-  }
-
   /**
    * Send a message across the channel.  The name of the function to
    * call must be registered as a message listener.
    *
    * Usage:
    *
    * <pre><code>
    *     let channel = new AsyncMessageChannel(
@@ -145,32 +142,32 @@ proxy.AsyncMessageChannel = class {
           default:
             throw new TypeError(`Unknown async response type: ${type}`);
         }
       };
 
       // The currently selected tab or window has been closed. No clean-up
       // is necessary to do because all loaded listeners are gone.
       this.closeHandler = ({type, target}) => {
-        logger.debug(`Received DOM event ${type} for ${target}`);
+        log.debug(`Received DOM event ${type} for ${target}`);
 
         switch (type) {
           case "TabClose":
           case "unload":
             this.removeHandlers();
             resolve();
             break;
         }
       };
 
       // A modal or tab modal dialog has been opened. To be able to handle it,
       // the active command has to be aborted. Therefore remove all handlers,
       // and cancel any ongoing requests in the listener.
       this.dialogueObserver_ = (subject, topic) => {
-        logger.debug(`Received observer notification ${topic}`);
+        log.debug(`Received observer notification ${topic}`);
 
         this.removeAllListeners_();
         // TODO(ato): It's not ideal to have listener specific behaviour here:
         this.sendAsync("cancelRequest");
 
         this.removeHandlers();
         resolve();
       };
@@ -289,27 +286,27 @@ proxy.AsyncMessageChannel = class {
 
   addListener_(path, callback) {
     let autoRemover = msg => {
       this.removeListener_(path);
       this.removeHandlers();
       callback(msg);
     };
 
-    this.mm.addMessageListener(path, autoRemover);
+    globalMessageManager.addMessageListener(path, autoRemover);
     this.listeners_.set(path, autoRemover);
   }
 
   removeListener_(path) {
     if (!this.listeners_.has(path)) {
       return true;
     }
 
     let l = this.listeners_.get(path);
-    this.mm.removeMessageListener(path, l[1]);
+    globalMessageManager.removeMessageListener(path, l[1]);
     return this.listeners_.delete(path);
   }
 
   removeAllListeners_() {
     let ok = true;
     for (let [p] of this.listeners_) {
       ok |= this.removeListener_(p);
     }