Bug 1311350 - Make close window commands synchronous and return remaining window handles. draft
authorHenrik Skupin <mail@hskupin.info>
Tue, 10 Jan 2017 16:36:49 +0100
changeset 458923 a38b812b8927b2d70dae328e98c6f7c0f17a193a
parent 458771 2963cf6be7f830c0d2155e2968cfc53585868a76
child 458924 7e23ab2505a45822bfa192adff6d134b274cb6db
push id41108
push userbmo:hskupin@gmail.com
push dateWed, 11 Jan 2017 09:47:10 +0000
bugs1311350
milestone53.0a1
Bug 1311350 - Make close window commands synchronous and return remaining window handles. To avoid a race condition for the close() commands Marionette has to wait until the current window/tab has actually been closed. To make this work we have to wait for the appropriate events to occur. Also the methods have to return the list of remaining window handles. MozReview-Commit-ID: DegcTJyKXCx
testing/marionette/browser.js
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/driver.js
--- a/testing/marionette/browser.js
+++ b/testing/marionette/browser.js
@@ -2,16 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const {utils: Cu} = Components;
 
 Cu.import("chrome://marionette/content/element.js");
+Cu.import("chrome://marionette/content/error.js");
 Cu.import("chrome://marionette/content/frame.js");
 
 this.EXPORTED_SYMBOLS = ["browser"];
 
 this.browser = {};
 
 const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
@@ -143,28 +144,60 @@ browser.Context = class {
         break;
 
       case "Fennec":
         this.browser = win.BrowserApp;
         break;
     }
   }
 
+  /**
+   * Close the current window.
+   *
+   * @return {Promise}
+   *     A promise which is resolved when the current window has been closed.
+   */
+  closeWindow() {
+    return new Promise(resolve => {
+      this.window.addEventListener("unload", ev => {
+        resolve();
+      }, {once: true});
+      this.window.close();
+    });
+  }
+
   /** Called when we start a session with this browser. */
   startSession(newSession, win, callback) {
     callback(win, newSession);
   }
 
