Skip to content

Commit

Permalink
Cherry-pick 265870.357@safari-7616-branch (8e677b3). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=260046

    Main frame URL is wrong after server-side redirect to a page serving the COOP header
    https://bugs.webkit.org/show_bug.cgi?id=260046
    rdar://111855179

    Reviewed by Brent Fulgham and Alex Christensen.

    In the poc, the page is opening a popup (without opener) to the same origin URL1.
    This URL1 does a server-side redirect to URL2 which serves the `COOP: same-origin`
    HTTP header. After the navigation, Safari was displaying URL1 instead of URL2 in
    the URL bar.

    It is important to note that that 2 process-swap occur here. The first occurs when
    we do the navigation to URL1 in a popup that doesn't have an opener (in the
    decidePolicyForNavigationAction). The second one occurs when we receive the
    COOP header from URL2 (on navigation response).

    In ProvisionalPageProxy::didCreateMainFrame(), we have code which does the following:
    ```
    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
            // so we need to tell it so it can update its provisional URL.
            m_mainFrame->didStartProvisionalLoad(previousMainFrame->provisionalURL());
        }
    ```

    During the second process-swap, we forward the provisional URL from the committed
    frame to the provisional one. This is because the didStartProvisionalLoad IPC was
    handled by the committed main frame, before we decided to process-swap on resource
    response later on. As a result, the provisional main frame doesn't know yet about
    the provisional load and we have to let it know about it so it sets its provisional
    URL.

    This worked fine in the usual case where the COOP process-swap doesn't follow
    another process swap. However, in this case, the provisional URL got updated by
    an earlier server side redirect which got handled by a provisional frame, not the
    committed one. As a result, the committed frame didn't know about the latest
    provisional URL, only the original one before the server side redirect.

    To address the issue, whenever a provisional main frame receives a server-side
    redirect, we now let the committed main frame know about it too so that the
    committed frame's provisional URL always stays up-to-date. As a result, when
    ProvisionalPageProxy::didCreateMainFrame() forwards the committed frame's URL to
    the new provisional frame, it is now accurate.

    * Source/WebKit/UIProcess/WebPageProxy.cpp:
    (WebKit::WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrameShared):
    * Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

    Canonical link: https://commits.webkit.org/265870.357@safari-7616-branch
  • Loading branch information
cdumez authored and aperezdc committed Oct 19, 2023
1 parent 9e46160 commit 3942867
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 2 deletions.
8 changes: 6 additions & 2 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5506,9 +5506,13 @@ void WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrameShared(Ref<

auto transaction = internals().pageLoadState.transaction();

if (frame->isMainFrame())
if (frame->isMainFrame()) {
internals().pageLoadState.didReceiveServerRedirectForProvisionalLoad(transaction, request.url().string());

// If the main frame in a provisional page is getting a server-side redirect, make sure the
// committed page's provisional URL is kept up-to-date too.
if (frame != m_mainFrame && !m_mainFrame->frameLoadState().provisionalURL().isEmpty())
m_mainFrame->didReceiveServerRedirectForProvisionalLoad(request.url());
}
frame->didReceiveServerRedirectForProvisionalLoad(request.url());

internals().pageLoadState.commitChanges();
Expand Down
49 changes: 49 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7415,6 +7415,55 @@ HTTPServer server({
} while (isFrozen);
}

TEST(ProcessSwap, MainFrameURLAfterServerSideRedirectToCOOP)
{
using namespace TestWebKitAPI;
HTTPServer server({
{ "/source.html"_s, { "<a href='redirect.html' target='_blank' id='testLink'>Click here</a>"_s } },
{ "/redirect.html"_s, { 302, { { "Location"_s, "destination.html"_s } }, "redirecting..."_s } },
{ "/destination.html"_s, { { { "Content-Type"_s, "text/html"_s }, { "Cross-Origin-Opener-Policy"_s, "same-origin"_s } }, "bar"_s } },
}, HTTPServer::Protocol::Https);

auto processPoolConfiguration = psonProcessPoolConfiguration();
auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);

auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
[webViewConfiguration setProcessPool:processPool.get()];
for (_WKFeature *feature in [WKPreferences _features]) {
if ([feature.key isEqualToString:@"CrossOriginOpenerPolicyEnabled"])
[[webViewConfiguration preferences] _setEnabled:YES forFeature:feature];
else if ([feature.key isEqualToString:@"CrossOriginEmbedderPolicyEnabled"])
[[webViewConfiguration preferences] _setEnabled:YES forFeature:feature];
}

auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
[webView setNavigationDelegate:navigationDelegate.get()];
auto uiDelegate = adoptNS([[PSONUIDelegate alloc] initWithNavigationDelegate:navigationDelegate.get()]);
[webView setUIDelegate:uiDelegate.get()];

done = false;
[webView loadRequest:server.request("/source.html"_s)];
Util::run(&done);

done = false;
[webView evaluateJavaScript:@"document.getElementById('testLink').click()" completionHandler:nil];

TestWebKitAPI::Util::run(&didCreateWebView);
didCreateWebView = false;

TestWebKitAPI::Util::run(&done);

auto pid1 = [webView _webProcessIdentifier];
EXPECT_TRUE(!!pid1);
auto pid2 = [createdWebView _webProcessIdentifier];
EXPECT_TRUE(!!pid2);
EXPECT_NE(pid1, pid2);

EXPECT_WK_STREQ(server.request("/destination.html"_s).URL.absoluteString, [[createdWebView URL] absoluteString]);
EXPECT_WK_STREQ(server.request("/destination.html"_s).URL.absoluteString, [[createdWebView _mainFrameURL] absoluteString]);
}

TEST(ProcessSwap, NavigateBackAfterNavigatingAwayFromCOOP)
{
using namespace TestWebKitAPI;
Expand Down

0 comments on commit 3942867

Please sign in to comment.