Skip to content

Commit

Permalink
Bug 1810619 - Part 2: Get the content window from client.openWindow o…
Browse files Browse the repository at this point in the history
…n 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
  • Loading branch information
mystor committed Mar 15, 2023
1 parent ba562b2 commit dddcb16
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 6 deletions.
14 changes: 9 additions & 5 deletions dom/clients/manager/ClientOpenWindowUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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> browsingContext;
nsresult rv = [&sessionId, &browsingContext]() -> nsresult {
nsresult rv;
Expand All @@ -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<nsIDocShellTreeOwner> 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;
}();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ static GeckoRuntime getInstance() {
if (!session.isOpen()) {
session.open(sRuntime);
}
session.loadUri(url);
result.complete(session.getId());
} else {
result.complete(null);
Expand Down
9 changes: 9 additions & 0 deletions mobile/android/modules/geckoview/GeckoViewTestUtils.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
};

0 comments on commit dddcb16

Please sign in to comment.