WebKit · webkit-commit-queue · Nov 8, 2023 · Nov 8, 2023
diff --git a/LayoutTests/http/tests/site-isolation/iframe-and-window-open-expected.txt b/LayoutTests/http/tests/site-isolation/iframe-and-window-open-expected.txt
@@ -0,0 +1,10 @@
+Verifies window.open can successfully open a window with the same origin as an iframe of this window
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS received message from opened window: opened window received: initial reply
+PASS successfullyParsed is true
+
+TEST COMPLETE
+click to run test manually in a browser
diff --git a/LayoutTests/http/tests/site-isolation/iframe-and-window-open.html b/LayoutTests/http/tests/site-isolation/iframe-and-window-open.html
@@ -0,0 +1,27 @@
+<!-- webkit-test-runner [ SiteIsolationEnabled=true ] -->
+<script src="/js-test-resources/js-test.js"></script>
+<script>
+
+description("Verifies window.open can successfully open a window with the same origin as an iframe of this window");
+jsTestIsAsync = true;
+
+function runTest() {
+    openedWindow = window.open("http://localhost:8000/site-isolation/resources/post-message-to-opener.html");
+}
+
+function runTestIfInTestRunner() { if (window.testRunner) { runTest() } }
+
+addEventListener("message", (event) => {
+    if (event.data == 'initial ping') {
+        openedWindow.postMessage('initial reply', '*');
+        return;
+    }
+    testPassed("received message from opened window: " + event.data);
+    finishJSTest();
+});
+
+</script>
+<body onload="runTestIfInTestRunner()">
+<button onclick="runTest()">click to run test manually in a browser</button>
+<iframe src='http://localhost:8000/site-isolation/resources/green-background.html'></iframe>
+</body>
diff --git a/LayoutTests/http/tests/site-isolation/resources/post-message-to-opener.html b/LayoutTests/http/tests/site-isolation/resources/post-message-to-opener.html
@@ -0,0 +1,6 @@
+<script>
+    addEventListener("message", (event) => {
+        window.opener.postMessage("opened window received: " + event.data, "*")
+    });
+    window.opener.postMessage("initial ping", "*")
+</script>
@@ -1520,7 +1520,7 @@ void DocumentLoader::attachToFrame()
     DOCUMENTLOADER_RELEASE_LOG("DocumentLoader::attachToFrame: m_frame=%p", m_frame.get());
 }
 
-void DocumentLoader::detachFromFrame()
+void DocumentLoader::detachFromFrame(LoadWillContinueInAnotherProcess)
 {
     DOCUMENTLOADER_RELEASE_LOG("DocumentLoader::detachFromFrame: m_frame=%p", m_frame.get());
 

@@ -95,6 +95,7 @@ class SubstituteResource;
 class UserContentURLPattern;
 
 enum class ClearSiteDataValue : uint8_t;
+enum class LoadWillContinueInAnotherProcess : bool;
 enum class ShouldContinue;
 
 using ResourceLoaderMap = HashMap<ResourceLoaderIdentifier, RefPtr<ResourceLoader>>;
@@ -185,7 +186,7 @@ class DocumentLoader
 
     void attachToFrame(LocalFrame&);
 
-    WEBCORE_EXPORT virtual void detachFromFrame();
+    WEBCORE_EXPORT virtual void detachFromFrame(LoadWillContinueInAnotherProcess);
 
     WEBCORE_EXPORT FrameLoader* frameLoader() const;
     WEBCORE_EXPORT SubresourceLoader* mainResourceLoader() const;

@@ -2033,12 +2033,12 @@ void FrameLoader::setDocumentLoader(RefPtr<DocumentLoader>&& loader)
         return;
 
     if (RefPtr documentLoader = m_documentLoader)
-        documentLoader->detachFromFrame();
+        documentLoader->detachFromFrame(LoadWillContinueInAnotherProcess::No);
 
     m_documentLoader = WTFMove(loader);
 }
 
-void FrameLoader::setPolicyDocumentLoader(RefPtr<DocumentLoader>&& loader)
+void FrameLoader::setPolicyDocumentLoader(RefPtr<DocumentLoader>&& loader, LoadWillContinueInAnotherProcess loadWillContinueInAnotherProcess)
 {
     if (m_policyDocumentLoader == loader)
         return;
@@ -2051,7 +2051,7 @@ void FrameLoader::setPolicyDocumentLoader(RefPtr<DocumentLoader>&& loader)
     if (RefPtr policyDocumentLoader = m_policyDocumentLoader; policyDocumentLoader
         && policyDocumentLoader != m_provisionalDocumentLoader
         && policyDocumentLoader != m_documentLoader) {
-        policyDocumentLoader->detachFromFrame();
+        policyDocumentLoader->detachFromFrame(loadWillContinueInAnotherProcess);
     }
 
     m_policyDocumentLoader = WTFMove(loader);
@@ -2068,7 +2068,7 @@ void FrameLoader::setProvisionalDocumentLoader(RefPtr<DocumentLoader>&& loader)
     RELEASE_ASSERT(!loader || loader->frameLoader() == this);
 
     if (RefPtr provisionalDocumentLoader = m_provisionalDocumentLoader; provisionalDocumentLoader && provisionalDocumentLoader != m_documentLoader)
-        provisionalDocumentLoader->detachFromFrame();
+        provisionalDocumentLoader->detachFromFrame(LoadWillContinueInAnotherProcess::No);
 
     m_provisionalDocumentLoader = WTFMove(loader);
 }
