commit 2dae50a6324ccf9b41d32a3487a98de18963b0d6
parent 300c9edaeee389d7182e1c66552f527316824931
Author: Makoto Kato <m_kato@ga2.so-net.ne.jp>
Date: Mon, 10 Nov 2025 15:20:14 +0000
Bug 1990446 - Refactor nsIBrowserDOMWindow methods to merge multiple event loops. r=geckoview-reviewers,ohall,owlish
`nsIBrowserDOMWindow` methods aren't asynchronous, so we spin multiple
event loops since GeckoView delegate API runs on Android UI thread.
It is unnecessary to spin multiple event loops here, so this fix merges
multiple event loops into one.
Also, `open-features-tokenization-noreferrer.html` WPT opens over 40
windows, then close it soon. This test is failed since parent window is
closed before child windows are opened. Multiple event loops causes
unexpected timing issue here, so merging multiple event loops fixes
this.
Differential Revision: https://phabricator.services.mozilla.com/D270490
Diffstat:
5 files changed, 140 insertions(+), 84 deletions(-)
diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java
@@ -733,7 +733,7 @@ public class GeckoSession {
delegate.onLoadRequest(GeckoSession.this, request);
if (result == null) {
- callback.sendSuccess(null);
+ callback.sendSuccess(false);
return;
}
@@ -779,6 +779,13 @@ public class GeckoSession {
}));
} else if ("GeckoView:OnNewSession".equals(event)) {
final String uri = message.getString("uri");
+
+ // Early check whether parent is opened. If not opened, we cannot get parent's runtime.
+ if (!GeckoSession.this.isOpen()) {
+ callback.sendError("Parent session is closed or isn't opened yet");
+ return;
+ }
+
final GeckoResult<GeckoSession> result = delegate.onNewSession(GeckoSession.this, uri);
if (result == null) {
callback.sendSuccess(false);
diff --git a/mobile/shared/modules/geckoview/GeckoViewNavigation.sys.mjs b/mobile/shared/modules/geckoview/GeckoViewNavigation.sys.mjs
@@ -308,17 +308,14 @@ export class GeckoViewNavigation extends GeckoViewModule {
debug`handleNewSession: uri=${aUri && aUri.spec}
where=${aWhere} flags=${aFlags}`;
- const setupPromise = this.#handleNewSessionAsync(
+ let browser = undefined;
+ this._handleNewSessionAsync({
aUri,
aOpenWindowInfo,
- aFlags,
- aName
- );
-
- let browser = undefined;
- setupPromise.then(
- window => {
- browser = window.browser;
+ aName,
+ }).then(
+ result => {
+ browser = result;
},
() => {
browser = null;
@@ -344,7 +341,7 @@ export class GeckoViewNavigation extends GeckoViewModule {
* Similar to handleNewSession. But this returns a promise to wait for new
* browser.
*/
- #handleNewSessionAsync(aUri, aOpenWindowInfo, aFlags, aName) {
+ _handleNewSessionAsync({ aUri, aOpenWindowInfo, aName }) {
if (!this.enabled) {
return Promise.reject();
}
@@ -377,6 +374,9 @@ export class GeckoViewNavigation extends GeckoViewModule {
return Promise.reject();
}
return setupPromise;
+ })
+ .then(newWindow => {
+ return newWindow.browser;
});
}
@@ -397,29 +397,26 @@ export class GeckoViewNavigation extends GeckoViewModule {
return null;
}
- if (
- lazy.LoadURIDelegate.load(
- this.window,
- this.eventDispatcher,
+ const promise = lazy.LoadURIDelegate.load(
+ this.window,
+ this.eventDispatcher,
+ aUri,
+ aWhere,
+ aFlags,
+ aTriggeringPrincipal
+ ).then(handled => {
+ if (handled) {
+ // This will throw NS_ERROR_ABORT
+ return Promise.reject();
+ }
+ return this._handleNewSessionAsync({
aUri,
+ aOpenWindowInfo,
aWhere,
- aFlags,
- aTriggeringPrincipal
- )
- ) {
- // The app has handled the load, abort open-window handling.
- Components.returnCode = Cr.NS_ERROR_ABORT;
- return null;
- }
+ });
+ });
const newTab = this.#isNewTab(aWhere);
- const promise = this.#handleNewSessionAsync(
- aUri,
- aOpenWindowInfo,
- aWhere,
- aFlags,
- null
- );
// Actually, GeckoView's createContentWindow always creates new window even
// if OPEN_NEWTAB. So the browsing context will be observed via
@@ -436,8 +433,8 @@ export class GeckoViewNavigation extends GeckoViewModule {
let browser = undefined;
promise.then(
- window => {
- browser = window.browser;
+ result => {
+ browser = result;
},
() => {
browser = null;
@@ -459,18 +456,9 @@ export class GeckoViewNavigation extends GeckoViewModule {
return browser.browsingContext;
}
- // nsIBrowserDOMWindow.
- createContentWindowInFrame(aUri, aParams, aWhere, aFlags, aName) {
- debug`createContentWindowInFrame: uri=${aUri && aUri.spec}
- where=${aWhere} flags=${aFlags}
- name=${aName}`;
-
- if (aWhere === Ci.nsIBrowserDOMWindow.OPEN_PRINT_BROWSER) {
- return this.window.moduleManager.onPrintWindow(aParams);
- }
-
+ async _createContentWindowInFrameAsync(aUri, aParams, aWhere, aFlags, aName) {
if (
- lazy.LoadURIDelegate.load(
+ await lazy.LoadURIDelegate.load(
this.window,
this.eventDispatcher,
aUri,
@@ -479,18 +467,49 @@ export class GeckoViewNavigation extends GeckoViewModule {
aParams.triggeringPrincipal
)
) {
- // The app has handled the load, abort open-window handling.
- Components.returnCode = Cr.NS_ERROR_ABORT;
return null;
}
- const browser = this.handleNewSession(
+ return await this._handleNewSessionAsync({
+ aUri,
+ aOpenWindowInfo: aParams.openWindowInfo,
+ aName,
+ });
+ }
+
+ // nsIBrowserDOMWindow.
+ createContentWindowInFrame(aUri, aParams, aWhere, aFlags, aName) {
+ debug`createContentWindowInFrame: uri=${aUri && aUri.spec}
+ where=${aWhere} flags=${aFlags}
+ name=${aName}`;
+
+ if (aWhere === Ci.nsIBrowserDOMWindow.OPEN_PRINT_BROWSER) {
+ return this.window.moduleManager.onPrintWindow(aParams);
+ }
+
+ let browser = undefined;
+ this._createContentWindowInFrameAsync(
aUri,
- aParams.openWindowInfo,
+ aParams,
aWhere,
aFlags,
aName
+ ).then(
+ result => {
+ browser = result;
+ },
+ () => {
+ browser = null;
+ }
+ );
+
+ // Wait indefinitely for app to respond with a browser or null.
+ // if browser is null, return error.
+ Services.tm.spinEventLoopUntil(
+ "GeckoViewNavigation.sys.mjs:createContentWindowInFrame",
+ () => this.window.closed || browser !== undefined
);
+
if (!browser) {
Components.returnCode = Cr.NS_ERROR_ABORT;
return null;
@@ -499,7 +518,7 @@ export class GeckoViewNavigation extends GeckoViewModule {
return browser;
}
- handleOpenUri({
+ async _handleOpenUriAsync({
uri,
openWindowInfo,
where,
@@ -509,11 +528,10 @@ export class GeckoViewNavigation extends GeckoViewModule {
referrerInfo = null,
name = null,
}) {
- debug`handleOpenUri: uri=${uri && uri.spec}
- where=${where} flags=${flags}`;
+ debug`_handleOpenUriAsync: uri=${uri?.spec} where=${where} flags=${flags}`;
if (
- lazy.LoadURIDelegate.load(
+ await lazy.LoadURIDelegate.load(
this.window,
this.eventDispatcher,
uri,
@@ -525,19 +543,23 @@ export class GeckoViewNavigation extends GeckoViewModule {
return null;
}
- let browser = this.browser;
-
- if (
- where === Ci.nsIBrowserDOMWindow.OPEN_NEWWINDOW ||
- where === Ci.nsIBrowserDOMWindow.OPEN_NEWTAB ||
- where === Ci.nsIBrowserDOMWindow.OPEN_NEWTAB_BACKGROUND ||
- where === Ci.nsIBrowserDOMWindow.OPEN_NEWTAB_FOREGROUND
- ) {
- browser = this.handleNewSession(uri, openWindowInfo, where, flags, name);
- }
+ const browser = await (async () => {
+ if (
+ where === Ci.nsIBrowserDOMWindow.OPEN_NEWWINDOW ||
+ where === Ci.nsIBrowserDOMWindow.OPEN_NEWTAB ||
+ where === Ci.nsIBrowserDOMWindow.OPEN_NEWTAB_BACKGROUND ||
+ where === Ci.nsIBrowserDOMWindow.OPEN_NEWTAB_FOREGROUND
+ ) {
+ return await this._handleNewSessionAsync({
+ aUri: uri,
+ aOpenWindowInfo: openWindowInfo,
+ aName: name,
+ });
+ }
+ return this.browser;
+ })();
if (!browser) {
- // Should we throw?
return null;
}
@@ -551,6 +573,26 @@ export class GeckoViewNavigation extends GeckoViewModule {
textDirectiveUserActivation:
!!openWindowInfo?.textDirectiveUserActivation,
});
+
+ return browser;
+ }
+
+ _handleOpenUri(openUriInfo) {
+ let browser = undefined;
+ this._handleOpenUriAsync(openUriInfo).then(
+ result => {
+ browser = result;
+ },
+ () => {
+ browser = null;
+ }
+ );
+
+ Services.tm.spinEventLoopUntil(
+ "GeckoViewNavigation.sys.mjs:_handleOpenUri",
+ () => this.window.closed || browser !== undefined
+ );
+
return browser;
}
@@ -563,7 +605,7 @@ export class GeckoViewNavigation extends GeckoViewModule {
aTriggeringPrincipal,
aPolicyContainer
) {
- const browser = this.handleOpenUri({
+ const browser = this._handleOpenUri({
uri: aUri,
openWindowInfo: aOpenWindowInfo,
where: aWhere,
@@ -571,12 +613,18 @@ export class GeckoViewNavigation extends GeckoViewModule {
triggeringPrincipal: aTriggeringPrincipal,
policyContainer: aPolicyContainer,
});
+
+ if (!browser) {
+ Components.returnCode = Cr.NS_ERROR_ABORT;
+ return null;
+ }
+
return browser && browser.browsingContext;
}
// nsIBrowserDOMWindow.
openURIInFrame(aUri, aParams, aWhere, aFlags, aName) {
- const browser = this.handleOpenUri({
+ const browser = this._handleOpenUri({
uri: aUri,
openWindowInfo: aParams.openWindowInfo,
where: aWhere,
@@ -586,6 +634,12 @@ export class GeckoViewNavigation extends GeckoViewModule {
referrerInfo: aParams.referrerInfo,
name: aName,
});
+
+ if (!browser) {
+ Components.returnCode = Cr.NS_ERROR_ABORT;
+ return null;
+ }
+
return browser;
}
diff --git a/mobile/shared/modules/geckoview/LoadURIDelegate.sys.mjs b/mobile/shared/modules/geckoview/LoadURIDelegate.sys.mjs
@@ -10,7 +10,14 @@ const { debug, warn } = GeckoViewUtils.initLogging("LoadURIDelegate");
export const LoadURIDelegate = {
// Delegate URI loading to the app.
// Return whether the loading has been handled.
- load(aWindow, aEventDispatcher, aUri, aWhere, aFlags, aTriggeringPrincipal) {
+ async load(
+ aWindow,
+ aEventDispatcher,
+ aUri,
+ aWhere,
+ aFlags,
+ aTriggeringPrincipal
+ ) {
if (!aWindow) {
return false;
}
@@ -28,23 +35,13 @@ export const LoadURIDelegate = {
hasUserGesture: aWindow.document.hasValidTransientUserGestureActivation,
};
- let handled = undefined;
- aEventDispatcher.sendRequestForResult(message).then(
- response => {
- handled = response;
- },
- () => {
- // There was an error or listener was not registered in GeckoSession,
- // treat as unhandled.
- handled = false;
- }
- );
- Services.tm.spinEventLoopUntil(
- "LoadURIDelegate.sys.mjs:load",
- () => aWindow.closed || handled !== undefined
- );
-
- return handled || false;
+ try {
+ return await aEventDispatcher.sendRequestForResult(message);
+ } catch (e) {
+ // There was an error or listener was not registered in GeckoSession,
+ // treat as unhandled.
+ return false;
+ }
},
handleLoadError(aWindow, aEventDispatcher, aUri, aError, aErrorModule) {
diff --git a/testing/web-platform/meta/html/browsers/the-window-object/open-close/open-features-tokenization-noreferrer.html.ini b/testing/web-platform/meta/html/browsers/the-window-object/open-close/open-features-tokenization-noreferrer.html.ini
@@ -2,4 +2,3 @@
expected:
if (os == "win") and not debug and (processor == "x86"): [OK, TIMEOUT]
if (os == "linux") and (processor == "x86"): [OK, TIMEOUT]
- if os == "android": [OK, ERROR, TIMEOUT, CRASH]
diff --git a/testing/web-platform/meta/html/browsers/the-window-object/open-close/open-features-tokenization-screenx-screeny.html.ini b/testing/web-platform/meta/html/browsers/the-window-object/open-close/open-features-tokenization-screenx-screeny.html.ini
@@ -2,7 +2,6 @@
expected:
if (os == "win") and not debug and (processor == "x86"): [OK, TIMEOUT]
if (os == "linux") and not fission and not debug: [OK, TIMEOUT]
- if os == "android": [OK, ERROR, TIMEOUT, CRASH]
["screenx=141" should set left position of opened window]
expected:
if (os == "linux") and fission and not debug and not asan and not tsan: [PASS, FAIL]
