Bug 1324467 - Make copy of data to send to listener; r?automatedtester draft
authorAndreas Tolfsen <ato@mozilla.com>
Mon, 19 Dec 2016 19:08:46 +0000
changeset 451157 7960157193edf70730d6c9224918b62c8838e43b
parent 451111 d4b3146a5567a7ddbcdfa5244945db55616cb8d1
child 539939 d078dc892f174c207b5285a5a1702f4ad90eb530
push id39067
push userbmo:ato@mozilla.com
push dateMon, 19 Dec 2016 19:12:58 +0000
reviewersautomatedtester
bugs1324467
milestone53.0a1
Bug 1324467 - Make copy of data to send to listener; r?automatedtester The payload sent to the listener through `GeckoDriver#sendAsync` is sometimes mutated if a `commandID` parameter is given. Because `data` is sometimes a reference to an object, the original object gets modified with an additional `command_id` field. To avoid this we copy the object before mutating it and pass it through to the message manager. MozReview-Commit-ID: HM2tnPqbAge
testing/marionette/driver.js
testing/marionette/listener.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -204,58 +204,71 @@ GeckoDriver.prototype.switchToGlobalMess
     this.curBrowser.frameManager.currentRemoteFrame = null;
   }
   this.mm = globalMessageManager;
 };
 
 /**
  * Helper method to send async messages to the content listener.
  * Correct usage is to pass in the name of a function in listener.js,
- * a message object consisting of JSON serialisable primitives,
- * and the current command's ID.
+ * a serialisable object, and optionally the current command's ID
+ * when not using the modern dispatching technique.
  *
  * @param {string} name
- *     Suffix of the targetted message listener ({@code Marionette:<suffix>}).
+ *     Suffix of the targetted message listener
+ *     ({@code Marionette:<suffix>}).
  * @param {Object=} msg
- *     JSON serialisable object to send to the listener.
- * @param {number=} cmdId
- *     Command ID to ensure synchronisity.
+ *     Optional JSON serialisable object to send to the listener.
+ * @param {number=} commandID
+ *     Optional command ID to ensure synchronisity.
  */
