Bug 1379490 - Query capabilities configuration from chrome. r?automatedtester draft
authorAndreas Tolfsen <ato@sny.no>
Tue, 02 Jan 2018 12:00:41 +0000
changeset 715158 1aeef587d71bfa659542a4c2416ef1b377f92a6d
parent 715146 4d1423f75c677447cafb8cd3f9a5fbdff8bfbca8
child 744713 75522c96379154ca5add84679dc0380c8c91247c
push id94072
push userbmo:ato@sny.no
push dateTue, 02 Jan 2018 12:09:23 +0000
reviewersautomatedtester
bugs1379490
milestone59.0a1
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;