Skip to content

Commit 86b715c

Browse files
committed
REGRESSION(248082@main): wpt /html/browsers/browsing-the-web/overlapping-navigations-and-traversals/tentative/forward-to-pruned-entry.html is flaky
https://bugs.webkit.org/show_bug.cgi?id=243518 <rdar://98082718> Reviewed by Geoffrey Garen. The test calls `history.forward()` which determines that the next HistoryItem is #1 and schedules a navigation to #1. The test then does a synchronous fragment navigation, which prunes the forward HistoryItem from the back/forward list. When the attempt to navigate to HistoryItem #1 in the async task, it should no longer be part of the back/forward and thus no navigation should happen. The navigation to #1 was happening in WebKit however and this was causing the test to be flaky (since the test checks on a timer to see if the navigation to #1 happened or not). WebKit was trying to deal with this by checking BackForwardController::containsItem() in ScheduledHistoryNavigation::fire() and aborting if the BackForwardController no longer contains the HistoryItem. However, in the WebKit2 implementation, the Back / Forward list lives in the UIProcess and WebBackForwardListProxy::containsItem() was failing to ask the UIProcess. Instead, it was relying on the idToHistoryItemMap() map on the WebProcess side. The issue with this is that the map only gets updated asynchronously via IPC from the UIProcess. In the context of the test, we may not have received this IPC from the UIProcess yet when the ScheduledHistoryNavigation fires since the navigation that pruned the HistoryItem was a synchronous fragment navigation. To address the issue, I updated ebBackForwardListProxy::containsItem() to ask the UIProcess instead of relying on idToHistoryItemMap(), for better reliability. * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::backForwardListContainsItem): * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/UIProcess/WebPageProxy.messages.in: * Source/WebKit/WebProcess/WebPage/WebBackForwardListProxy.cpp: (WebKit::WebBackForwardListProxy::containsItem const): Canonical link: https://commits.webkit.org/253121@main
1 parent 801dcc1 commit 86b715c

File tree

4 files changed

+12
-1
lines changed

4 files changed

+12
-1
lines changed

Source/WebKit/UIProcess/WebPageProxy.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6979,6 +6979,11 @@ void WebPageProxy::backForwardGoToItem(const BackForwardItemIdentifier& itemID,
69796979
backForwardGoToItemShared(m_process.copyRef(), itemID, WTFMove(completionHandler));
69806980
}
69816981

6982+
void WebPageProxy::backForwardListContainsItem(const WebCore::BackForwardItemIdentifier& itemID, CompletionHandler<void(bool)>&& completionHandler)
6983+
{
6984+
completionHandler(m_backForwardList->itemForID(itemID));
6985+
}
6986+
69826987
void WebPageProxy::backForwardGoToItemShared(Ref<WebProcessProxy>&& process, const BackForwardItemIdentifier& itemID, CompletionHandler<void(const WebBackForwardListCounts&)>&& completionHandler)
69836988
{
69846989
MESSAGE_CHECK_COMPLETION(m_process, !WebKit::isInspectorPage(*this), completionHandler(m_backForwardList->counts()));

Source/WebKit/UIProcess/WebPageProxy.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2370,6 +2370,7 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>
23702370
// Back/Forward list management
23712371
void backForwardAddItem(BackForwardListItemState&&);
23722372
void backForwardGoToItem(const WebCore::BackForwardItemIdentifier&, CompletionHandler<void(const WebBackForwardListCounts&)>&&);
2373+
void backForwardListContainsItem(const WebCore::BackForwardItemIdentifier&, CompletionHandler<void(bool)>&&);
23732374
void backForwardItemAtIndex(int32_t index, CompletionHandler<void(std::optional<WebCore::BackForwardItemIdentifier>&&)>&&);
23742375
void backForwardListCounts(Messages::WebPageProxy::BackForwardListCountsDelayedReply&&);
23752376
void backForwardClear();

Source/WebKit/UIProcess/WebPageProxy.messages.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ messages -> WebPageProxy {
190190
BackForwardAddItem(struct WebKit::BackForwardListItemState itemState)
191191
BackForwardGoToItem(struct WebCore::BackForwardItemIdentifier itemID) -> (struct WebKit::WebBackForwardListCounts counts) Synchronous
192192
BackForwardItemAtIndex(int32_t itemIndex) -> (std::optional<WebCore::BackForwardItemIdentifier> itemID) Synchronous
193+
BackForwardListContainsItem(struct WebCore::BackForwardItemIdentifier itemID) -> (bool contains) Synchronous
193194
BackForwardListCounts() -> (struct WebKit::WebBackForwardListCounts counts) Synchronous
194195
BackForwardClear()
195196
WillGoToBackForwardListItem(struct WebCore::BackForwardItemIdentifier itemID, bool inBackForwardCache)

Source/WebKit/WebProcess/WebPage/WebBackForwardListProxy.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,11 @@ unsigned WebBackForwardListProxy::forwardListCount() const
141141

142142
bool WebBackForwardListProxy::containsItem(const WebCore::HistoryItem& item) const
143143
{
144-
return idToHistoryItemMap().contains(item.identifier());
144+
// Items are removed asynchronously from idToHistoryItemMap() via IPC from the UIProcess so we need to ask
145+
// the UIProcess to make sure this HistoryItem is still part of the back/forward list.
146+
bool contains = false;
147+
m_page->sendSync(Messages::WebPageProxy::BackForwardListContainsItem(item.identifier()), Messages::WebPageProxy::BackForwardListContainsItem::Reply(contains), m_page->identifier());
148+
return contains;
145149
}
146150

147151
const WebBackForwardListCounts& WebBackForwardListProxy::cacheListCountsIfNecessary() const

0 commit comments

Comments
 (0)