-  /** Closes current tab. */
+  /**
+   * Close the current tab.
+   *
+   * @return {Promise}
+   *     A promise which is resolved when the current tab has been closed.
+   */
   closeTab() {
-    if (this.browser &&
-        this.browser.removeTab &&
-        this.tab !== null && (this.driver.appName != "B2G")) {
-      this.browser.removeTab(this.tab);
+    // If the current window is not a browser then close it directly. Do the
+    // same if only one remaining tab is open, or no tab selected at all.
+    if (!this.browser || !this.tab || this.browser.browsers.length == 1) {
+      return this.closeWindow();
     }
+
+    return new Promise((resolve, reject) => {
+      if (this.browser.removeTab) {
+        this.tab.addEventListener("TabClose", ev => {
+          resolve();
+        }, {once: true});
+        this.browser.removeTab(this.tab);
+      } else {
+        reject(new UnsupportedOperationError(
+            `closeTab() not supported in ${this.driver.appName}`));
+      }
+    });
   }
 
   /**
    * Opens a tab with given URI.
    *
    * @param {string} uri
    *      URI to open.
    */
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -1450,51 +1450,53 @@ class Marionette(object):
         If called in the content context it will return a list of
         references to all available browser windows.  Called in the
         chrome context, it will list all available windows, not just
         browser windows (e.g. not just navigator.browser).
 
         Each window handle is assigned by the server, and the list of
         strings returned does not have a guaranteed ordering.
 
-        :returns: unordered list of unique window handles as strings
+        :returns: Unordered list of unique window handles as strings
         """
         return self._send_message(
             "getWindowHandles", key="value" if self.protocol == 1 else None)
 
     @property
     def chrome_window_handles(self):
         """Get a list of currently open chrome windows.
 
         Each window handle is assigned by the server, and the list of
         strings returned does not have a guaranteed ordering.
 
-        :returns: unordered list of unique window handles as strings
+        :returns: Unordered list of unique chrome window handles as strings
         """
         return self._send_message(
             "getChromeWindowHandles", key="value" if self.protocol == 1 else None)
 
     @property
     def page_source(self):
         """A string representation of the DOM."""
         return self._send_message("getPageSource", key="value")
 
     def close(self):
         """Close the current window, ending the session if it's the last
         window currently open.
 
+        :returns: Unordered list of remaining unique window handles as strings
         """
-        self._send_message("close")
+        return self._send_message("close")
 
     def close_chrome_window(self):
         """Close the currently selected chrome window, ending the session
         if it's the last window open.
 
+        :returns: Unordered list of remaining unique chrome window handles as strings
         """
-        self._send_message("closeChromeWindow")
+        return self._send_message("closeChromeWindow")
 
     def set_context(self, context):
         """Sets the context that Marionette commands are running in.
 
         :param context: Context, may be one of the class properties
             `CONTEXT_CHROME` or `CONTEXT_CONTENT`.
 
         Usage example::
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -157,16 +157,83 @@ this.GeckoDriver = function (appName, se
 };
 
 Object.defineProperty(GeckoDriver.prototype, "a11yChecks", {
   get: function () {
     return this.capabilities.get("moz:accessibilityChecks");
   }
 });
 
+Object.defineProperty(GeckoDriver.prototype, "proxy", {
+  get: function () {
+    return this.capabilities.get("proxy");
+  }
+});
+
+Object.defineProperty(GeckoDriver.prototype, "secureTLS", {
+  get: function () {
+    return !this.capabilities.get("acceptInsecureCerts");
+  }
+});
+
+Object.defineProperty(GeckoDriver.prototype, "timeouts", {
+  get: function () {
+    return this.capabilities.get("timeouts");
+  },
+
+  set: function (newTimeouts) {
+    this.capabilities.set("timeouts", newTimeouts);
+  },
+});
+
+Object.defineProperty(GeckoDriver.prototype, "windowHandles", {
+  get: function () {
+    let hs = [];
+    let winEn = Services.wm.getEnumerator(null);
+
+    while (winEn.hasMoreElements()) {
+      let win = winEn.getNext();
+      if (win.gBrowser) {
+        let tabbrowser = win.gBrowser;
+        for (let i = 0; i < tabbrowser.browsers.length; ++i) {
+          let winId = this.getIdForBrowser(tabbrowser.getBrowserAtIndex(i));
+          if (winId !== null) {
+            hs.push(winId);
+          }
+        }
+      } else {
+        // For other chrome windows beside the browser window, only count the window itself.
+        let winId = win.QueryInterface(Ci.nsIInterfaceRequestor)
+            .getInterface(Ci.nsIDOMWindowUtils)
+            .outerWindowID;
+        hs.push(winId.toString());
+      }
+    }
+
+    return hs;
+  },
+});
+
+Object.defineProperty(GeckoDriver.prototype, "chromeWindowHandles", {
+  get : function () {
+    let hs = [];
+    let winEn = Services.wm.getEnumerator(null);
+
+    while (winEn.hasMoreElements()) {
+      let foundWin = winEn.getNext();
+      let winId = foundWin.QueryInterface(Ci.nsIInterfaceRequestor)
+          .getInterface(Ci.nsIDOMWindowUtils)
+          .outerWindowID;
+      hs.push(winId.toString());
+    }
+
+    return hs;
+  },
+});
+
 GeckoDriver.prototype.QueryInterface = XPCOMUtils.generateQI([
   Ci.nsIMessageListener,
   Ci.nsIObserver,
   Ci.nsISupportsWeakReference,
 ]);
 
 /**
  * Switches to the global ChromeMessageBroadcaster, potentially replacing
@@ -486,38 +553,16 @@ GeckoDriver.prototype.listeningPromise =
     let cb = () => {
       this.mm.removeMessageListener(li, cb);
       resolve();
     };
     this.mm.addMessageListener(li, cb);
   });
 };
 
-Object.defineProperty(GeckoDriver.prototype, "timeouts", {
-  get: function () {
-    return this.capabilities.get("timeouts");
-  },
-
-  set: function (newTimeouts) {
-    this.capabilities.set("timeouts", newTimeouts);
-  },
-});
-
-Object.defineProperty(GeckoDriver.prototype, "secureTLS", {
-  get: function () {
-    return !this.capabilities.get("acceptInsecureCerts");
-  }
-});
-
-Object.defineProperty(GeckoDriver.prototype, "proxy", {
-  get: function () {
-    return this.capabilities.get("proxy");
-  }
-});
-
 /** Create a new session. */
 GeckoDriver.prototype.newSession = function*(cmd, resp) {
   if (this.sessionId) {
     throw new SessionNotCreatedError("Maximum number of active sessions");
   }
 
   this.sessionId = cmd.parameters.sessionId ||
       cmd.parameters.session_id ||
@@ -1003,105 +1048,85 @@ GeckoDriver.prototype.goForward = functi
 /** Refresh the page. */
 GeckoDriver.prototype.refresh = function*(cmd, resp) {
   assert.content(this.context);
 
   yield this.listener.refresh();
 };
 
 /**
+ * Forces an update for the given browser's id.
+ */
+GeckoDriver.prototype.updateIdForBrowser = function (browser, newId) {
+  this._browserIds.set(browser.permanentKey, newId);
+};
+
+/**
+ * Retrieves a listener id for the given xul browser element. In case
+ * the browser is not known, an attempt is made to retrieve the id from
+ * a CPOW, and null is returned if this fails.
+ */
+GeckoDriver.prototype.getIdForBrowser = function (browser) {
+  if (browser === null) {
+    return null;
+  }
+  let permKey = browser.permanentKey;
+  if (this._browserIds.has(permKey)) {
+    return this._browserIds.get(permKey);
+  }
+
+  let winId = browser.outerWindowID;
+  if (winId) {
+    winId = winId.toString();
+    this._browserIds.set(permKey, winId);
+    return winId;
+  }
+  return null;
+},
+
+/**
  * Get the current window's handle. On desktop this typically corresponds
  * to the currently selected tab.
  *
  * Return an opaque server-assigned identifier to this window that
  * uniquely identifies it within this Marionette instance.  This can
  * be used to switch to this window at a later point.
  *
  * @return {string}
  *     Unique window handle.
  */
 GeckoDriver.prototype.getWindowHandle = function (cmd, resp) {
   // curFrameId always holds the current tab.
-  if (this.curBrowser.curFrameId && this.appName != "B2G") {
+  if (this.curBrowser.curFrameId) {
     resp.body.value = this.curBrowser.curFrameId;
     return;
   }
 
   for (let i in this.browsers) {
     if (this.curBrowser == this.browsers[i]) {
       resp.body.value = i;
       return;
     }
   }
 };
 
 /**
- * Forces an update for the given browser's id.
- */
-GeckoDriver.prototype.updateIdForBrowser = function (browser, newId) {
-  this._browserIds.set(browser.permanentKey, newId);
-};
-
-/**
- * Retrieves a listener id for the given xul browser element. In case
- * the browser is not known, an attempt is made to retrieve the id from
- * a CPOW, and null is returned if this fails.
- */
-GeckoDriver.prototype.getIdForBrowser = function getIdForBrowser(browser) {
-  if (browser === null) {
-    return null;
-  }
-  let permKey = browser.permanentKey;
-  if (this._browserIds.has(permKey)) {
-    return this._browserIds.get(permKey);
-  }
-
-  let winId = browser.outerWindowID;
-  if (winId) {
-    winId += "";
-    this._browserIds.set(permKey, winId);
-    return winId;
-  }
-  return null;
-},
-
-/**
- * Get a list of top-level browsing contexts.  On desktop this typically
- * corresponds to the set of open tabs.
+ * Get a list of top-level browsing contexts. On desktop this typically
+ * corresponds to the set of open tabs for browser windows, or the window itself
+ * for non-browser chrome windows.
  *
  * Each window handle is assigned by the server and is guaranteed unique,
  * however the return array does not have a specified ordering.
  *
  * @return {Array.<string>}
  *     Unique window handles.
  */
 GeckoDriver.prototype.getWindowHandles = function (cmd, resp) {
-  let hs = [];
-  let winEn = Services.wm.getEnumerator(null);
-  while (winEn.hasMoreElements()) {
-    let win = winEn.getNext();
-    if (win.gBrowser && this.appName != "B2G") {
-      let tabbrowser = win.gBrowser;
-      for (let i = 0; i < tabbrowser.browsers.length; ++i) {
-        let winId = this.getIdForBrowser(tabbrowser.getBrowserAtIndex(i));
-        if (winId !== null) {
-          hs.push(winId);
-        }
-      }
-    } else {
-      // XUL Windows, at least, do not have gBrowser
-      let winId = win.QueryInterface(Ci.nsIInterfaceRequestor)
-          .getInterface(Ci.nsIDOMWindowUtils)
-          .outerWindowID;
-      winId += (this.appName == "B2G") ? "-b2g" : "";
-      hs.push(winId);
-    }
-  }
-  resp.body = hs;
-};
+  return this.windowHandles;
+}
 
 /**
  * Get the current window's handle.  This corresponds to a window that
  * may itself contain tabs.
  *
  * Return an opaque server-assigned identifier to this window that
  * uniquely identifies it within this Marionette instance.  This can
  * be used to switch to this window at a later point.
@@ -1121,28 +1146,18 @@ GeckoDriver.prototype.getChromeWindowHan
 /**
  * Returns identifiers for each open chrome window for tests interested in
  * managing a set of chrome windows and tabs separately.
  *
  * @return {Array.<string>}
  *     Unique window handles.
  */
 GeckoDriver.prototype.getChromeWindowHandles = function (cmd, resp) {
-  let hs = [];
-  let winEn = Services.wm.getEnumerator(null);
-  while (winEn.hasMoreElements()) {
-    let foundWin = winEn.getNext();
-    let winId = foundWin.QueryInterface(Ci.nsIInterfaceRequestor)
-        .getInterface(Ci.nsIDOMWindowUtils)
-        .outerWindowID;
-    winId = winId + ((this.appName == "B2G") ? "-b2g" : "");
-    hs.push(winId);
-  }
-  resp.body = hs;
-};
+  return this.chromeWindowHandles;
+}
 
 /**
  * Get the current window position.
  *
  * @return {Object.<string, number>}
  *     Object with |x| and |y| coordinates.
  */
 GeckoDriver.prototype.getWindowPosition = function (cmd, resp) {
@@ -2076,96 +2091,92 @@ GeckoDriver.prototype.deleteCookie = fun
     return true;
   };
 
   this.mm.addMessageListener("Marionette:deleteCookie", cb);
   yield this.listener.deleteCookie(cmd.parameters.name);
 };
 
 /**
- * Close the current window, ending the session if it's the last
- * window currently open.
+ * Close the currently selected tab/window.
  *
- * On B2G this method is a noop and will return immediately.
+ * With multiple open tabs present the currently selected tab will be closed.
+ * Otherwise the window itself will be closed. If it is the last window
+ * currently open, the window will not be closed to prevent a shutdown of the
+ * application. Instead the returned list of window handles is empty.
+ *
+ * @return {Array.<string>}
+ *     Unique window handles of remaining windows.
  */
 GeckoDriver.prototype.close = function (cmd, resp) {
-  // can't close windows on B2G
-  if (this.appName == "B2G") {
-    return;
-  }
-
   let nwins = 0;
   let winEn = Services.wm.getEnumerator(null);
+
   while (winEn.hasMoreElements()) {
     let win = winEn.getNext();
 
-    // count both windows and tabs
+    // For browser windows count the tabs. Otherwise take the window itself.
     if (win.gBrowser) {
       nwins += win.gBrowser.browsers.length;
     } else {
       nwins++;
     }
   }
 
-  // if there is only 1 window left, delete the session
+  // If there is only 1 window left, do not close it. Instead return a faked
+  // empty array of window handles. This will instruct geckodriver to terminate
+  // the application.
   if (nwins == 1) {
-    this.sessionTearDown();
-    return;
+    return [];
   }
 
-  try {
-    if (this.mm != globalMessageManager) {
-      this.mm.removeDelayedFrameScript(FRAME_SCRIPT);
-    }
+  if (this.mm != globalMessageManager) {
+    this.mm.removeDelayedFrameScript(FRAME_SCRIPT);
+  }
 
-    if (this.curBrowser.tab) {
-      this.curBrowser.closeTab();
-    } else {
-      this.getCurrentWindow().close();
-    }
-  } catch (e) {
-    throw new UnknownError(`Could not close window: ${e.message}`);
-  }
+  return this.curBrowser.closeTab().then(() => this.windowHandles);
 };
 
 /**
- * Close the currently selected chrome window, ending the session if it's the last
- * window currently open.
+ * Close the currently selected chrome window.
  *
- * On B2G this method is a noop and will return immediately.
+ * If it is the last window currently open, the chrome window will not be
+ * closed to prevent a shutdown of the application. Instead the returned
+ * list of chrome window handles is empty.
+ *
+ * @return {Array.<string>}
+ *     Unique chrome window handles of remaining chrome windows.
  */
 GeckoDriver.prototype.closeChromeWindow = function (cmd, resp) {
-  // can't close windows on B2G
-  if (this.appName == "B2G") {
-    return;
-  }
+  assert.firefox();
 
   // Get the total number of windows
   let nwins = 0;
   let winEn = Services.wm.getEnumerator(null);
+
   while (winEn.hasMoreElements()) {
     nwins++;
     winEn.getNext();
   }
 
-  // if there is only 1 window left, delete the session
+  // If there is only 1 window left, do not close it. Instead return a faked
+  // empty array of window handles. This will instruct geckodriver to terminate
+  // the application.
   if (nwins == 1) {
-    this.sessionTearDown();
-    return;
+    return [];
   }
 
-  try {
-    // reset frame to the top-most frame
-    this.curFrame = null;
+  // reset frame to the top-most frame
+  this.curFrame = null;
 
+  if (this.mm != globalMessageManager) {
     this.mm.removeDelayedFrameScript(FRAME_SCRIPT);
-    this.getCurrentWindow().close();
-  } catch (e) {
-    throw new UnknownError(`Could not close window: ${e.message}`);
   }
+
+  return this.curBrowser.closeWindow().then(() => this.chromeWindowHandles);
 };
 
 /**
  * Deletes the session.
  *
  * If it is a desktop environment, it will close all listeners.
  *
  * If it is a B2G environment, it will make the main content listener