Bug 1379490 - Query capabilities configuration from chrome. r=automatedtester
authorAndreas Tolfsen <ato@sny.no>
Tue, 02 Jan 2018 12:00:41 +0000
changeset 449938 2102ecc18aefa9ac90b297f2f02321b13e1ee40f
parent 449937 7565fa6f3481056a8ed1050c94811d041661f38b
child 449939 8ba13ef1fec4be584729443cf4ed6af3d576a3ed
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersautomatedtester
bugs1379490
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1379490 - Query capabilities configuration from chrome. r=automatedtester Capabilities are currently sent to the content frame script when its IPC message listeners attach (on Marionette:ListenersAttached). If for whatever reason a capability's value changes since the script was registered, for example by a user calling the WebDriver:SetTimeouts command, a race condition is introduced where the capabilities in chrome will differ from those cached in the frame script. To remove any chance of race conditions, this patch changes the content frame script to query the capabilities from chrome every time it needs them. This is slightly less efficient, but should be neglible. The patch also clears up some unused state, such as the curBrowser.newSessionCommandId property, which did not appear to be used for anything interesting. MozReview-Commit-ID: 1bSrRu5nK3h
testing/marionette/driver.js
testing/marionette/listener.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -273,28 +273,30 @@ Object.defineProperty(GeckoDriver.protot
 
 GeckoDriver.prototype.QueryInterface = XPCOMUtils.generateQI([
   Ci.nsIMessageListener,
   Ci.nsIObserver,
   Ci.nsISupportsWeakReference,
 ]);
 
 GeckoDriver.prototype.init = function() {
+  this.mm.addMessageListener("Marionette:WebDriver:GetCapabilities", this);
   this.mm.addMessageListener("Marionette:GetLogLevel", this);
   this.mm.addMessageListener("Marionette:getVisibleCookies", this);
-  this.mm.addMessageListener("Marionette:listenersAttached", this);
-  this.mm.addMessageListener("Marionette:register", this);
+  this.mm.addMessageListener("Marionette:ListenersAttached", this);
+  this.mm.addMessageListener("Marionette:Register", this);
   this.mm.addMessageListener("Marionette:switchedToFrame", this);
 };
 
 GeckoDriver.prototype.uninit = function() {
+  this.mm.removeMessageListener("Marionette:WebDriver:GetCapabilities", this);
   this.mm.removeMessageListener("Marionette:GetLogLevel", this);
   this.mm.removeMessageListener("Marionette:getVisibleCookies", this);
-  this.mm.removeMessageListener("Marionette:listenersAttached", this);
-  this.mm.removeMessageListener("Marionette:register", this);
+  this.mm.removeMessageListener("Marionette:ListenersAttached", this);
+  this.mm.removeMessageListener("Marionette:Register", this);
   this.mm.removeMessageListener("Marionette:switchedToFrame", this);
 };
 
 /**
  * Callback used to observe the creation of new modal or tab modal dialogs
  * during the session's lifetime.
  */
 GeckoDriver.prototype.globalModalDialogHandler = function(subject, topic) {
@@ -438,17 +440,16 @@ GeckoDriver.prototype.addBrowser = funct
  *     Window whose browser we need to access.
  * @param {boolean=} [false] isNewSession
  *     True if this is the first time we're talking to this browser.
  */
 GeckoDriver.prototype.startBrowser = function(window, isNewSession = false) {
   this.mainFrame = window;
   this.curFrame = null;
   this.addBrowser(window);
-  this.curBrowser.isNewSession = isNewSession;
   this.whenBrowserStarted(window, isNewSession);
 };
 
 /**
  * Callback invoked after a new session has been started in a browser.
  * Loads the Marionette frame script into the browser if needed.
  *
  * @param {ChromeWindow} window
@@ -531,59 +532,53 @@ GeckoDriver.prototype.registerBrowser = 
   if (this.appId != APP_ID_FIREFOX || be.namespaceURI != XUL_NS ||
       be.nodeName != "browser" || be.getTabBrowser()) {
     // curBrowser holds all the registered frames in knownFrames
     this.curBrowser.register(id, be);
   }
 
   this.wins.set(id, listenerWindow);
   if (nullPrevious && (this.curBrowser.curFrameId !== null)) {
-    this.sendAsync(
-        "newSession",
-        this.capabilities,
-        this.newSessionCommandId);
-    if (this.curBrowser.isNewSession) {
-      this.newSessionCommandId = null;
-    }
+    this.sendAsync("newSession");
   }
 
-  return [id, this.capabilities.toJSON()];
+  return id;
 };
 
 GeckoDriver.prototype.registerPromise = function() {
-  const li = "Marionette:register";
+  const li = "Marionette:Register";
 
   return new Promise(resolve => {
     let cb = msg => {
       let wid = msg.json.value;
       let be = msg.target;
-      let rv = this.registerBrowser(wid, be);
+      let outerWindowID = this.registerBrowser(wid, be);
 
       if (this.curBrowser.frameRegsPending > 0) {
         this.curBrowser.frameRegsPending--;
       }
 
       if (this.curBrowser.frameRegsPending === 0) {
         this.mm.removeMessageListener(li, cb);
         resolve();
       }
 
       // this is a sync message and listeners expect the ID back
-      return rv;
+      return outerWindowID;
     };
     this.mm.addMessageListener(li, cb);
   });
 };
 
 GeckoDriver.prototype.listeningPromise = function() {
-  const li = "Marionette:listenersAttached";
+  const li = "Marionette:ListenersAttached";
 
   return new Promise(resolve => {
     let cb = msg => {
-      if (msg.json.listenerId === this.curBrowser.curFrameId) {
+      if (msg.json.outerWindowID === this.curBrowser.curFrameId) {
         this.mm.removeMessageListener(li, cb);
         resolve();
       }
     };
     this.mm.addMessageListener(li, cb);
   });
 };
 
@@ -694,17 +689,16 @@ GeckoDriver.prototype.listeningPromise =
  * @throws {SessionNotCreatedError}
  *     If, for whatever reason, a session could not be created.
  */
 GeckoDriver.prototype.newSession = async function(cmd) {
   if (this.sessionID) {
     throw new SessionNotCreatedError("Maximum number of active sessions");
   }
   this.sessionID = WebElement.generateUUID();
-  this.newSessionCommandId = cmd.id;
 
   try {
     this.capabilities = session.Capabilities.fromJSON(cmd.parameters);
 
     if (!this.secureTLS) {
       logger.warn("TLS certificate errors will be ignored for this session");
       let acceptAllCerts = new cert.InsecureSweepingOverride();
       cert.installOverride(acceptAllCerts);
@@ -3338,32 +3332,35 @@ GeckoDriver.prototype.receiveMessage = f
           this.currentFrameElement =
               new ChromeWebElement(message.json.frameValue);
         } else {
           this.currentFrameElement = null;
         }
       }
       break;
 
-    case "Marionette:register":
+    case "Marionette:Register":
       let wid = message.json.value;
       let be = message.target;
-      let rv = this.registerBrowser(wid, be);
-      return rv;
-
-    case "Marionette:listenersAttached":
-      if (message.json.listenerId === this.curBrowser.curFrameId) {
+      let outerWindowID = this.registerBrowser(wid, be);
+      return {outerWindowID};
+
+    case "Marionette:ListenersAttached":
+      if (message.json.outerWindowID === this.curBrowser.curFrameId) {
         // If the frame script gets reloaded we need to call newSession.
         // In the case of desktop this just sets up a small amount of state
         // that doesn't change over the course of a session.
-        this.sendAsync("newSession", this.capabilities);
+        this.sendAsync("newSession");
         this.curBrowser.flushPendingCommands();
       }
       break;
 
+    case "Marionette:WebDriver:GetCapabilities":
+      return this.capabilities.toJSON();
+
     case "Marionette:GetLogLevel":
       return logger.level;
   }
 };
 /* eslint-enable consistent-return */
 
 GeckoDriver.prototype.responseCompleted = function() {
   if (this.curBrowser !== null) {
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -42,17 +42,17 @@ const {ContentEventObserverService} = Cu
 Cu.import("chrome://marionette/content/interaction.js");
 Cu.import("chrome://marionette/content/legacyaction.js");
 Cu.import("chrome://marionette/content/navigate.js");
 Cu.import("chrome://marionette/content/proxy.js");
 Cu.import("chrome://marionette/content/session.js");
 
 Cu.importGlobalProperties(["URL"]);
 
-let listenerId = null; // unique ID of this listener
+let outerWindowID = null;
 let curContainer = {frame: content, shadowRoot: null};
 
 // Listen for click event to indicate one click has happened, so actions
 // code can send dblclick event, also resetClick and cancelTimer
 // after dblclick has happened.
 addEventListener("click", event.DoubleClickTracker.setClick);
 addEventListener("dblclick", event.DoubleClickTracker.resetClick);
 addEventListener("dblclick", event.DoubleClickTracker.cancelTimer);
@@ -64,17 +64,23 @@ const SUPPORTED_STRATEGIES = new Set([
   element.Strategy.ID,
   element.Strategy.Name,
   element.Strategy.LinkText,
   element.Strategy.PartialLinkText,
   element.Strategy.TagName,
   element.Strategy.XPath,
 ]);
 
-let capabilities;
+Object.defineProperty(this, "capabilities", {
+  get() {
+    let payload = sendSyncMessage("Marionette:WebDriver:GetCapabilities");
+    return session.Capabilities.fromJSON(payload[0]);
+  },
+  configurable: true,
+});
 
 let legacyactions = new legacyaction.Chain();
 
 // last touch for each fingerId
 let multiLast = {};
 
 // TODO: Log.jsm is not e10s compatible (see https://bugzil.la/1411513),
 // query the main process for the current log level
@@ -442,25 +448,21 @@ const loadListener = {
  * an ID, we start the listeners. Otherwise, nothing happens.
  */
 function registerSelf() {
   let msg = {value: winUtil.outerWindowID};
   logger.debug(`Register listener.js for window ${msg.value}`);
 
   // register will have the ID and a boolean describing if this is the
   // main process or not
-  let register = sendSyncMessage("Marionette:register", msg);
+  let register = sendSyncMessage("Marionette:Register", msg);
   if (register[0]) {
-    listenerId = register[0][0];
-    capabilities = session.Capabilities.fromJSON(register[0][1]);
-    if (typeof listenerId != "undefined") {
-      startListeners();
-      sendAsyncMessage("Marionette:listenersAttached",
-          {"listenerId": listenerId});
-    }
+    outerWindowID = register[0].outerWindowID;
+    startListeners();
+    sendAsyncMessage("Marionette:ListenersAttached", {outerWindowID});
   }
 }
 
 // Eventually we will not have a closure for every single command,
 // but use a generic dispatch for all listener commands.
 //
 // Worth nothing that this shares many characteristics with
 // server.TCPConnection#execute.  Perhaps this could be generalised
@@ -485,28 +487,22 @@ function dispatch(fn) {
       resolve(rv);
     });
 
     req.then(rv => sendResponse(rv, id), err => sendError(err, id))
         .catch(err => sendError(err, id));
   };
 }
 
-/**
- * Add a message listener that's tied to our listenerId.
- */
 function addMessageListenerId(messageName, handler) {
-  addMessageListener(messageName + listenerId, handler);
+  addMessageListener(messageName + outerWindowID, handler);
 }
 
-/**
- * Remove a message listener that's tied to our listenerId.
- */
 function removeMessageListenerId(messageName, handler) {
-  removeMessageListener(messageName + listenerId, handler);
+  removeMessageListener(messageName + outerWindowID, handler);
 }
 
 let getPageSourceFn = dispatch(getPageSource);
 let getActiveElementFn = dispatch(getActiveElement);
 let getElementAttributeFn = dispatch(getElementAttribute);
 let getElementPropertyFn = dispatch(getElementProperty);
 let getElementTextFn = dispatch(getElementText);
 let getElementTagNameFn = dispatch(getElementTagName);
@@ -577,19 +573,22 @@ function startListeners() {
   addMessageListener("Marionette:DOM:AddEventListener", domAddEventListener);
   addMessageListener("Marionette:DOM:RemoveEventListener", domRemoveEventListener);
 }
 
 /**
  * Called when we start a new session. It registers the
  * current environment, and resets all values
  */
-function newSession(msg) {
-  capabilities = session.Capabilities.fromJSON(msg.json);
-  resetValues();
+function newSession() {
+  sandboxes.clear();
+  curContainer = {frame: content, shadowRoot: null};
+  legacyactions.mouseEventsOnly = false;
+  action.inputStateMap = new Map();
+  action.inputsToCancel = [];
 }
 
 /**
  * Removes all listeners
  */
 function deleteSession() {
   removeMessageListenerId("Marionette:newSession", newSession);
   removeMessageListenerId("Marionette:execute", executeFn);
@@ -701,27 +700,16 @@ function sendOk(uuid) {
  *     Error to notify chrome of.
  * @param {UUID} uuid
  *     Unique identifier of the request.
  */
 function sendError(err, uuid) {
   sendToServer(uuid, err);
 }
 
-/**
- * Clear test values after completion of test
- */
-function resetValues() {
-  sandboxes.clear();
-  curContainer = {frame: content, shadowRoot: null};
-  legacyactions.mouseEventsOnly = false;
-  action.inputStateMap = new Map();
-  action.inputsToCancel = [];
-}
-
 async function execute(script, args, timeout, opts) {
   opts.timeout = timeout;
   let sb = sandbox.createMutable(curContainer.frame);
   return evaluate.sandbox(sb, script, args, opts);
 }
 
 async function executeInSandbox(script, args, timeout, opts) {
   opts.timeout = timeout;