commit 1cb031c1390abcb54c059e596e60fd1b8e3abd58
parent 116b1706e6db02978586c01be0285e85f71f83d2
Author: Henrik Skupin <mail@hskupin.info>
Date: Tue, 23 Dec 2025 11:36:51 +0000
Bug 2002949 - [remote] Don't share the clientWindow ids of the WindowManager with Marionette. r=Sasha
With WebDriver BiDi the clientWindow is referring to a ChromeWindow
instance while in Marionette a window is a top-level browsing
context. As such the same ids should not be shared, but Marionette
needs to rely on Navigable ids based on the top-level browsing
context.
Differential Revision: https://phabricator.services.mozilla.com/D277074
Diffstat:
9 files changed, 265 insertions(+), 113 deletions(-)
diff --git a/remote/marionette/driver.sys.mjs b/remote/marionette/driver.sys.mjs
@@ -597,7 +597,9 @@ GeckoDriver.prototype.isReftestBrowser = function (element) {
*/
GeckoDriver.prototype.addBrowser = function (win) {
let context = new lazy.browser.Context(win, this);
- let winId = lazy.windowManager.getIdForWindow(win);
+ let winId = lazy.NavigableManager.getIdForBrowsingContext(
+ win.browsingContext
+ );
this.browsers[winId] = context;
this.curBrowser = this.browsers[winId];
@@ -1393,7 +1395,9 @@ GeckoDriver.prototype.getWindowHandle = function () {
lazy.assert.open(this.getBrowsingContext({ top: true }));
if (this.context == lazy.Context.Chrome) {
- return lazy.windowManager.getIdForWindow(this.curBrowser.window);
+ return lazy.NavigableManager.getIdForBrowsingContext(
+ this.currentSession.chromeBrowsingContext
+ );
}
return this.curBrowser.contentBrowserId;
@@ -1416,7 +1420,7 @@ GeckoDriver.prototype.getWindowHandle = function () {
GeckoDriver.prototype.getWindowHandles = function () {
if (this.context == lazy.Context.Chrome) {
return lazy.windowManager.windows.map(window =>
- lazy.windowManager.getIdForWindow(window)
+ lazy.NavigableManager.getIdForBrowsingContext(window.browsingContext)
);
}
@@ -1544,7 +1548,7 @@ GeckoDriver.prototype._findWindowByHandle = function (handle) {
win.browsingContext
);
if (chromeWindowId == handle) {
- return lazy.windowManager.getWindowProperties(win);
+ return this.getWindowProperties(win);
}
// Otherwise check if the chrome window has a tab browser, and that it
@@ -1559,7 +1563,7 @@ GeckoDriver.prototype._findWindowByHandle = function (handle) {
lazy.NavigableManager.getIdForBrowser(contentBrowser);
if (contentWindowId == handle) {
- return lazy.windowManager.getWindowProperties(win, { tabIndex: i });
+ return this.getWindowProperties(win, { tabIndex: i });
}
}
}
@@ -1617,6 +1621,50 @@ GeckoDriver.prototype.switchToWindow = async function (cmd) {
};
/**
+ * A set of properties that describe a window and allow it to be uniquely
+ * identified. The described window can either be a Chrome Window or a
+ * Content Window.
+ *
+ * @typedef {object} WindowProperties
+ * @property {Window} win
+ * The Chrome Window containing the window. When describing
+ * a Chrome Window, this is the window itself.
+ * @property {string} id
+ * The unique id of the containing Chrome Window.
+ * @property {boolean} hasTabBrowser
+ * `true` if the Chrome Window has a tabBrowser.
+ * @property {number=} tabIndex
+ * Optional, the index of the specific tab within the window.
+ */
+
+/**
+ * Returns a WindowProperties object, that can be used with :js:func:`GeckoDriver#setWindowHandle`.
+ *
+ * @param {Window} win
+ * The Chrome Window for which we want to create a properties object.
+ * @param {object=} options
+ * @param {number} options.tabIndex
+ * Tab index of a specific Content Window in the specified Chrome Window.
+ *
+ * @returns {WindowProperties}
+ * A window properties object.
+ */
+GeckoDriver.prototype.getWindowProperties = function (win, options = {}) {
+ const { tabIndex } = options;
+
+ if (!Window.isInstance(win)) {
+ throw new TypeError("Invalid argument, expected a Window object");
+ }
+
+ return {
+ win,
+ id: lazy.NavigableManager.getIdForBrowsingContext(win.browsingContext),
+ hasTabBrowser: !!lazy.TabManager.getTabBrowser(win),
+ tabIndex,
+ };
+};
+
+/**
* Switch the marionette window to a given window. If the browser in
* the window is unregistered, register that browser and wait for
* the registration is complete. If |focus| is true then set the focus
diff --git a/remote/marionette/reftest.sys.mjs b/remote/marionette/reftest.sys.mjs
@@ -15,7 +15,6 @@ ChromeUtils.defineESModuleGetters(lazy, {
Log: "chrome://remote/content/shared/Log.sys.mjs",
navigate: "chrome://remote/content/marionette/navigate.sys.mjs",
print: "chrome://remote/content/shared/PDF.sys.mjs",
- windowManager: "chrome://remote/content/shared/WindowManager.sys.mjs",
});
ChromeUtils.defineLazyGetter(lazy, "logger", () =>
@@ -170,7 +169,7 @@ reftest.Runner = class {
this.windowUtils = reftestWin.windowUtils;
this.reftestWin = reftestWin;
- let windowHandle = lazy.windowManager.getWindowProperties(reftestWin);
+ let windowHandle = this.driver.getWindowProperties(reftestWin);
await this.driver.setWindowHandle(windowHandle, true);
const url = await this.driver._getCurrentURL();
@@ -243,9 +242,7 @@ reftest.Runner = class {
async abort() {
if (this.reftestWin && this.reftestWin != this.parentWindow) {
await this.driver.closeChromeWindow();
- let parentHandle = lazy.windowManager.getWindowProperties(
- this.parentWindow
- );
+ let parentHandle = this.driver.getWindowProperties(this.parentWindow);
await this.driver.setWindowHandle(parentHandle);
}
this.reftestWin = null;
diff --git a/remote/shared/WindowManager.sys.mjs b/remote/shared/WindowManager.sys.mjs
@@ -9,14 +9,15 @@ ChromeUtils.defineESModuleGetters(lazy, {
AnimationFramePromise: "chrome://remote/content/shared/Sync.sys.mjs",
AppInfo: "chrome://remote/content/shared/AppInfo.sys.mjs",
+ BiMap: "chrome://remote/content/shared/BiMap.sys.mjs",
BrowsingContextListener:
"chrome://remote/content/shared/listeners/BrowsingContextListener.sys.mjs",
+ ChromeWindowListener:
+ "chrome://remote/content/shared/listeners/ChromeWindowListener.sys.mjs",
DebounceCallback: "chrome://remote/content/marionette/sync.sys.mjs",
error: "chrome://remote/content/shared/webdriver/Errors.sys.mjs",
EventPromise: "chrome://remote/content/shared/Sync.sys.mjs",
Log: "chrome://remote/content/shared/Log.sys.mjs",
- NavigableManager: "chrome://remote/content/shared/NavigableManager.sys.mjs",
- TabManager: "chrome://remote/content/shared/TabManager.sys.mjs",
TimedPromise: "chrome://remote/content/shared/Sync.sys.mjs",
UserContextManager:
"chrome://remote/content/shared/UserContextManager.sys.mjs",
@@ -35,8 +36,11 @@ const TIMEOUT_NO_WINDOW_MANAGER = 5000;
* @class WindowManager
*/
class WindowManager {
+ #chromeWindowListener;
#clientWindowIds;
#contextListener;
+ #contextToWindowMap;
+ #tracking;
constructor() {
/**
@@ -44,11 +48,58 @@ class WindowManager {
* contextDestroyed event is fired, the context is already destroyed so
* we cannot query for the client window at that time.
*/
- this.#clientWindowIds = new WeakMap();
+ this.#clientWindowIds = new lazy.BiMap();
+ // For content browsing contexts, the embedder element may already be
+ // gone by the time when it is getting discarded. To ensure we can still
+ // retrieve the corresponding chrome window, we maintain a mapping from
+ // each top-level content browsing context to its chrome window.
+ this.#contextToWindowMap = new WeakMap();
this.#contextListener = new lazy.BrowsingContextListener();
+
+ this.#tracking = false;
+
+ this.#chromeWindowListener = new lazy.ChromeWindowListener();
+ }
+
+ destroy() {
+ this.stopTracking();
+ }
+
+ startTracking() {
+ if (this.#tracking) {
+ return;
+ }
+
+ this.#chromeWindowListener.on("closed", this.#onChromeWindowClosed);
+ this.#chromeWindowListener.on("opened", this.#onChromeWindowOpened);
+ this.#chromeWindowListener.startListening();
+
this.#contextListener.on("attached", this.#onContextAttached);
this.#contextListener.startListening();
+
+ // Pre-fill the internal window id mapping.
+ this.windows.forEach(window => this.getIdForWindow(window));
+
+ this.#tracking = true;
+ }
+
+ stopTracking() {
+ if (!this.#tracking) {
+ return;
+ }
+
+ this.#chromeWindowListener.stopListening();
+ this.#chromeWindowListener.off("closed", this.#onChromeWindowClosed);
+ this.#chromeWindowListener.off("opened", this.#onChromeWindowOpened);
+
+ this.#contextListener.stopListening();
+ this.#contextListener.off("attached", this.#onContextAttached);
+
+ this.#clientWindowIds = new lazy.BiMap();
+ this.#contextToWindowMap = new WeakMap();
+
+ this.#tracking = false;
}
/**
@@ -71,71 +122,36 @@ class WindowManager {
}
/**
- * A set of properties describing a window and that should allow to uniquely
- * identify it. The described window can either be a Chrome Window or a
- * Content Window.
+ * Retrieves an id for the given chrome window. The id is a dynamically
+ * generated uuid by the WindowManager and associated with the
+ * top-level browsing context of that chrome window.
*
- * @typedef {object} WindowProperties
- * @property {Window} win - The Chrome Window containing the window.
- * When describing a Chrome Window, this is the window itself.
- * @property {string} id - The unique id of the containing Chrome Window.
- * @property {boolean} hasTabBrowser - `true` if the Chrome Window has a
- * tabBrowser.
- * @property {number} tabIndex - Optional, the index of the specific tab
- * within the window.
- */
-
- /**
- * Returns a WindowProperties object, that can be used with :js:func:`GeckoDriver#setWindowHandle`.
+ * @param {ChromeWindow} win
+ * The chrome window for which we want to retrieve the id.
*
- * @param {Window} win
- * The Chrome Window for which we want to create a properties object.
- * @param {object} options
- * @param {number} options.tabIndex
- * Tab index of a specific Content Window in the specified Chrome Window.
- * @returns {WindowProperties} A window properties object.
+ * @returns {string|null}
+ * The unique id for this chrome window or `null` if not a valid window.
*/
- getWindowProperties(win, options = {}) {
- if (!Window.isInstance(win)) {
- throw new TypeError("Invalid argument, expected a Window object");
+ getIdForWindow(win) {
+ if (win) {
+ return this.#clientWindowIds.getOrInsert(win);
}
- return {
- win,
- id: this.getIdForWindow(win),
- hasTabBrowser: !!lazy.TabManager.getTabBrowser(win),
- tabIndex: options.tabIndex,
- };
+ return null;
}
/**
- * Returns the window ID for a specific browsing context.
+ * Retrieve the Chrome Window corresponding to the provided window id.
*
- * @param {BrowsingContext} context
- * The browsing context for which we want to retrieve the window ID.
+ * @param {string} id
+ * A unique id for the chrome window.
*
- * @returns {(string|undefined)}
- * The ID of the window associated with the browsing context.
+ * @returns {ChromeWindow|undefined}
+ * The chrome window found for this id, `null` if none
+ * was found.
*/
- getIdForBrowsingContext(context) {
- const window = this.getWindowForBrowsingContext(context);
-
- return window
- ? this.getIdForWindow(window)
- : this.#clientWindowIds.get(context);
- }
-
- /**
- * Retrieves an id for the given chrome window. The id is a dynamically
- * generated uuid by the NavigableManager and associated with the
- * top-level browsing context of that chrome window.
- *
- * @param {window} win
- * The chrome window for which we want to retrieve the id.
- * @returns {string} The unique id for this chrome window.
- */
- getIdForWindow(win) {
- return lazy.NavigableManager.getIdForBrowsingContext(win.browsingContext);
+ getWindowById(id) {
+ return this.#clientWindowIds.getObject(id);
}
/**
@@ -285,18 +301,26 @@ class WindowManager {
}
/**
- * Returns the window for a specific browsing context.
+ * Returns the chrome window for a specific browsing context.
*
* @param {BrowsingContext} context
* The browsing context for which we want to retrieve the window.
*
- * @returns {ChromeWindow}
+ * @returns {ChromeWindow|null}
* The chrome window associated with the browsing context.
+ * Otherwise `null` is returned.
*/
- getWindowForBrowsingContext(context) {
- return lazy.AppInfo.isAndroid
- ? context.top.embedderElement?.ownerGlobal
- : context.topChromeWindow;
+ getChromeWindowForBrowsingContext(context) {
+ if (!context.isContent) {
+ // Chrome browsing contexts always have a chrome window set.
+ return context.topChromeWindow;
+ }
+
+ if (this.#contextToWindowMap.has(context.top)) {
+ return this.#contextToWindowMap.get(context.top);
+ }
+
+ return this.#setChromeWindowForBrowsingContext(context);
}
/**
@@ -480,16 +504,37 @@ class WindowManager {
);
}
+ #setChromeWindowForBrowsingContext(context) {
+ const chromeWindow = context.top.embedderElement?.ownerGlobal;
+ if (chromeWindow) {
+ return this.#contextToWindowMap.getOrInsert(context.top, chromeWindow);
+ }
+
+ return null;
+ }
+
+ /* Event handlers */
+
#onContextAttached = (_, data = {}) => {
const { browsingContext } = data;
- const window = this.getWindowForBrowsingContext(browsingContext);
- if (!window) {
- // Short-lived iframes might already be disconnected from their parent
- // window.
+ if (!browsingContext.isContent) {
return;
}
- this.#clientWindowIds.set(browsingContext, this.getIdForWindow(window));
+
+ this.#setChromeWindowForBrowsingContext(browsingContext);
+ };
+
+ #onChromeWindowClosed = (_, data = {}) => {
+ const { window } = data;
+
+ this.#clientWindowIds.deleteByObject(window);
+ };
+
+ #onChromeWindowOpened = (_, data = {}) => {
+ const { window } = data;
+
+ this.getIdForWindow(window);
};
}
diff --git a/remote/shared/test/browser/browser_NavigableManager.js b/remote/shared/test/browser/browser_NavigableManager.js
@@ -15,10 +15,6 @@ const FRAME_MARKUP = `
`;
const TEST_URL = BUILDER_URL + encodeURI(FRAME_MARKUP);
-const numberRegex = /[0-9]+/i;
-const uuidRegex =
- /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
-
describe("NavigableManager", function () {
let testData;
diff --git a/remote/shared/test/browser/browser_WindowManager.js b/remote/shared/test/browser/browser_WindowManager.js
@@ -105,30 +105,6 @@ add_task(async function test_adjustWindowGeometry_invalid_values() {
}
});
-add_task(async function test_windows() {
- const win1 = await BrowserTestUtils.openNewBrowserWindow();
- const win2 = await BrowserTestUtils.openNewBrowserWindow();
- const win3 = await BrowserTestUtils.openNewBrowserWindow();
-
- const expectedWindows = [gBrowser.ownerGlobal, win1, win2, win3];
-
- try {
- is(
- windowManager.windows.length,
- 5,
- "All browser windows and the Mochikit harness window were returned"
- );
- ok(
- expectedWindows.every(win => windowManager.windows.includes(win)),
- "Expected windows were returned"
- );
- } finally {
- await BrowserTestUtils.closeWindow(win3);
- await BrowserTestUtils.closeWindow(win2);
- await BrowserTestUtils.closeWindow(win1);
- }
-});
-
add_task(async function test_minimizeWindow() {
const testWin = await BrowserTestUtils.openNewBrowserWindow();
@@ -267,3 +243,88 @@ add_task(async function test_setFullscreen() {
await BrowserTestUtils.closeWindow(testWin);
}
});
+
+add_task(async function test_windows() {
+ const win1 = await BrowserTestUtils.openNewBrowserWindow();
+ const win2 = await BrowserTestUtils.openNewBrowserWindow();
+ const win3 = await BrowserTestUtils.openNewBrowserWindow();
+
+ const expectedWindows = [gBrowser.ownerGlobal, win1, win2, win3];
+
+ try {
+ is(
+ windowManager.windows.length,
+ 5,
+ "All browser windows and the Mochikit harness window were returned"
+ );
+ ok(
+ expectedWindows.every(win => windowManager.windows.includes(win)),
+ "Expected windows were returned"
+ );
+ } finally {
+ await BrowserTestUtils.closeWindow(win3);
+ await BrowserTestUtils.closeWindow(win2);
+ await BrowserTestUtils.closeWindow(win1);
+ }
+});
+
+add_task(async function test_getIdForWindow() {
+ const win1 = await BrowserTestUtils.openNewBrowserWindow();
+ const win2 = await BrowserTestUtils.openNewBrowserWindow();
+
+ try {
+ windowManager.startTracking();
+
+ const win1Id = windowManager.getIdForWindow(win1);
+ Assert.stringMatches(
+ win1Id,
+ uuidRegex,
+ "The first window id is a valid UUID"
+ );
+ is(
+ windowManager.getIdForWindow(win1),
+ win1Id,
+ "getIdForWindow returns the same id when called multiple times for the same window"
+ );
+
+ const win2Id = windowManager.getIdForWindow(win2);
+ Assert.stringMatches(
+ win2Id,
+ uuidRegex,
+ "The second window id is a valid UUID"
+ );
+ isnot(
+ win1Id,
+ win2Id,
+ "getIdForWindow returns different ids for different windows"
+ );
+ } finally {
+ await BrowserTestUtils.closeWindow(win2);
+ await BrowserTestUtils.closeWindow(win1);
+
+ windowManager.destroy();
+ }
+});
+
+add_task(async function test_getWindowById() {
+ windowManager.startTracking();
+ const win = await BrowserTestUtils.openNewBrowserWindow();
+
+ try {
+ const winId = windowManager.getIdForWindow(win);
+ is(
+ windowManager.getWindowById(winId),
+ win,
+ "getWindowById returns the correct window for a valid id"
+ );
+ is(
+ windowManager.getWindowById("non-existent-id"),
+ undefined,
+ "getWindowById returns undefined for a non-existent id"
+ );
+ } finally {
+ await BrowserTestUtils.closeWindow(win);
+
+ windowManager.destroy();
+ }
+});
diff --git a/remote/shared/test/browser/head.js b/remote/shared/test/browser/head.js
@@ -8,6 +8,10 @@ const { NavigationManager } = ChromeUtils.importESModule(
"chrome://remote/content/shared/NavigationManager.sys.mjs"
);
+const numberRegex = /[0-9]+/i;
+const uuidRegex =
+ /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+
/**
* Add a new tab in a given browser, pointing to a given URL and automatically
* register the cleanup function to remove it at the end of the test.
diff --git a/remote/shared/webdriver/Session.sys.mjs b/remote/shared/webdriver/Session.sys.mjs
@@ -32,6 +32,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
"chrome://remote/content/webdriver-bidi/WebDriverBiDiConnection.sys.mjs",
WebSocketHandshake:
"chrome://remote/content/server/WebSocketHandshake.sys.mjs",
+ windowManager: "chrome://remote/content/shared/WindowManager.sys.mjs",
});
ChromeUtils.defineLazyGetter(lazy, "logger", () => lazy.Log.get());
@@ -295,6 +296,7 @@ export class WebDriverSession {
// Start the tracking of browsing contexts to create Navigable ids.
lazy.NavigableManager.startTracking();
+ lazy.windowManager.startTracking();
webDriverSessions.set(this.#id, this);
}
@@ -305,6 +307,7 @@ export class WebDriverSession {
// Stop the tracking of browsing contexts when no WebDriver
// session exists anymore.
lazy.NavigableManager.stopTracking();
+ lazy.windowManager.stopTracking();
lazy.unregisterProcessDataActor();
diff --git a/remote/webdriver-bidi/modules/root/browsingContext.sys.mjs b/remote/webdriver-bidi/modules/root/browsingContext.sys.mjs
@@ -2615,11 +2615,14 @@ export const getBrowsingContextInfo = (context, options = {}) => {
);
}
- const userContext = lazy.UserContextManager.getIdByBrowsingContext(context);
+ const chromeWindow =
+ lazy.windowManager.getChromeWindowForBrowsingContext(context);
const originalOpener =
context.crossGroupOpener !== null
? lazy.NavigableManager.getIdForBrowsingContext(context.crossGroupOpener)
: null;
+ const userContext = lazy.UserContextManager.getIdByBrowsingContext(context);
+
const contextInfo = {
children,
context: lazy.NavigableManager.getIdForBrowsingContext(context),
@@ -2630,7 +2633,7 @@ export const getBrowsingContextInfo = (context, options = {}) => {
originalOpener: originalOpener === undefined ? null : originalOpener,
url: context.currentURI.spec,
userContext,
- clientWindow: lazy.windowManager.getIdForBrowsingContext(context),
+ clientWindow: lazy.windowManager.getIdForWindow(chromeWindow),
};
if (includeParentId) {
diff --git a/testing/web-platform/mozilla/meta/webdriver/bidi/browsing_context/get_tree/moz_scope.py.ini b/testing/web-platform/mozilla/meta/webdriver/bidi/browsing_context/get_tree/moz_scope.py.ini
@@ -1,9 +1,4 @@
[moz_scope.py]
- [test_multiple_browser_windows]
- bug: https://bugzilla.mozilla.org/show_bug.cgi?id=2004388
- expected:
- if os == "android": FAIL
-
[test_custom_chrome_window_without_iframes]
disabled:
if os == "android": https://bugzilla.mozilla.org/show_bug.cgi?id=1762066
