[webkit-changes] [WebKit/WebKit] 2b008f: Using Cross-Origin-Opener-Policy HTTP header may c...

[Chris Dumez]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 2b008f6776a29ebcc21676061f084bbcfc53d0ed
      https://github.com/WebKit/WebKit/commit/2b008f6776a29ebcc21676061f084bbcfc53d0ed
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2024-10-26 (Sat, 26 Oct 2024)

  Changed paths:
    M Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp
    M Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.h
    M Source/WebKit/WebProcess/WebPage/WebPage.cpp
    M Source/WebKit/WebProcess/WebPage/WebPage.h
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

  Log Message:
  -----------
  Using Cross-Origin-Opener-Policy HTTP header may corrupt the back/forward list
https://bugs.webkit.org/show_bug.cgi?id=281175
rdar://137635838

Reviewed by Sihui Liu.

The reason the back/forward list was getting "corrupted" is that we would process-swap
on link navigation but use an API::Navigation object from an earlier back/forward
navigation, instead of create a new API::Navigation object for this new navigation.
This would cause us to think this is a back/forward navigation and we would incorrectly
overwrite the current back/forward item.

The reason we were using the wrong API::Navigation object is that this is a navigation
triggered by the page (not the client app). As a result, the navigation request comes
from the WebProcess to the UIProcess and this request contained a navigationID which
was the navigationID of the previous same-document back navigation. (If the navigation
was trigger by the client app, we would create a new navigation object right away and
pass this navigationID to the WebProcess).

Now the reason why the WebProcess was sending us an old navigationID is because when
the WebProcess did the last same-document back navigation, it stored the navigationID
in WebPage::m_pendingNavigationID. Normally, this navigationID would get set on the
DocumentLoader later on when it gets created and m_pendingNavigationID would get cleared.
Here, because it was a same document navigation, no DocumentLoader gets created. Our
code was dealing with that by overriding `dispatchDidChangeLocationWithinPage()` and
clearing m_pendingNavigationID then. However, `dispatchDidChangeLocationWithinPage()`
only gets called on fragment navigations. The reproduction site here was using a same
document navigation using `history.pushState()` but the new URL would append `/foo` to
the URL instead of `#foo`, which is allowed. It's still a same-document navigation but
not a fragment navigation. As a result, it wouldn't call `dispatchDidChangeLocationWithinPage()`
and it wouldn't clear m_pendingNavigationID. Then when the next link navigation would
occur, it would end up mistakenly reusing the leftover m_pendingNavigationID.

To address the issue, I now override `dispatchDidNavigateWithinPage()` too and clear
`WebPage::m_pendingNavigationID` when it gets called. `dispatchDidNavigateWithinPage()`
gets called on all same-document navigations.

This is covered by a new API test.

* Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp:
(WebKit::WebLocalFrameLoaderClient::dispatchDidNavigateWithinPage):
* Source/WebKit/WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didNavigateWithinPageForFrame):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
((ProcessSwap, NavigateBackAfterNavigatingAwayFromCrossOriginOpenerPolicyUsingBackForwardCache2)):

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



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications