window.open should be able to handle redirects with site isolation en…

…abled

https://bugs.webkit.org/show_bug.cgi?id=264970
rdar://118522568

Reviewed by Pascoe.

3 things were needed to make this work correctly.

1. In WebPageProxy::decidePolicyForNavigationAction we need to get the correct
   frameProcessBeforeNavigation whether we use a ProvisionalFrameProxy (for non-main frame
   site-isolated navigation) or a ProvisionalPageProxy (for main frame site-isolated
   navigation like window.open).
2. In ProvisionalPageProxy::didCreateMainFrame we need to call didReceiveServerRedirectForProvisionalLoad
   instead of didStartProvisionalLoad because after multiple HTTP redirects we make a ProvisionalPageProxy
   for each redirect.  Future work is already scheduled to fix this, but until then we can make the
   FrameLoadState have the correct state by adding some site-isolation specific logic here.
3. In WebPageProxy::addOpenedRemotePageProxy we need to remove an assertion until that same work is
   complete because we currently make a RemotePageProxy to communicate with each web content process
   along an HTTP redirect chain.

* Source/WebKit/UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::didCreateMainFrame):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
(WebKit::WebPageProxy::addOpenedRemotePageProxy):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
(TestWebKitAPI::openerAndOpenedViews):
(TestWebKitAPI::TEST):

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

Source/WebKit/UIProcess/ProvisionalPageProxy.cpp

