Make window.open use correct process when called from a site isolated…

… iframe

https://bugs.webkit.org/show_bug.cgi?id=271514

Reviewed by Charlie Wolfe.

When JS calls window.open, WebPageProxy::createNewPage gives an API::PageConfiguration
(which is now equivalent to a WKWebViewConfiguration) to the UIClient containing state
that must be used to make the new WKWebView.  Before site isolation, that state was just
the related page, whose process must be shared to keep functionality working such as
iframes with the same origin as the opener being able to directly access the opener's DOM.
With site isolation, the state that is needed to keep this functionality working is the
BrowsingContextGroup, which contains the maps of domains to processes, and the opener
frame identifier, which needs to be used to put the opened frame initially in the same
process as the opener while the opened frame contains only about:blank.  From about:blank
it can then use the BrowsingContextGroup to navigate to the correct process for any domain.

Unfortunately, though, there is SPI to do tricky things with the related page, and
WebKitTestRunner uses this SPI to retrofit an unrelated WKWebViewConfiguration into looking
enough like the suggested WKWebViewConfiguration that WebKit accepts it and uses it correctly.
In order to keep this working, I needed to make additional SPI to copy the site isolation
state from one WKPageConfigurationRef to another.  Conveninetly, WKPageConfigurationRef
can now be converted to WKWebViewConfiguration using toll-free-bridging.

It looks like all that work I did over the last week to clean up WKWebViewConfiguration
is already paying off!

I verified this fixes http/tests/site-isolation/iframe-and-window-open.html
but only how we used to run it before 276416@main.  Additional work is being done
on our strategy for running existing layout tests to verify site isolation correctness.

* LayoutTests/platform/mac-site-isolation/TestExpectations:
* Source/WebKit/UIProcess/API/APIPageConfiguration.cpp:
(API::PageConfiguration::setBrowsingContextGroup):
(API::PageConfiguration::openerFrameID const):
(API::PageConfiguration::setOpenerFrameID):
* Source/WebKit/UIProcess/API/APIPageConfiguration.h:
* Source/WebKit/UIProcess/API/C/WKPageConfigurationRef.cpp:
(WKPageConfigurationCopySiteIsolationState):
* Source/WebKit/UIProcess/API/C/WKPageConfigurationRef.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::createNewPage):
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::createWebPage):
* Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:
(WTR::TestController::platformCreateOtherPage):

Canonical link: https://commits.webkit.org/276636@main

LayoutTests/platform/mac-site-isolation/TestExpectations

@@ -113,7 +113,6 @@ http/tests/security/mixedContent [ Skip ]
 http/tests/security/originHeader [ Skip ]
 http/tests/security/postMessage [ Skip ]
 http/tests/security/XFrameOptions [ Skip ]
-http/tests/site [ Skip ]-isolation
 http/tests/ssl [ Skip ]
 http/tests/storage [ Skip ]
 http/tests/storageAccess [ Skip ]
@@ -407,7 +406,6 @@ http/tests/loading/simple-subframe.html [ Skip ]
 http/tests/loading/sizes/preload-image-sizes.html [ Skip ]
 http/tests/loading/slow-parsing-subframe.html [ Skip ]
 http/tests/loading/state-object-security-exception.html [ Skip ]
-http/tests/loading/test.html [ Skip ]
 http/tests/security/basic-auth-subresource.html [ Skip ]
 http/tests/security/block-top-level-navigation-to-different-scheme-by-third-party-iframes.html [ Skip ]
 http/tests/security/block-top-level-navigations-by-sandboxed-iframe-with-propagated-user-gesture.html [ Skip ]

Source/WebKit/UIProcess/API/APIPageConfiguration.cpp

@@ -100,6 +100,21 @@ BrowsingContextGroup& PageConfiguration::browsingContextGroup() const
     return m_data.browsingContextGroup.get();
 }
 
