Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: f3f75e999f1ddd211c21e69c3af81b978e4b92a0
https://github.com/WebKit/WebKit/commit/f3f75e999f1ddd211c21e69c3af81b978e4b92a0
Author: Alexey Proskuryakov <[email protected]>
Date: 2026-08-13 (Thu, 13 Aug 2026)
Changed paths:
A
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-in-detached-frame-expected.txt
A
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-in-detached-frame.html
A
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-cross-document-navigation-expected.txt
A
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-cross-document-navigation.html
A
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-fragment-navigation-expected.txt
A
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-fragment-navigation.html
A
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-nested-click-expected.txt
A
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-nested-click.html
A
LayoutTests/fast/dom/HTMLAnchorElement/resources/anchor-download-then-navigate-frame-2.html
A
LayoutTests/fast/dom/HTMLAnchorElement/resources/anchor-download-then-navigate-frame.html
M Source/WebCore/loader/FrameLoader.cpp
M Source/WebKit/UIProcess/WebFrameProxy.cpp
M Source/WebKit/UIProcess/WebFrameProxy.h
M Source/WebKit/UIProcess/WebPageProxy.cpp
M Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
M Source/WebKit/WebProcess/WebPage/WebFrame.cpp
M Source/WebKit/WebProcess/WebPage/WebFrame.h
M Tools/TestWebKitAPI/Tests/WebKit/WKWebView/Download.mm
Log Message:
-----------
Same-frame fragment navigation that starts after a download attribute click,
in the same task, cancels the download
https://bugs.webkit.org/show_bug.cgi?id=302895
rdar://165498647
Reviewed by Brady Eidson and Alex Christensen.
Activating a link with a download attribute runs "download the hyperlink" [1],
not "navigate", and its
steps run in parallel with the navigable. WebKit instead routed it through the
frame's single navigation
policy slot, so the fragment navigation in FrameLoader::loadURL() called
PolicyChecker::stopCheck(), which
answered every outstanding check with PolicyAction::Ignore. The download's
decision then never arrived as
PolicyAction::Download, so startDownload() was never called and nothing
happened.
There are two such cancellations, in both processes, and both have to go. In
the web process
PolicyChecker::stopCheck() answers every outstanding check with Ignore. In the
UI process WebFrameProxy
keeps one policy listener per frame and setUpPolicyListenerProxy() resolves the
previous one with
ignore(), so the navigation's check displaces the download's there regardless
of what the web process
does. Download checks are now kept out of both.
A download check therefore outlives the navigation that started it, so record
the document that
initiated it and only honor it while that document is still the one in this
frame and the frame is
still in a page.
The frame check avoids a crash: detaching a frame starts no navigation, so
nothing else answers the check
and the decision arrives for a frame that has left its page. The document check
matters because
PolicyChecker decides against the frame's live state, including the sandbox
allow-downloads flag that
LocalFrame::effectiveSandboxFlags() takes from the current document. Keeping
the check out of the UI
process cancellation is what makes that reachable: a decision can now arrive
after another document has
committed, and would otherwise be evaluated against it.
Also stop a late decision from tearing down a newer navigation.
The policy delegate is still consulted for these loads: WKNavigationAction's
shouldPerformDownload is
unchanged, and both Download and Allow are still honored. Allow is honored in
one case where it was
previously dropped, since the check it belongs to is no longer cancelled.
WebKitLegacy is left unfixed: its WebFrameLoaderClient::cancelPolicyCheck()
invalidates a single
policy listener and cannot tell a download check apart.
[1] https://html.spec.whatwg.org/multipage/links.html#downloading-hyperlinks
Tests: TestWebKitAPI.WKDownload.DownloadAttributeAllowedByClientIsANavigation
TestWebKitAPI.WKDownload.DownloadAttributeSurvivesLaterNavigation
fast/dom/HTMLAnchorElement/anchor-download-in-detached-frame.html
fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-cross-document-navigation.html
fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-fragment-navigation.html
fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-nested-click.html
* Source/WebCore/loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadWithDocumentLoader): Don't continue once a newer
navigation owns the policy
document loader, which hits the entry assertion in
continueLoadAfterNavigationPolicy() and tears down that
newer navigation. Only a check that survived cancellation can get there.
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): Record
the document initiating a
download attribute check.
* Source/WebKit/UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::webProcessWillShutDown): Answer download checks, which
navigation no longer
cancels, so their replies are not lost with the frame.
(WebKit::WebFrameProxy::setUpPolicyListenerProxy): Keep download checks in
their own list so they neither
displace the navigation check nor are displaced by it.
* Source/WebKit/UIProcess/WebFrameProxy.h: Added IsDownloadPolicyCheck and the
list of outstanding download
listeners. They are keyed by an ID rather than held in a single member because,
unlike navigation checks,
several can be outstanding at once now that none of them displaces another.
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationAction): Mark a check for an
action with a download
attribute.
(WebKit::WebPageProxy::decidePolicyForNewWindowAction): Neither of these can be
a download attribute
check, so both pass No.
(WebKit::WebPageProxy::decidePolicyForResponseShared): Ditto.
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::invalidate): Answer any check still outstanding, rather than
destroying its
CompletionHandler. Navigation used to have cancelled them all by this point. No
test: for frame detach the
page condition above already cancels the check first, and I could not find
another path that reaches here
with one outstanding.
(WebKit::WebFrame::setUpPolicyListener): Store it.
(WebKit::WebFrame::shouldHonorDownloadAttributePolicyCheck): Added.
(WebKit::WebFrame::invalidatePolicyListeners): Leave still-valid download
attribute checks outstanding
rather than ignoring them. Reinstate them before cancelling the rest, as
cancelling can add checks back.
(WebKit::WebFrame::didReceivePolicyDecision): Ignore a download attribute check
that should no longer be
honored, and skip navigationID and origin-keying only when the decision is
actually Download - answering
Use for such a link makes it an ordinary navigation. Answer rather than drop a
check for a frame whose
core frame is gone, which previously left an uncalled CompletionHandler behind.
* Source/WebKit/WebProcess/WebPage/WebFrame.h: Added the initiating document to
PolicyCheck and to
setUpPolicyListener(). It is a Markable rather than a separate flag plus an
identifier so that a download
attribute check cannot exist without the document it has to be validated
against, and so that its presence
is what tells such a check apart from a navigation one. Declared
shouldHonorDownloadAttributePolicyCheck().
* Tools/TestWebKitAPI/Tests/WebKit/WKWebView/Download.mm:
(TEST(WKDownload, DownloadAttributeAllowedByClientIsANavigation)): Added.
Clicks a download attribute link
that the navigation delegate answers Use for, and checks the client is given
the same non-null WKNavigation
on didStartProvisionalNavigation and didFinishNavigation. Skipping navigationID
for such a check leaves the
UI process unable to match the load to its API::Navigation, and both are null.
Serves both documents from
one custom scheme so the attribute is not dropped as cross origin, and as
displayable HTML so the load
commits rather than becoming a download at the response stage.
(TEST(WKDownload, DownloadAttributeSurvivesLaterNavigation)): Added. The
delegate does not answer the
download decision when asked: it holds it until a later navigation of the same
frame has been decided, then
answers Download, and the test checks the file is written. Holding it is what
leaves the decision
outstanding while the frame navigates, which is the state both cancellations
destroy. A delegate that
answers immediately never reaches that state, which is also why
WebKitTestRunner, and so any layout test,
cannot cover the UI process half.
*
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-in-detached-frame-expected.txt:
Added.
*
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-in-detached-frame.html:
Added. Clicks a download
attribute link in a subframe and then removes the subframe, and checks that
nothing crashes and no download
callbacks are logged. The decision arrives for a frame that has left its page,
which is the one thing
navigation no longer does for us.
*
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-cross-document-navigation-expected.txt:
Added.
*
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-cross-document-navigation.html:
Added.
A subframe clicks a download attribute link and then activates a link to
another document. Child frames are
dumped, so this checks both that the download completes and that the subframe
reaches the second document.
Activating a link navigates synchronously, unlike assigning location.href, so
the newer navigation owns the
policy document loader before the decision arrives; without the FrameLoader
change that decision tears the
newer navigation down and trips the assertion at the top of
continueLoadAfterNavigationPolicy().
*
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-fragment-navigation-expected.txt:
Added.
*
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-fragment-navigation.html:
Added.
Clicks a download attribute link and then sets location.hash, and checks the
download still starts and
finishes. This is the reduced form of the bug: no nesting is involved. The hash
toggles between two values
so it stays a real fragment navigation whatever fragment the document was
loaded with, which keeps the test
working when it is repeated.
*
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-nested-click-expected.txt:
Added.
*
LayoutTests/fast/dom/HTMLAnchorElement/anchor-download-not-cancelled-by-nested-click.html:
Added. The form
the bug was reported in: a link with href="#" whose click handler clicks a
download attribute link, so the
fragment navigation comes from the outer link's default action once the handler
returns.
*
LayoutTests/fast/dom/HTMLAnchorElement/resources/anchor-download-then-navigate-frame-2.html:
Added.
*
LayoutTests/fast/dom/HTMLAnchorElement/resources/anchor-download-then-navigate-frame.html:
Added.
Canonical link: https://commits.webkit.org/319105@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications