Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 26559c13662dfdf3815d1e73fef7e3f56314b967
https://github.com/WebKit/WebKit/commit/26559c13662dfdf3815d1e73fef7e3f56314b967
Author: Basuke Suzuki <[email protected]>
Date: 2026-06-16 (Tue, 16 Jun 2026)
Changed paths:
M Source/WebCore/page/Navigation.cpp
M Source/WebCore/page/Navigation.h
Log Message:
-----------
Navigation API: extract a MethodTrackerRegistry for the upcoming/ongoing
tracker slots
https://bugs.webkit.org/show_bug.cgi?id=316879
rdar://179303305
Reviewed by Chris Dumez and Rupin Mittal.
m_apiMethodTrackersLock guarded three Navigation member slots that hold
NavigationAPIMethodTrackers (the ongoing tracker, the upcoming
non-traverse slot, and the upcoming-traverse map). All mutation happens
on the main thread; the lock exists only because
visitAdditionalChildrenInGCThread visits the trackers' info() values
from the GC thread.
The lock granularity was correct, but its use was dispersed: about a
dozen call sites hand-rolled "lock, copy RefPtr, unlock" blocks, nothing
prevented new code from touching a slot without the lock, and the
long-standing FIXMEs about ongoing/upcoming-slot invariants had nowhere
to live.
The promoteUpcomingAPIMethodTracker(const String&) algorithm also
encoded the traverse / non-traverse distinction in
null-vs-empty-vs-non-empty String semantics, with three branches:
if (!destinationKey.isEmpty()) // take from upcoming-traverse
else if (destinationKey.isNull()) // take from upcoming-non-traverse
else if (destinationKey.isEmpty() && !upcoming-traverse.isEmpty())
// take HashMap first entry
The third branch was unreachable: NavigationDestination::key() returns
either a UUID (non-null, non-empty) or null, never a non-null empty
string, and even if reachable its HashMap-iteration-order semantics
were unspecified-behavior. The second branch was also a latent spec
deviation: if a navigation.navigate() call had registered an upcoming
non-traverse tracker, and a browser-driven cross-document Traverse then
arrived whose target HistoryItem wasn't in m_entries (so destination
key is null), the second branch would steal the non-traverse tracker
to satisfy the traversal — wiring the navigate()'s committed/finished
promises to an unrelated traversal's outcome. That has been removed
along with the rest of the old dispatch.
Move the slots, the lock, and the spec's slot algorithms (promote-an-
upcoming-api-method-tracker-to-ongoing, navigation-api-method-tracker-
clean-up, the upcoming-traverse lookup, the GC-thread visit) into a
private nested class, MethodTrackerRegistry. Navigation keeps tracker
*creation* (it needs the global object and the promises); the registry
only stores, promotes, looks up, and cleans up. Its API:
setUpcomingNonTraverse(Ref<NavigationAPIMethodTracker>&&);
addUpcomingTraverse(const String& key, Ref<NavigationAPIMethodTracker>&&);
upcomingTraverse(const String& key) const -> RefPtr<...>;
ongoing() const -> RefPtr<...>;
takeUpcomingNonTraverseIfEquals(NavigationAPIMethodTracker&) -> RefPtr<...>;
promoteUpcomingNonTraverseToOngoing() -> RefPtr<...>;
promoteUpcomingTraverseToOngoing(const String& destinationKey) ->
RefPtr<...>;
unregister(NavigationAPIMethodTracker&);
isEmpty() const -> bool;
visitInGCThread(JSC::AbstractSlotVisitor&) const;
The traverse / non-traverse distinction is now encoded as two methods
the caller picks based on its existing navigationType discriminator,
instead of a runtime branch on a String's null/empty state. The two
promote methods return the newly-promoted tracker, so the call site
inside innerDispatchNavigateEvent gets the ongoing tracker without a
second lock acquisition. takeUpcomingNonTraverseIfEquals returns the
taken tracker (or null) instead of a bool, matching the rest of the
registry's take→RefPtr shape. The pure slot-op methods on Navigation
(promoteUpcomingAPIMethodTracker, cleanupAPIMethodTracker) are removed;
their call sites now invoke the registry directly. The two facades
that still mix tracker creation with policy
(maybeSetUpcomingNonTraversalTracker, addUpcomingTraverseAPIMethodTracker)
stay on Navigation and forward storage to the registry.
upcomingTraverseMethodTracker(const String&) — the public accessor used
by NavigationScheduler — now returns RefPtr instead of a raw pointer.
The returned pointer was previously fetched under the lock and used
after the lock released; the existing caller already assigned to a
local RefPtr at the call site, so the change is type-compatible and
makes lifetime safe by construction.
All Locker use and WTF_GUARDED_BY_LOCK annotations now live in one
screenful inside the registry. The pre-existing FIXME assertions about
the ongoing-tracker and upcoming-traverse-key invariants also moved
into the registry's promote / unregister paths.
This is preparation for the upcoming NavigationInterceptOptions
precommitHandler work: with the registry in place, precommit support
reduces to one more tracker state (added in 316812@main) and one more
registry slot, instead of another round of flags threaded through
Navigation.
Behavior change limited to the previously-described latent fix in the
null-key Traverse path; existing imported/w3c/web-platform-tests/
navigation-api tests pass in both release and debug.
* Source/WebCore/page/Navigation.h:
(WebCore::Navigation::MethodTrackerRegistry):
* Source/WebCore/page/Navigation.cpp:
(WebCore::Navigation::maybeSetUpcomingNonTraversalTracker):
(WebCore::Navigation::addUpcomingTraverseAPIMethodTracker):
(WebCore::Navigation::navigate):
(WebCore::Navigation::performTraversal):
(WebCore::Navigation::resolveFinishedPromise):
(WebCore::Navigation::rejectFinishedPromise):
(WebCore::Navigation::notifyCommittedToEntry):
(WebCore::Navigation::updateForNavigation):
(WebCore::Navigation::abortOngoingNavigation):
(WebCore::Navigation::handleSameDocumentNavigation):
(WebCore::Navigation::innerDispatchNavigateEvent):
(WebCore::Navigation::visitAdditionalChildrenInGCThread):
(WebCore::Navigation::MethodTrackerRegistry::setUpcomingNonTraverse):
(WebCore::Navigation::MethodTrackerRegistry::addUpcomingTraverse):
(WebCore::Navigation::MethodTrackerRegistry::upcomingTraverse):
(WebCore::Navigation::MethodTrackerRegistry::ongoing):
(WebCore::Navigation::MethodTrackerRegistry::takeUpcomingNonTraverseIfEquals):
(WebCore::Navigation::MethodTrackerRegistry::promoteUpcomingNonTraverseToOngoing):
(WebCore::Navigation::MethodTrackerRegistry::promoteUpcomingTraverseToOngoing):
(WebCore::Navigation::MethodTrackerRegistry::unregister):
(WebCore::Navigation::MethodTrackerRegistry::isEmpty):
(WebCore::Navigation::MethodTrackerRegistry::visitInGCThread):
(WebCore::Navigation::upcomingTraverseMethodTracker):
(WebCore::Navigation::cleanupAPIMethodTracker): Deleted.
(WebCore::Navigation::promoteUpcomingAPIMethodTracker): Deleted.
Canonical link: https://commits.webkit.org/315301@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
