Bug 1324467 - Make copy of data to send to listener; r=automatedtester a=test-only
☠☠ backed out by 4fbf5d14ce92 ☠ ☠
authorAndreas Tolfsen <ato@mozilla.com>
Wed, 21 Dec 2016 09:29:00 +0100
changeset 356994 fa1565d8ff442286e635a1fed61b02e007e636fb
parent 356993 e736096955d7c716af287e146b3fbd1e6a1855a3
child 356995 65271a0777167a85058f50fdea4f2d794d7d5027
push id6701
push usercbook@mozilla.com
push dateThu, 22 Dec 2016 15:06:02 +0000
treeherdermozilla-beta@fa1565d8ff44 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersautomatedtester, test-only
bugs1324467
milestone51.0
Bug 1324467 - Make copy of data to send to listener; r=automatedtester a=test-only 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
@@ -188,58 +188,72 @@ 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) {
+GeckoDriver.prototype.sendAsync = function (name, msg, cmdId) {
   let curRemoteFrame = this.curBrowser.frameManager.currentRemoteFrame;
   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}
@@ -435,18 +449,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--;
       }
 
@@ -459,17 +473,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);
   });
 };
 
@@ -541,18 +555,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.
@@ -2771,8 +2787,17 @@ GeckoDriver.prototype.commands = {
   "setWindowSize": GeckoDriver.prototype.setWindowSize,
   "maximizeWindow": GeckoDriver.prototype.maximizeWindow,
   "dismissDialog": GeckoDriver.prototype.dismissDialog,
   "acceptDialog": GeckoDriver.prototype.acceptDialog,
   "getTextFromDialog": GeckoDriver.prototype.getTextFromDialog,
   "sendKeysToDialog": GeckoDriver.prototype.sendKeysToDialog,
   "quitApplication": GeckoDriver.prototype.quitApplication,
 };
+
+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);