Bug 1680479 - [marionette] Use stable unique ids as window handles r=marionette-reviewers,webdriver-reviewers,whimboo

Differential Revision: https://phabricator.services.mozilla.com/D113879

testing/marionette/browser.js

@@ -424,13 +424,6 @@ browser.Context = class {
     if (!this.tab) {
       this.switchToTab();
     }
-
-    if (target === this.contentBrowser) {
-      // Note that browsing contexts can be swapped during navigation in which
-      // case this id would no longer match the target. See Bug 1680479.
-      const uid = target.browsingContext.id;
-      windowManager.updateIdForBrowser(this.contentBrowser, uid);
-    }
   }
 };
 

testing/marionette/driver.js

@@ -620,14 +620,6 @@ GeckoDriver.prototype.handleEvent = function({ target, type }) {
         );
 
         this.currentSession.contentBrowsingContext = target.browsingContext;
-
-        // Manually update the stored browsing context id.
-        // Switching to browserId instead of browsingContext.id would make
-        // this call unnecessary. See Bug 1681973.
-        windowManager.updateIdForBrowser(
-          this.curBrowser.contentBrowser,
-          target.browsingContext.id
-        );
       }
       break;
   }
@@ -1087,14 +1079,14 @@ GeckoDriver.prototype.refresh = async function() {
  *     Top-level browsing context has been discarded.
  */
 GeckoDriver.prototype.getWindowHandle = function() {
-  const browsingContext = assert.open(
+  assert.open(
     this.getBrowsingContext({
       context: Context.Content,
       top: true,
     })
   );
 
-  return browsingContext.id.toString();
+  return windowManager.getIdForBrowser(this.curBrowser.contentBrowser);
 };
 
 /**
@@ -1129,14 +1121,14 @@ GeckoDriver.prototype.getWindowHandles = function() {
  *     Internal browsing context reference not found
  */
 GeckoDriver.prototype.getChromeWindowHandle = function() {
-  const browsingContext = assert.open(
+  assert.open(
     this.getBrowsingContext({
       context: Context.Chrome,
       top: true,
     })
   );
 
-  return browsingContext.id.toString();
+  return windowManager.getIdForWindow(this.curBrowser.window);
 };
 
 /**
@@ -1268,7 +1260,7 @@ GeckoDriver.prototype.switchToWindow = async function(cmd) {
   );
   assert.boolean(focus, pprint`Expected "focus" to be a boolean, got ${focus}`);
 
-  const found = windowManager.findWindowByHandle(parseInt(handle));
+  const found = windowManager.findWindowByHandle(handle);
 
   let selected = false;
   if (found) {

testing/marionette/harness/marionette_harness/runner/mixins/window_manager.py

@@ -97,8 +97,8 @@ class WindowManagerMixin(object):
                 return self.marionette.execute_script(
                     """
                   Components.utils.import("resource://gre/modules/Services.jsm");
-
-                  const win = BrowsingContext.get(Number(arguments[0])).window;
+                  const { windowManager } = ChromeUtils.import("chrome://marionette/content/window-manager.js");
+                  const win = windowManager.findWindowByHandle(arguments[0]).win;
                   return win.document.readyState == "complete";
                 """,
                     script_args=[handle],

testing/marionette/window-manager.js

@@ -22,6 +22,13 @@ XPCOMUtils.defineLazyModuleGetters(this, {
 
 XPCOMUtils.defineLazyGetter(this, "logger", () => Log.get());
 
+XPCOMUtils.defineLazyServiceGetter(
+  this,
+  "uuidGen",
+  "@mozilla.org/uuid-generator;1",
+  "nsIUUIDGenerator"
+);
+
 /**
  * Provides helpers to interact with Window objects.
  *
@@ -29,8 +36,10 @@ XPCOMUtils.defineLazyGetter(this, "logger", () => Log.get());
  */
 class WindowManager {
   constructor() {
-    // Maps permanentKey to browsing context id: WeakMap.<Object, number>
-    this._browserIds = new WeakMap();
+    // Maps browser's permanentKey to uuid: WeakMap.<Object, string>
+    this._windowHandles = new WeakMap();
+    // Maps ChromeWindow to uuid: WeakMap.<Object, string>
+    this._chromeWindowHandles = new WeakMap();
   }
 
   get windowHandles() {
@@ -70,7 +79,7 @@ class WindowManager {
   /**
    * Find a specific window matching the provided window handle.
    *
-   * @param {Number} handle
+   * @param {String} handle
    *     The unique handle of either a chrome window or a content browser, as
    *     returned by :js:func:`#getIdForBrowser` or :js:func:`#getIdForWindow`.
    *
@@ -135,55 +144,48 @@ class WindowManager {
 
     return {
       win,
-      id: win.browsingContext.id,
+      id: this.getIdForWindow(win),
       hasTabBrowser: !!browser.getTabBrowser(win),
       tabIndex: options.tabIndex,
     };
   }
 
   /**
-   * Forces an update for the given browser's id.
-   */
-  updateIdForBrowser(browserElement, newId) {
-    this._browserIds.set(browserElement.permanentKey, newId);
-  }
-
-  /**
-   * Retrieves an 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.
+   * Retrieves an id for the given xul browser element. The id is a dynamically
+   * generated uuid associated with the permanentKey property of the given
+   * browser element.
    *
    * @param {xul:browser} browserElement
    *     The <xul:browser> for which we want to retrieve the id.
-   * @return {Number} The unique id for this browser.
+   * @return {String} The unique id for this browser.
    */
   getIdForBrowser(browserElement) {
     if (browserElement === null) {
       return null;
     }
 
-    const permKey = browserElement.permanentKey;
-    if (this._browserIds.has(permKey)) {
-      return this._browserIds.get(permKey);
+    const key = browserElement.permanentKey;
+    if (!this._windowHandles.has(key)) {
+      const uuid = uuidGen.generateUUID().toString();
+      this._windowHandles.set(key, uuid.substring(1, uuid.length - 1));
     }
-
-    const winId = browserElement.browsingContext.id;
-    if (winId) {
-      this._browserIds.set(permKey, winId);
-      return winId;
-    }
-    return null;
+    return this._windowHandles.get(key);
   }
 
   /**
-   * Retrieves an id for the given chrome window.
+   * Retrieves an id for the given chrome window. The id is a dynamically
+   * generated uuid associated with the window object.
    *
    * @param {window} win
    *     The window object for which we want to retrieve the id.
-   * @return {Number} The unique id for this chrome window.
+   * @return {String} The unique id for this chrome window.
    */
   getIdForWindow(win) {
-    return win.browsingContext.id;
+    if (!this._chromeWindowHandles.has(win)) {
+      const uuid = uuidGen.generateUUID().toString();
+      this._chromeWindowHandles.set(win, uuid.substring(1, uuid.length - 1));
+    }
+    return this._chromeWindowHandles.get(win);
   }
 
   /**

testing/web-platform/tests/webdriver/tests/get_window_handle/get.py

@@ -1,3 +1,5 @@
+import pytest
+
 from tests.support.asserts import assert_error, assert_success
 
 
@@ -19,3 +21,23 @@ def test_no_browsing_context(session, closed_frame):
 def test_basic(session):
     response = get_window_handle(session)
     assert_success(response, session.window_handle)
+
+
+# Capability needed as long as no valid certificate is available:
+#   https://github.com/web-platform-tests/wpt/issues/28847
+@pytest.mark.capabilities({"acceptInsecureCerts": True})
+def test_navigation_with_coop_headers(session, url):
+    base_path = ("/webdriver/tests/support/html/subframe.html" +
+                 "?pipe=header(Cross-Origin-Opener-Policy,same-origin")
+
+    session.url = url(base_path, protocol="https")
+    response = get_window_handle(session)
+    first_handle = assert_success(response)
+
+    # navigating to another domain with COOP headers will force a process change
+    # in most browsers
+    session.url = url(base_path, protocol="https", domain="alt")
+    response = get_window_handle(session)
+    second_handle = assert_success(response)
+
+    assert first_handle == second_handle