@@ -329,7 +329,12 @@ void ProvisionalPageProxy::didCreateMainFrame(FrameIdentifier frameID)
     // If we are process swapping in response to a server redirect then that notification will not come from the new WebContent process.
     // In this case we have the UIProcess synthesize the redirect notification at the appropriate time.
     if (m_isServerRedirect) {
-        m_mainFrame->frameLoadState().didStartProvisionalLoad(m_request.url());
+        // FIXME: When <rdar://116203552> is fixed we should not need this case here
+        // because main frame redirect messages won't come from the web content process.
+        if (m_page->preferences().siteIsolationEnabled() && !m_mainFrame->frameLoadState().provisionalURL().isEmpty())
+            m_mainFrame->frameLoadState().didReceiveServerRedirectForProvisionalLoad(m_request.url());
+        else
+            m_mainFrame->frameLoadState().didStartProvisionalLoad(m_request.url());
         m_page->didReceiveServerRedirectForProvisionalLoadForFrameShared(m_process.copyRef(), m_mainFrame->frameID(), m_navigationID, WTFMove(m_request), { });
     } else if (previousMainFrame && !previousMainFrame->provisionalURL().isEmpty()) {
         // In case of a process swap after response policy, the didStartProvisionalLoad already happened but the new main frame doesn't know about it

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -6647,7 +6647,7 @@ void WebPageProxy::decidePolicyForNavigationAction(Ref<WebProcessProxy>&& proces
     else if (m_needsInitialLinkDecorationFilteringData)
         sendCachedLinkDecorationFilteringData();
 #endif
-    Ref frameProcessBeforeNavigation { frame.provisionalFrame() ? frame.provisionalFrame()->process() : frame.process() };
+    Ref frameProcessBeforeNavigation { frame.provisionalFrame() ? frame.provisionalFrame()->process() : frame.isMainFrame() && m_provisionalPage ? m_provisionalPage->process() : frame.process() };
     Ref listener = frame.setUpPolicyListenerProxy([this, protectedThis = Ref { *this }, frameProcessBeforeNavigation = WTFMove(frameProcessBeforeNavigation), processInitiatingNavigation = process, frame = Ref { frame }, sender = WTFMove(sender), navigation, navigationAction, frameInfo] (PolicyAction policyAction, API::WebsitePolicies* policies, ProcessSwapRequestedByClient processSwapRequestedByClient, RefPtr<SafeBrowsingWarning>&& safeBrowsingWarning, std::optional<NavigatingToAppBoundDomain> isAppBoundDomain, WasNavigationIntercepted wasNavigationIntercepted) mutable {
         WEBPAGEPROXY_RELEASE_LOG(Loading, "decidePolicyForNavigationAction: listener called: frameID=%llu, isMainFrame=%d, navigationID=%llu, policyAction=%u, safeBrowsingWarning=%d, isAppBoundDomain=%d, wasNavigationIntercepted=%d", frame->frameID().object().toUInt64(), frame->isMainFrame(), navigation ? navigation->navigationID() : 0, (unsigned)policyAction, !!safeBrowsingWarning, !!isAppBoundDomain, wasNavigationIntercepted == WasNavigationIntercepted::Yes);
 
@@ -12966,8 +12966,10 @@ void WebPageProxy::removeOpenedRemotePageProxy(WebPageProxyIdentifier pageID)
 
 void WebPageProxy::addOpenedRemotePageProxy(WebPageProxyIdentifier pageID, Ref<RemotePageProxy>&& page)
 {
-    auto addResult = internals().openedRemotePageProxies.add(pageID, WTFMove(page));
-    ASSERT_UNUSED(addResult, addResult.isNewEntry);
+    // FIXME: When <rdar://116203552> is fixed we should be able to assert that the add result is a
+    // new entry because we won't make an opened RemotePageProxy until we know what process
+    // the opened page's main frame will end up in after all redirects.
+    internals().openedRemotePageProxies.add(pageID, WTFMove(page));
 }
 
 #if ENABLE(ADVANCED_PRIVACY_PROTECTIONS)

Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm

@@ -329,7 +329,7 @@ HTTPServer server({
     RetainPtr<TestUIDelegate> uiDelegate;
 };
 
-static std::pair<WebViewAndDelegates, WebViewAndDelegates> openerAndOpenedViews(const HTTPServer& server)
+static std::pair<WebViewAndDelegates, WebViewAndDelegates> openerAndOpenedViews(const HTTPServer& server, NSString *url = @"https://example.com/example")
 {
     __block WebViewAndDelegates opener;
     __block WebViewAndDelegates opened;
@@ -352,7 +352,7 @@ HTTPServer server({
     };
     [opener.webView setUIDelegate:opener.uiDelegate.get()];
     opener.webView.get().configuration.preferences.javaScriptCanOpenWindowsAutomatically = YES;
-    [opener.webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://example.com/example"]]];
+    [opener.webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:url]]];
     while (!opened.webView)
         Util::spinRunLoop();
     [opened.navigationDelegate waitForDidFinishNavigation];
@@ -382,6 +382,34 @@ HTTPServer server({
         Util::spinRunLoop();
 }
 
+TEST(SiteIsolation, WindowOpenRedirect)
+{
+    HTTPServer server({
+        { "/example1"_s, { "<script>w = window.open('https://webkit.org/webkit1')</script>"_s } },
+        { "/webkit1"_s, { 302, { { "Location"_s, "/webkit2"_s } }, "redirecting..."_s } },
+        { "/webkit2"_s, { "loaded!"_s } },
+        { "/example2"_s, { "<script>w = window.open('https://webkit.org/webkit3')</script>"_s } },
+        { "/webkit3"_s, { 302, { { "Location"_s, "https://example.com/example3"_s } }, "redirecting..."_s } },
+        { "/example3"_s, { "loaded!"_s } },
+        { "/example4"_s, { "<script>w = window.open('https://webkit.org/webkit4')</script>"_s } },
+        { "/webkit4"_s, { 302, { { "Location"_s, "https://apple.com/apple"_s } }, "redirecting..."_s } },
+        { "/apple"_s, { "loaded!"_s } },
+    }, HTTPServer::Protocol::HttpsProxy);
+
+    {
+        auto [opener, opened] = openerAndOpenedViews(server, @"https://example.com/example1");
+        EXPECT_WK_STREQ(opened.webView.get().URL.absoluteString, "https://webkit.org/webkit2");
+    }
+    {
+        auto [opener, opened] = openerAndOpenedViews(server, @"https://example.com/example2");
+        EXPECT_WK_STREQ(opened.webView.get().URL.absoluteString, "https://example.com/example3");
+    }
+    {
+        auto [opener, opened] = openerAndOpenedViews(server, @"https://example.com/example4");
+        EXPECT_WK_STREQ(opened.webView.get().URL.absoluteString, "https://apple.com/apple");
+    }
+}
+
 TEST(SiteIsolation, CloseAfterWindowOpen)
 {
     HTTPServer server({