-GeckoDriver.prototype.sendAsync = function (name, msg, cmdId) {
-  let curRemoteFrame = this.curBrowser.frameManager.currentRemoteFrame;
+GeckoDriver.prototype.sendAsync = function (name, data, commandID) {
   name = "Marionette:" + name;
+  let payload = copy(data);
 
   // TODO(ato): When proxy.AsyncMessageChannel
   // is used for all chrome <-> content communication
   // this can be removed.
-  if (cmdId) {
-    msg.command_id = cmdId;
+  if (commandID) {
+    payload.command_id = commandID;
   }
 
-  if (curRemoteFrame === null) {
-    this.curBrowser.executeWhenReady(() => {
-      if (this.curBrowser.curFrameId) {
-        this.mm.broadcastAsyncMessage(name + this.curBrowser.curFrameId, msg);
-      } else {
-        throw new NoSuchWindowError(
-            "No such content frame; perhaps the listener was not registered?");
-      }
-    });
+  if (!this.curBrowser.frameManager.currentRemoteFrame) {
+    this.broadcastDelayedAsyncMessage_(name, payload);
   } else {
-    let remoteFrameId = curRemoteFrame.targetFrameId;
-    try {
-      this.mm.sendAsyncMessage(name + remoteFrameId, msg);
-    } catch (e) {
-      switch(e.result) {
-        case Cr.NS_ERROR_FAILURE:
-        case Cr.NS_ERROR_NOT_INITIALIZED:
-          throw new NoSuchWindowError();
-        default:
-          throw new WebDriverError(e.toString());
-      }
+    this.sendTargettedAsyncMessage_(name, payload);
+  }
+};
+
+GeckoDriver.prototype.broadcastDelayedAsyncMessage_ = function (name, payload) {
+  this.curBrowser.executeWhenReady(() => {
+    if (this.curBrowser.curFrameId) {
+      const target = name + this.curBrowser.curFrameId;
+      this.mm.broadcastAsyncMessage(target, payload);
+    } else {
+      throw new NoSuchWindowError(
+          "No such content frame; perhaps the listener was not registered?");
+    }
+  });
+};
+
+GeckoDriver.prototype.sendTargettedAsyncMessage_ = function (name, payload) {
+  const curRemoteFrame = this.curBrowser.frameManager.currentRemoteFrame;
+  const target = name + curRemoteFrame.targetFrameId;
+
+  try {
+    this.mm.sendAsyncMessage(target, payload);
+  } catch (e) {
+    switch (e.result) {
+      case Cr.NS_ERROR_FAILURE:
+      case Cr.NS_ERROR_NOT_INITIALIZED:
+        throw new NoSuchWindowError();
+
+      default:
+        throw new WebDriverError(e);
     }
   }
 };
 
 /**
  * Gets the current active window.
  *
  * @return {nsIDOMWindow}
@@ -457,18 +470,18 @@ GeckoDriver.prototype.registerBrowser = 
   }
 
   return [reg, mainContent, this.sessionCapabilities];
 };
 
 GeckoDriver.prototype.registerPromise = function() {
   const li = "Marionette:register";
 
-  return new Promise((resolve) => {
-    let cb = (msg) => {
+  return new Promise(resolve => {
+    let cb = msg => {
       let wid = msg.json.value;
       let be = msg.target;
       let rv = this.registerBrowser(wid, be);
 
       if (this.curBrowser.frameRegsPending > 0) {
         this.curBrowser.frameRegsPending--;
       }
 
@@ -481,17 +494,17 @@ GeckoDriver.prototype.registerPromise = 
       return rv;
     };
     this.mm.addMessageListener(li, cb);
   });
 };
 
 GeckoDriver.prototype.listeningPromise = function() {
   const li = "Marionette:listenersAttached";
-  return new Promise((resolve) => {
+  return new Promise(resolve => {
     let cb = () => {
       this.mm.removeMessageListener(li, cb);
       resolve();
     };
     this.mm.addMessageListener(li, cb);
   });
 };
 
@@ -579,18 +592,20 @@ GeckoDriver.prototype.newSession = funct
     }, BROWSER_STARTUP_FINISHED, false);
   } else {
     runSessionStart.call(this);
   }
 
   yield registerBrowsers;
   yield browserListening;
 
-  resp.body.sessionId = this.sessionId;
-  resp.body.capabilities = this.sessionCapabilities;
+  return {
+    sessionId: this.sessionId,
+    capabilities: this.sessionCapabilities,
+  };
 };
 
 /**
  * Send the current session's capabilities to the client.
  *
  * Capabilities informs the client of which WebDriver features are
  * supported by Firefox and Marionette.  They are immutable for the
  * length of the session.
@@ -666,17 +681,16 @@ GeckoDriver.prototype.setSessionCapabili
     throw new SessionNotCreatedError(
         `Not all requiredCapabilities could be met: ${JSON.stringify(errors)}`);
   };
 
   // clone, overwrite, and set
   let caps = copy(this.sessionCapabilities);
   caps = copy(newCaps, caps);
   logger.config("Changing capabilities: " + JSON.stringify(caps));
-
   this.sessionCapabilities = caps;
 };
 
 GeckoDriver.prototype.setUpProxy = function (proxy) {
   logger.config("User-provided proxy settings: " + JSON.stringify(proxy));
 
   assert.object(proxy);
   if (!proxy.hasOwnProperty("proxyType")) {
@@ -2925,8 +2939,17 @@ GeckoDriver.prototype.commands = {
   "quitApplication": GeckoDriver.prototype.quitApplication,
 
   "localization:l10n:localizeEntity": GeckoDriver.prototype.localizeEntity,
   "localization:l10n:localizeProperty": GeckoDriver.prototype.localizeProperty,
 
   "addon:install": GeckoDriver.prototype.installAddon,
   "addon:uninstall": GeckoDriver.prototype.uninstallAddon,
 };
+
+function copy (obj) {
+  if (Array.isArray(obj)) {
+    return obj.slice();
+  } else if (typeof obj == "object") {
+    return Object.assign({}, obj);
+  }
+  return obj;
+}
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -118,21 +118,20 @@ var sandboxName = "default";
 function registerSelf() {
   let msg = {value: winUtil.outerWindowID};
   // register will have the ID and a boolean describing if this is the main process or not
   let register = sendSyncMessage("Marionette:register", msg);
 
   if (register[0]) {
     let {id, remotenessChange} = register[0][0];
     capabilities = register[0][2];
-    isB2G = capabilities.platformName == "b2g";
     listenerId = id;
     if (typeof id != "undefined") {
       // check if we're the main process
-      if (register[0][1] == true) {
+      if (register[0][1]) {
         addMessageListener("MarionetteMainListener:emitTouchEvent", emitTouchEventForIFrame);
       }
       startListeners();
       let rv = {};
       if (remotenessChange) {
         rv.listenerId = id;
       }
       sendAsyncMessage("Marionette:listenersAttached", rv);