Bug 1311350 - Make close window commands synchronous and return remaining window handles. draft
authorHenrik Skupin <mail@hskupin.info>
Mon, 09 Jan 2017 22:51:04 +0100
changeset 458398 f86d6c3986d0221e8becda742b6bdf35b643bf1f
parent 457660 2977ca1224525680cbfb5c3ce3018818b6dfd8f2
child 458399 9fefaf3b5377b40ce697f6afdd7f326010d2b4f3
child 458421 c43dec1bdd0ded4fda0bba53dd20219a2642d208
push id40942
push userbmo:hskupin@gmail.com
push dateTue, 10 Jan 2017 07:43:51 +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
@@ -1,16 +1,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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("resource://gre/modules/Log.jsm");
+const logger = Log.repository.getLogger("Marionette");
+
 Cu.import("chrome://marionette/content/element.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 +146,59 @@ 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 Error(`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
@@ -1477,24 +1477,24 @@ class Marionette(object):
         """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.
 
         """
-        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.
 
         """
-        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, "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;
+  },
+});
+
+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;
+  },
+});
+
 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 ||
@@ -1015,17 +1060,17 @@ GeckoDriver.prototype.refresh = function
  * 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;
@@ -1040,68 +1085,47 @@ GeckoDriver.prototype.updateIdForBrowser
   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) {
+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 += "";
-    this._browserIds.set(permKey, winId);
-    return winId;
+    this._browserIds.set(permKey, winId.toString());
+    return winId.toString();
   }
   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;
-};
+  resp.body = 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.
@@ -2076,59 +2100,50 @@ 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 current window.
  *
- * On B2G this method is a noop and will return immediately.
+ * If it was the last window currently open, the session will be closed.
+ *
+ * @return {Array.<string>}
+ *     Unique window handles as detected by getWindowHandles command.
  */
 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 instead
     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 session.
   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.
  *
  * On B2G this method is a noop and will return immediately.
  */