Skip to content

Commit

Permalink
Site isolation feature flag should also enable process swapping on wi…
Browse files Browse the repository at this point in the history
…ndow.open

https://bugs.webkit.org/show_bug.cgi?id=264265
rdar://118006257

Reviewed by Pascoe.

Site isolation implies cross-site window.open swaps processes.

* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::initializeWebPage):
(WebKit::ProvisionalPageProxy::didCreateMainFrame):
(WebKit::ProvisionalPageProxy::didStartProvisionalLoadForFrame):
(WebKit::ProvisionalPageProxy::didCommitLoadForFrame):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::suspendCurrentPageIfPossible):
(WebKit::WebPageProxy::shouldClosePreviousPage):
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigationInternal):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::hasRootFrames):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::TEST):
(TestWebKitAPI::openerAndOpenedViews):
(TestWebKitAPI::enableWindowOpenPSON): Deleted.

Canonical link: https://commits.webkit.org/270278@main
  • Loading branch information
achristensen07 committed Nov 6, 2023
1 parent 55af19e commit 7524670
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 23 deletions.
8 changes: 4 additions & 4 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ void ProvisionalPageProxy::initializeWebPage(RefPtr<API::WebsitePolicies>&& webs
bool sendPageCreationParameters { true };
auto parameters = m_page->creationParameters(m_process, *m_drawingArea, WTFMove(websitePolicies));

if (page().preferences().processSwapOnCrossSiteWindowOpenEnabled()) {
if (page().preferences().processSwapOnCrossSiteWindowOpenEnabled() || page().preferences().siteIsolationEnabled()) {
RegistrableDomain navigationDomain(m_request.url());
RefPtr openerFrame = m_page->openerFrame();
if (RefPtr openerPage = openerFrame ? openerFrame->page() : nullptr) {
Expand Down Expand Up @@ -305,7 +305,7 @@ void ProvisionalPageProxy::didCreateMainFrame(FrameIdentifier frameID)
ASSERT(!m_mainFrame);

RefPtr<WebFrameProxy> previousMainFrame = m_page->mainFrame();
if (page().preferences().processSwapOnCrossSiteWindowOpenEnabled() && m_page->openerFrame()) {
if (m_page->openerFrame() && (page().preferences().processSwapOnCrossSiteWindowOpenEnabled() || page().preferences().siteIsolationEnabled())) {
ASSERT(m_page->mainFrame()->frameID() == frameID);
m_mainFrame = m_page->mainFrame();
} else
Expand Down Expand Up @@ -356,7 +356,7 @@ void ProvisionalPageProxy::didStartProvisionalLoadForFrame(FrameIdentifier frame
return;

// When this is true, we are reusing the main WebFrameProxy and its frame state will be updated in didStartProvisionalLoadForFrameShared.
bool isCrossSiteWindowOpen = page().preferences().processSwapOnCrossSiteWindowOpenEnabled() && m_page->openerFrame();
bool isCrossSiteWindowOpen = m_page->openerFrame() && (page().preferences().processSwapOnCrossSiteWindowOpenEnabled() || page().preferences().siteIsolationEnabled());

// Clients expect the Page's main frame's expectedURL to be the provisional one when a provisional load is started.
if (auto* pageMainFrame = m_page->mainFrame(); pageMainFrame && !isCrossSiteWindowOpen)
Expand Down Expand Up @@ -390,7 +390,7 @@ void ProvisionalPageProxy::didCommitLoadForFrame(FrameIdentifier frameID, FrameI

PROVISIONALPAGEPROXY_RELEASE_LOG(ProcessSwapping, "didCommitLoadForFrame: frameID=%" PRIu64, frameID.object().toUInt64());
auto page = protectedPage();
if (page->preferences().processSwapOnCrossSiteWindowOpenEnabled()) {
if (page->preferences().processSwapOnCrossSiteWindowOpenEnabled() || page->preferences().siteIsolationEnabled()) {
RefPtr openerFrame = m_page->openerFrame();
if (RefPtr openerPage = openerFrame ? openerFrame->page() : nullptr) {
RegistrableDomain openerDomain(openerFrame->url());
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,7 @@ bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, Ref
if (!mainFrame)
return false;

if (openerFrame() && preferences().processSwapOnCrossSiteWindowOpenEnabled()) {
if (openerFrame() && (preferences().processSwapOnCrossSiteWindowOpenEnabled() || preferences().siteIsolationEnabled())) {
WEBPAGEPROXY_RELEASE_LOG(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because it has an opener.", m_process->processID());
return false;
}
Expand Down Expand Up @@ -4331,7 +4331,7 @@ void WebPageProxy::commitProvisionalPage(FrameIdentifier frameID, FrameInfoData&
bool WebPageProxy::shouldClosePreviousPage()
{
auto* opener = openerFrame();
return !preferences().processSwapOnCrossSiteWindowOpenEnabled()
return !(preferences().processSwapOnCrossSiteWindowOpenEnabled() || preferences().siteIsolationEnabled())
|| !opener
|| opener->process() != process();
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/WebProcessPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1938,11 +1938,11 @@ std::tuple<Ref<WebProcessProxy>, SuspendedPageProxy*, ASCIILiteral> WebProcessPo

// FIXME: We should support process swap when a window has been opened via window.open() without 'noopener'.
// The issue is that the opener has a handle to the WindowProxy.
if (navigation.openedByDOMWithOpener() && !!page.openerFrame() && !page.preferences().processSwapOnCrossSiteWindowOpenEnabled())
if (navigation.openedByDOMWithOpener() && !!page.openerFrame() && !(page.preferences().processSwapOnCrossSiteWindowOpenEnabled() || page.preferences().siteIsolationEnabled()))
return { WTFMove(sourceProcess), nullptr, "Browsing context been opened by DOM without 'noopener'"_s };

// FIXME: We should support process swap when a window has opened other windows via window.open.
if (navigation.hasOpenedFrames() && page.hasOpenedPage() && !page.preferences().processSwapOnCrossSiteWindowOpenEnabled())
if (navigation.hasOpenedFrames() && page.hasOpenedPage() && !(page.preferences().processSwapOnCrossSiteWindowOpenEnabled() || page.preferences().siteIsolationEnabled()))
return { WTFMove(sourceProcess), nullptr, "Browsing context has opened other windows"_s };

if (RefPtr targetItem = navigation.targetItem()) {
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4774,7 +4774,7 @@ bool WebPage::hasRootFrames()
bool result = m_page && !m_page->rootFrames().isEmpty();
#if ASSERT_ENABLED
if (!result)
ASSERT(m_page->settings().processSwapOnCrossSiteWindowOpenEnabled());
ASSERT(m_page->settings().processSwapOnCrossSiteWindowOpenEnabled() || m_page->settings().siteIsolationEnabled());
#endif
return result;
}
Expand Down
15 changes: 1 addition & 14 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,6 @@ static void enableSiteIsolation(WKWebViewConfiguration *configuration)
}
}

static void enableWindowOpenPSON(WKWebViewConfiguration *configuration)
{
auto preferences = [configuration preferences];
for (_WKFeature *feature in [WKPreferences _features]) {
if ([feature.key isEqualToString:@"ProcessSwapOnCrossSiteWindowOpenEnabled"]) {
[preferences _setEnabled:YES forFeature:feature];
break;
}
}
}

static std::pair<RetainPtr<WKWebView>, RetainPtr<TestNavigationDelegate>> siteIsolatedViewAndDelegate(const HTTPServer& server)
{
auto navigationDelegate = adoptNS([TestNavigationDelegate new]);
Expand Down Expand Up @@ -299,7 +288,7 @@ HTTPServer server({
};

auto configuration = server.httpsProxyConfiguration();
enableWindowOpenPSON(configuration);
enableSiteIsolation(configuration);

__block RetainPtr<NSString> alert;
auto uiDelegate = adoptNS([TestUIDelegate new]);
Expand Down Expand Up @@ -347,13 +336,11 @@ HTTPServer server({
[opener.navigationDelegate allowAnyTLSCertificate];
auto configuration = server.httpsProxyConfiguration();
enableSiteIsolation(configuration);
enableWindowOpenPSON(configuration);
opener.webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration]);
opener.webView.get().navigationDelegate = opener.navigationDelegate.get();
opener.uiDelegate = adoptNS([TestUIDelegate new]);
opener.uiDelegate.get().createWebViewWithConfiguration = ^(WKWebViewConfiguration *configuration, WKNavigationAction *action, WKWindowFeatures *windowFeatures) {
enableSiteIsolation(configuration);
enableWindowOpenPSON(configuration);
opened.webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectZero configuration:configuration]);
opened.navigationDelegate = adoptNS([TestNavigationDelegate new]);
[opened.navigationDelegate allowAnyTLSCertificate];
Expand Down

0 comments on commit 7524670

Please sign in to comment.