+void PageConfiguration::setBrowsingContextGroup(RefPtr<BrowsingContextGroup>&& group)
+{
+    m_data.browsingContextGroup = WTFMove(group);
+}
+
+RefPtr<WebKit::WebProcessProxy> PageConfiguration::openerProcess() const
+{
+    return m_data.openerProcess;
+}
+
+void PageConfiguration::setOpenerProcess(RefPtr<WebKit::WebProcessProxy>&& process)
+{
+    m_data.openerProcess = WTFMove(process);
+}
+
 WebProcessPool& PageConfiguration::processPool() const
 {
     return m_data.processPool.get();

Source/WebKit/UIProcess/API/APIPageConfiguration.h

@@ -29,10 +29,12 @@
 #include "WebPreferencesDefaultValues.h"
 #include "WebURLSchemeHandler.h"
 #include <WebCore/ContentSecurityPolicy.h>
+#include <WebCore/FrameIdentifier.h>
 #include <WebCore/ShouldRelaxThirdPartyCookieBlocking.h>
 #include <wtf/Forward.h>
 #include <wtf/GetPtr.h>
 #include <wtf/HashMap.h>
+#include <wtf/Markable.h>
 #include <wtf/RobinHoodHashSet.h>
 #include <wtf/text/WTFString.h>
 
@@ -95,6 +97,10 @@ class PageConfiguration : public ObjectImpl<Object::Type::PageConfiguration> {
     Ref<PageConfiguration> copy() const;
 
     WebKit::BrowsingContextGroup& browsingContextGroup() const;
+    void setBrowsingContextGroup(RefPtr<WebKit::BrowsingContextGroup>&&);
+
+    RefPtr<WebKit::WebProcessProxy> openerProcess() const;
+    void setOpenerProcess(RefPtr<WebKit::WebProcessProxy>&&);
 
     WebKit::WebProcessPool& processPool() const;
     void setProcessPool(RefPtr<WebKit::WebProcessPool>&&);
@@ -446,6 +452,7 @@ class PageConfiguration : public ObjectImpl<Object::Type::PageConfiguration> {
 #endif
         RefPtr<WebKit::WebPageGroup> pageGroup;
         WeakPtr<WebKit::WebPageProxy> relatedPage;
+        RefPtr<WebKit::WebProcessProxy> openerProcess;
         WeakPtr<WebKit::WebPageProxy> pageToCloneSessionStorageFrom;
         WeakPtr<WebKit::WebPageProxy> alternateWebViewForNavigationGestures;
 

Source/WebKit/UIProcess/API/C/WKPageConfigurationRef.cpp

@@ -27,6 +27,7 @@
 #include "WKPageConfigurationRef.h"
 
 #include "APIPageConfiguration.h"
+#include "BrowsingContextGroup.h"
 #include "WKAPICast.h"
 #include "WebPageGroup.h"
 #include "WebPageProxy.h"
@@ -123,3 +124,9 @@ void WKPageConfigurationSetPortsForUpgradingInsecureSchemeForTesting(WKPageConfi
 {
     toImpl(configuration)->setPortsForUpgradingInsecureSchemeForTesting(upgradeFromInsecurePort, upgradeToSecurePort);
 }
+
+void WKPageConfigurationCopySiteIsolationState(WKPageConfigurationRef destination, WKPageConfigurationRef source)
+{
+    toImpl(destination)->setBrowsingContextGroup(&toImpl(source)->browsingContextGroup());
+    toImpl(destination)->setOpenerProcess(toImpl(source)->openerProcess());
+}

Source/WebKit/UIProcess/API/C/WKPageConfigurationRef.h

@@ -58,6 +58,8 @@ WK_EXPORT void WKPageConfigurationSetWebsiteDataStore(WKPageConfigurationRef con
 WK_EXPORT void WKPageConfigurationSetInitialCapitalizationEnabled(WKPageConfigurationRef configuration, bool enabled);
 WK_EXPORT void WKPageConfigurationSetBackgroundCPULimit(WKPageConfigurationRef configuration, double cpuLimit); // Terminates process if it uses more than CPU limit over an extended period of time while in the background.
 
+WK_EXPORT void WKPageConfigurationCopySiteIsolationState(WKPageConfigurationRef destination, WKPageConfigurationRef source);
+
 WK_EXPORT void WKPageConfigurationSetAllowTestOnlyIPC(WKPageConfigurationRef configuration, bool allowTestOnlyIPC);
 
 WK_EXPORT void WKPageConfigurationSetPortsForUpgradingInsecureSchemeForTesting(WKPageConfigurationRef configuration, uint16_t upgradeFromInsecurePort, uint16_t upgradeToSecurePort);

Source/WebKit/UIProcess/Cocoa/UIDelegate.mm

@@ -319,11 +319,12 @@
     ASSERT(delegate);
 
     auto apiWindowFeatures = API::WindowFeatures::create(windowFeatures);
+    RefPtr openerProcess = configuration->openerProcess();
 
     if (m_uiDelegate->m_delegateMethods.webViewCreateWebViewWithConfigurationForNavigationActionWindowFeaturesAsync) {
         auto checker = CompletionHandlerCallChecker::create(delegate.get(), @selector(_webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:completionHandler:));
 
-        [(id<WKUIDelegatePrivate>)delegate _webView:m_uiDelegate->m_webView.get().get() createWebViewWithConfiguration:wrapper(configuration) forNavigationAction:wrapper(navigationAction) windowFeatures:wrapper(apiWindowFeatures) completionHandler:makeBlockPtr([completionHandler = WTFMove(completionHandler), checker = WTFMove(checker), relatedWebView = m_uiDelegate->m_webView.get()] (WKWebView *webView) mutable {
+        [(id<WKUIDelegatePrivate>)delegate _webView:m_uiDelegate->m_webView.get().get() createWebViewWithConfiguration:wrapper(configuration) forNavigationAction:wrapper(navigationAction) windowFeatures:wrapper(apiWindowFeatures) completionHandler:makeBlockPtr([completionHandler = WTFMove(completionHandler), checker = WTFMove(checker), relatedWebView = m_uiDelegate->m_webView.get(), openerProcess] (WKWebView *webView) mutable {
             if (checker->completionHandlerHasBeenCalled())
                 return;
             checker->didCallCompletionHandler();
@@ -336,6 +337,9 @@
             if ([webView->_configuration _relatedWebView] != relatedWebView.get())
                 [NSException raise:NSInternalInconsistencyException format:@"Returned WKWebView was not created with the given configuration."];
 
+            if (openerProcess != webView->_configuration->_pageConfiguration->openerProcess())
+                [NSException raise:NSInternalInconsistencyException format:@"Returned WKWebView was not created with the given configuration."];
+
             completionHandler(webView->_page.get());
         }).get()];
         return;
@@ -349,6 +353,8 @@
 
     if ([webView.get()->_configuration _relatedWebView] != m_uiDelegate->m_webView.get().get())
         [NSException raise:NSInternalInconsistencyException format:@"Returned WKWebView was not created with the given configuration."];
+    if (openerProcess != webView.get()->_configuration->_pageConfiguration->openerProcess())
+        [NSException raise:NSInternalInconsistencyException format:@"Returned WKWebView was not created with the given configuration."];
     completionHandler(webView->_page.get());
 }
 

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -7447,6 +7447,10 @@ void WebPageProxy::createNewPage(WindowFeatures&& windowFeatures, NavigationActi
 
     Ref configuration = this->configuration().copy();
     configuration->setRelatedPage(*this);
+    if (m_preferences->siteIsolationEnabled()) {
+        if (RefPtr openerFrame = WebFrameProxy::webFrame(originatingFrameInfoData.frameID))
+            configuration->setOpenerProcess(&openerFrame->process());
+    }
 
     trySOAuthorization(WTFMove(navigationAction), *this, WTFMove(completionHandler), [this, protectedThis = Ref { *this }, windowFeatures = WTFMove(windowFeatures), configuration = WTFMove(configuration)] (Ref<API::NavigationAction>&& navigationAction, CompletionHandler<void(RefPtr<WebPageProxy>&&)>&& completionHandler) mutable {
 

Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -1223,8 +1223,11 @@ Ref<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API:
     RefPtr<WebProcessProxy> process;
     auto lockdownMode = pageConfiguration->lockdownModeEnabled() ? WebProcessProxy::LockdownMode::Enabled : WebProcessProxy::LockdownMode::Disabled;
     RefPtr relatedPage = pageConfiguration->relatedPage();
-    // FIXME: If site isolation is enabled, this needs to get the correct process instead of assuming we always want the process of the related page.
-    if (relatedPage && !relatedPage->isClosed() && relatedPage->hasSameGPUAndNetworkProcessPreferencesAs(pageConfiguration)) {
+
+    if (pageConfiguration->openerProcess()) {
+        ASSERT(pageConfiguration->preferences().siteIsolationEnabled());
+        process = pageConfiguration->openerProcess();
+    } else if (relatedPage && !relatedPage->isClosed() && relatedPage->hasSameGPUAndNetworkProcessPreferencesAs(pageConfiguration)) {
         // Sharing processes, e.g. when creating the page via window.open().
         process = &relatedPage->ensureRunningProcess();
         // We do not support several WebsiteDataStores sharing a single process.

Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm

@@ -340,13 +340,14 @@ void initializeWebViewConfiguration(const char* libraryPath, WKStringRef injecte
     [m_mainWebView->platformView() _setShareSheetCompletesImmediatelyWithResolutionForTesting:YES];
 }
 
-UniqueRef<PlatformWebView> TestController::platformCreateOtherPage(PlatformWebView* parentView, WKPageConfigurationRef, const TestOptions& options)
+UniqueRef<PlatformWebView> TestController::platformCreateOtherPage(PlatformWebView* parentView, WKPageConfigurationRef originalConfiguration, const TestOptions& options)
 {
     auto newConfiguration = adoptNS([globalWebViewConfiguration() copy]);
     if (parentView)
         [newConfiguration _setRelatedWebView:static_cast<WKWebView*>(parentView->platformView())];
     if ([newConfiguration _relatedWebView])
         [newConfiguration setWebsiteDataStore:[newConfiguration _relatedWebView].configuration.websiteDataStore];
+    WKPageConfigurationCopySiteIsolationState((__bridge WKPageConfigurationRef)newConfiguration.get(), originalConfiguration);
     [newConfiguration _setPortsForUpgradingInsecureSchemeForTesting:@[@(options.insecureUpgradePort()), @(options.secureUpgradePort())]];
     auto view = makeUniqueRef<PlatformWebView>(newConfiguration.get(), options);
     finishCreatingPlatformWebView(view.ptr(), options);