From 0acdb79a3ff58ca2519e7763c0c5890baf5fb6bc Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Wed, 15 Mar 2023 21:57:04 +0000 Subject: [PATCH] Bug 1810619 - Part 2: Get the content window from client.openWindow on geckoview, r=geckoview-reviewers,m_kato While writing part 1 of this patch, I noticed that the geckoview code for client.openWindow was returning the outer chrome window's BrowsingContext rather than the BrowsingContext of the primary content frame when opening a pop-up window. This meant that the native code would fail to start navigating the pop-up window (as it would try to navigate the chrome window which is not allowed). It turns out the tests were still passing because the geckoview code was actually starting the load itself, though with the wrong options and properties. In this patch I remove that call to load a URI from the Java code, and fix the code in ClientOpenWindowUtils to return the content BrowsingContext instead of the chrome one. Differential Revision: https://phabricator.services.mozilla.com/D171756 --- dom/clients/manager/ClientOpenWindowUtils.cpp | 14 +++++++++----- .../java/org/mozilla/geckoview/GeckoRuntime.java | 1 - .../modules/geckoview/GeckoViewTestUtils.jsm | 9 +++++++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/dom/clients/manager/ClientOpenWindowUtils.cpp b/dom/clients/manager/ClientOpenWindowUtils.cpp index c9edd51892d5..e741a266fdc3 100644 --- a/dom/clients/manager/ClientOpenWindowUtils.cpp +++ b/dom/clients/manager/ClientOpenWindowUtils.cpp @@ -328,9 +328,9 @@ void GeckoViewOpenWindow(const ClientOpenWindowArgsParsed& aArgsValidated, promiseResult->Then( GetMainThreadSerialEventTarget(), __func__, [aArgsValidated, promise](nsString sessionId) { - // Retrieve the browsing context by using the GeckoSession ID. The - // window is named the same as the ID of the GeckoSession it is - // associated with. + // Retrieve the primary content BrowsingContext using the GeckoSession + // ID. The chrome window is named the same as the ID of the GeckoSession + // it is associated with. RefPtr browsingContext; nsresult rv = [&sessionId, &browsingContext]() -> nsresult { nsresult rv; @@ -341,8 +341,12 @@ void GeckoViewOpenWindow(const ClientOpenWindowArgsParsed& aArgsValidated, rv = wwatch->GetWindowByName(sessionId, getter_AddRefs(chromeWindow)); NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_TRUE(chromeWindow, NS_ERROR_FAILURE); - browsingContext = - nsPIDOMWindowOuter::From(chromeWindow)->GetBrowsingContext(); + nsCOMPtr treeOwner = + nsPIDOMWindowOuter::From(chromeWindow)->GetTreeOwner(); + NS_ENSURE_TRUE(treeOwner, NS_ERROR_FAILURE); + rv = treeOwner->GetPrimaryContentBrowsingContext( + getter_AddRefs(browsingContext)); + NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_TRUE(browsingContext, NS_ERROR_FAILURE); return NS_OK; }(); diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java index aa5f01da59ee..1a66866d6ab0 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java @@ -272,7 +272,6 @@ public final class GeckoRuntime implements Parcelable { if (!session.isOpen()) { session.open(sRuntime); } - session.loadUri(url); result.complete(session.getId()); } else { result.complete(null); diff --git a/mobile/android/modules/geckoview/GeckoViewTestUtils.jsm b/mobile/android/modules/geckoview/GeckoViewTestUtils.jsm index e8cfad4f4271..966eae2d9072 100644 --- a/mobile/android/modules/geckoview/GeckoViewTestUtils.jsm +++ b/mobile/android/modules/geckoview/GeckoViewTestUtils.jsm @@ -51,6 +51,15 @@ const GeckoViewTabUtil = { } const window = await windowPromise; + + // Immediately load the URI in the browser after creating the new tab to + // load into. This isn't done from the Java side to align with the + // ServiceWorkerOpenWindow infrastructure which this is built on top of. + window.browser.fixupAndLoadURIString(url, { + flags: Ci.nsIWebNavigation.LOAD_FLAGS_NONE, + triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(), + }); + return window.tab; }, };