Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: c71c8ea9721a124f9013da3611269a193e74af4b
https://github.com/WebKit/WebKit/commit/c71c8ea9721a124f9013da3611269a193e74af4b
Author: Ben Nham <n...@apple.com>
Date: 2025-03-07 (Fri, 07 Mar 2025)
Changed paths:
A
LayoutTests/http/tests/navigation/back-in-iframe-does-not-stop-loading-expected.txt
A
LayoutTests/http/tests/navigation/back-in-iframe-does-not-stop-loading.html
A
LayoutTests/http/tests/navigation/resources/back-in-iframe-does-not-stop-loading-iframe.html
M Source/WebCore/loader/HistoryController.cpp
Log Message:
-----------
Calling history.back() in an iframe stops all pending network requests
https://bugs.webkit.org/show_bug.cgi?id=289294
rdar://146430092
Reviewed by Brady Eidson.
Going back in an iframe stops all outstanding network requests. This is because
`Page::goToItem`
will ask FrameLoader to stop all requests if we are doing a cross-doc
navigation (as defined by
`HistoryItem::shouldDoSameDocumentNavigationTo`). However, in the case where
the target top-level
HistoryItem is the same as the current top-level HistoryItem (as in the case of
going back in an
iframe), `shouldDoSameDocumentNavigationTo` always returns false. So even
though we're actually
staying in the same document, we still end up canceling all outstanding loads.
I thought about changing `shouldDoSameDocumentNavigationTo`, but that seems
like it might be risky
and risk further breakage, since that heuristic hasn't been changed in many
years.
I also thought about removing the call to eagerly stop all loaders at
`Page::goToItem` time
entirely. It seems like we could just wait for those loaders to stop naturally
once the document
navigation commits. However, changing this logic also seems potentially risky
since it seems like we
started to depend on eager stopping to prevent some crashes (see r79107).
So I ended up keeping the eager call to stop all loaders in `Page::goToItem`,
but it's now only
called if we are actually changing document objects as a result of the history
navigation (as defined
by a differing document sequence numbers in the history item).
Some history I pulled up here trying to understand the original logic:
1. Brady first added the call to `FrameLoader::stopAllLoaders` to
`Page::goToItem` in r18541. There
is not much explanation for why this was added. Perhaps this is due to
wanting to make sure
`history.go(0)` acts similar to a reload (see r55375).
2. We exempted documents with only a state change from eager `stopAllLoaders`
logic in r51644.
3. We started making `shouldDoSameDocumentNavigationTo` return false when
targeting the current item
in r61207 (refined in r68742) to fix a crash.
4. We exempted documents with only a hash change from eager `stopAllLoaders`
logic in r79107.
However, this change didn't fix the issue for hash changes in iframes (this
bug) due to the
change in (3).
*
LayoutTests/http/tests/navigation/back-in-iframe-does-not-stop-loading-expected.txt:
Added.
* LayoutTests/http/tests/navigation/back-in-iframe-does-not-stop-loading.html:
Added.
*
LayoutTests/http/tests/navigation/resources/back-in-iframe-does-not-stop-loading-iframe.html:
Added.
* Source/WebCore/loader/HistoryController.cpp:
(WebCore::HistoryController::shouldStopLoadingForHistoryItem const):
(WebCore::HistoryController::goToItem):
(WebCore::HistoryController::goToItemForNavigationAPI):
Canonical link: https://commits.webkit.org/291823@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