@@ -3712,7 +3712,7 @@ void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& reque
             m_checkTimer.stop();
         }
 
-        setPolicyDocumentLoader(nullptr);
+        setPolicyDocumentLoader(nullptr, navigationPolicyDecision == NavigationPolicyDecision::LoadWillContinueInAnotherProcess ? LoadWillContinueInAnotherProcess::Yes : LoadWillContinueInAnotherProcess::No);
         if (frame->isMainFrame() || navigationPolicyDecision != NavigationPolicyDecision::LoadWillContinueInAnotherProcess)
             checkCompleted();
         else {

@@ -386,7 +386,7 @@ class FrameLoader final : public CanMakeCheckedPtr {
     void handleLoadFailureRecovery(DocumentLoader&, const ResourceError&, bool);
 
     void setDocumentLoader(RefPtr<DocumentLoader>&&);
-    void setPolicyDocumentLoader(RefPtr<DocumentLoader>&&);
+    void setPolicyDocumentLoader(RefPtr<DocumentLoader>&&, LoadWillContinueInAnotherProcess = LoadWillContinueInAnotherProcess::No);
     void setProvisionalDocumentLoader(RefPtr<DocumentLoader>&&);
 
     void setState(FrameState);

@@ -142,6 +142,7 @@ enum class LockHistory : bool { No, Yes };
 enum class LockBackForwardList : bool { No, Yes };
 enum class AllowNavigationToInvalidURL : bool { No, Yes };
 enum class HasInsecureContent : bool { No, Yes };
+enum class LoadWillContinueInAnotherProcess : bool { No, Yes };
 
 // FIXME: This should move to somewhere else. It no longer is related to frame loading.
 struct SystemPreviewInfo {

@@ -214,6 +214,10 @@ void ProvisionalPageProxy::initializeWebPage(RefPtr<API::WebsitePolicies>&& webs
                 };
                 m_process->send(Messages::WebPage::TransitionFrameToLocal(localFrameCreationParameters, m_page->mainFrame()->frameID()), m_webPageID);
                 sendPageCreationParameters = false;
+            } else if (RefPtr existingRemotePageProxy = openerPage->remotePageProxyForRegistrableDomain(navigationDomain)) {
+                ASSERT(existingRemotePageProxy->process().processID() == m_process->processID());
+                openerPage->addOpenedRemotePageProxy(m_page->identifier(), existingRemotePageProxy.releaseNonNull());
+                m_needsCookieAccessAddedInNetworkProcess = true;
             } else {
                 auto remotePageProxy = RemotePageProxy::create(*openerPage, m_process, navigationDomain);
                 remotePageProxy->injectPageIntoNewProcess();

@@ -125,6 +125,8 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
 
     void processDidTerminate();
 
+    bool needsCookieAccessAddedInNetworkProcess() const { return m_needsCookieAccessAddedInNetworkProcess; }
+
 private:
     RefPtr<WebFrameProxy> protectedMainFrame() const;
 
@@ -189,6 +191,7 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
     ProcessSwapRequestedByClient m_processSwapRequestedByClient;
     bool m_wasCommitted { false };
     bool m_isProcessSwappingOnNavigationResponse { false };
+    bool m_needsCookieAccessAddedInNetworkProcess { false };
     URL m_provisionalLoadURL;
     WebPageProxyMessageReceiverRegistration m_messageReceiverRegistration;
 

@@ -4379,7 +4379,8 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, W
 
     m_provisionalPage = makeUnique<ProvisionalPageProxy>(*this, WTFMove(newProcess), WTFMove(suspendedPage), navigation, isServerSideRedirect, navigation.currentRequest(), processSwapRequestedByClient, isProcessSwappingOnNavigationResponse, websitePolicies.get());
 
-    auto continuation = [this, protectedThis = Ref { *this }, navigation = Ref { navigation }, shouldTreatAsContinuingLoad, websitePolicies = WTFMove(websitePolicies), existingNetworkResourceLoadIdentifierToResume]() mutable {
+    // FIXME: This should be a CompletionHandler, but http/tests/inspector/target/provisional-load-cancels-previous-load.html doesn't call it.
+    Function<void()> continuation = [this, protectedThis = Ref { *this }, navigation = Ref { navigation }, shouldTreatAsContinuingLoad, websitePolicies = WTFMove(websitePolicies), existingNetworkResourceLoadIdentifierToResume]() mutable {
         if (auto* item = navigation->targetItem()) {
             LOG(Loading, "WebPageProxy %p continueNavigationInNewProcess to back item URL %s", this, item->url().utf8().data());
 
@@ -4408,6 +4409,13 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, W
         else
             m_provisionalPage->loadRequest(navigation, ResourceRequest { navigation->currentRequest() }, nullptr, shouldTreatAsContinuingLoad, isNavigatingToAppBoundDomain(), WTFMove(websitePoliciesData), existingNetworkResourceLoadIdentifierToResume);
     };
+
+    if (m_provisionalPage->needsCookieAccessAddedInNetworkProcess()) {
+        continuation = [networkProcess = Ref { websiteDataStore().networkProcess() }, continuation = WTFMove(continuation), navigationDomain = RegistrableDomain(navigation.currentRequest().url()), processIdentifier = m_provisionalPage->process().coreProcessIdentifier()] () mutable {
+            networkProcess->sendWithAsyncReply(Messages::NetworkProcess::AddAllowedFirstPartyForCookies(processIdentifier, navigationDomain, LoadedWebArchive::No), WTFMove(continuation));
+        };
+    }
+
     if (m_inspectorController->shouldPauseLoading(*m_provisionalPage))
         m_inspectorController->setContinueLoadingCallback(*m_provisionalPage, WTFMove(continuation));
     else

@@ -38,12 +38,12 @@ WebDocumentLoader::WebDocumentLoader(const ResourceRequest& request, const Subst
 {
 }
 
-void WebDocumentLoader::detachFromFrame()
+void WebDocumentLoader::detachFromFrame(LoadWillContinueInAnotherProcess loadWillContinueInAnotherProcess)
 {
     if (auto navigationID = std::exchange(m_navigationID, 0))
-        WebFrame::fromCoreFrame(*frame())->documentLoaderDetached(navigationID);
+        WebFrame::fromCoreFrame(*frame())->documentLoaderDetached(navigationID, loadWillContinueInAnotherProcess == LoadWillContinueInAnotherProcess::Yes);
 
-    DocumentLoader::detachFromFrame();
+    DocumentLoader::detachFromFrame(loadWillContinueInAnotherProcess);
 }
 
 void WebDocumentLoader::setNavigationID(uint64_t navigationID)

@@ -46,7 +46,7 @@ class WebDocumentLoader : public WebCore::DocumentLoader {
 private:
     WebDocumentLoader(const WebCore::ResourceRequest&, const WebCore::SubstituteData&);
 
-    void detachFromFrame() override;
+    void detachFromFrame(WebCore::LoadWillContinueInAnotherProcess) override;
 
     uint64_t m_navigationID { 0 };
 };

@@ -1104,9 +1104,9 @@ void WebFrame::setTextDirection(const String& direction)
         localFrame->editor().setBaseWritingDirection(WritingDirection::RightToLeft);
 }
 
-void WebFrame::documentLoaderDetached(uint64_t navigationID)
+void WebFrame::documentLoaderDetached(uint64_t navigationID, bool loadWillContinueInAnotherProcess)
 {
-    if (auto* page = this->page())
+    if (auto* page = this->page(); page && !loadWillContinueInAnotherProcess)
         page->send(Messages::WebPageProxy::DidDestroyNavigation(navigationID));
 }
 

@@ -191,7 +191,7 @@ class WebFrame : public API::ObjectImpl<API::Object::Type::BundleFrame>, public
     void setTextDirection(const String&);
     void updateRemoteFrameSize(WebCore::IntSize);
 
-    void documentLoaderDetached(uint64_t navigationID);
+    void documentLoaderDetached(uint64_t navigationID, bool loadWillContinueInAnotherProcess);
 
     // Simple listener class used by plug-ins to know when frames finish or fail loading.
     class LoadListener : public CanMakeWeakPtr<LoadListener> {

diff --git a/Source/WebKitLegacy/mac/WebView/WebDocumentLoaderMac.h b/Source/WebKitLegacy/mac/WebView/WebDocumentLoaderMac.h
@@ -56,7 +56,7 @@ class WebDocumentLoaderMac : public WebCore::DocumentLoader {
     WebDocumentLoaderMac(const WebCore::ResourceRequest&, const WebCore::SubstituteData&);
 
     virtual void attachToFrame();
-    virtual void detachFromFrame();
+    virtual void detachFromFrame(WebCore::LoadWillContinueInAnotherProcess);
 
     void retainDataSource();
     void releaseDataSource();

diff --git a/Source/WebKitLegacy/mac/WebView/WebDocumentLoaderMac.mm b/Source/WebKitLegacy/mac/WebView/WebDocumentLoaderMac.mm
@@ -82,9 +82,9 @@ static inline bool needsDataLoadWorkaround(WebView *webView)
     retainDataSource();
 }
 
-void WebDocumentLoaderMac::detachFromFrame()
+void WebDocumentLoaderMac::detachFromFrame(LoadWillContinueInAnotherProcess loadWillContinueInAnotherProcess)
 {
-    DocumentLoader::detachFromFrame();
+    DocumentLoader::detachFromFrame(loadWillContinueInAnotherProcess);
 
     if (m_loadingResources.isEmpty())
         releaseDataSource();