Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 3d130b4a9d6acef3b925d9ec46abfb4352ecf7ff
https://github.com/WebKit/WebKit/commit/3d130b4a9d6acef3b925d9ec46abfb4352ecf7ff
Author: Ryosuke Niwa <[email protected]>
Date: 2026-09-01 (Tue, 01 Sep 2026)
Changed paths:
M Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
M Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h
M Source/WebCore/page/scrolling/ScrollingStateNode.cpp
M Source/WebCore/page/scrolling/ScrollingStateNode.h
M Source/WebCore/page/scrolling/ScrollingStateTree.cpp
M Source/WebCore/page/scrolling/ScrollingStateTree.h
M Source/WebCore/page/scrolling/ScrollingTree.h
M Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.cpp
M
Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp
M Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h
M Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp
M Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.h
M Source/WebKit/UIProcess/WebPageProxy.cpp
M
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.h
M
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.messages.in
M
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm
Log Message:
-----------
[Site Isolation]
imported/w3c/web-platform-tests/dom/events/scrolling/save-iframe-scroll-offset-when-display-none.html
fails
https://bugs.webkit.org/show_bug.cgi?id=322968
Reviewed by Simon Fraser.
imported/w3c/web-platform-tests/dom/events/scrolling/save-iframe-scroll-offset-when-display-none.html
intermittently hit an assertion in the UI process under site isolation.
imported/w3c/web-platform-tests/dom/events/scrolling/save-iframe-scroll-offset-when-display-none.html
intermittently hit an assertion in the UI process under site isolation.
The iframe's ScrollingTreeFrameScrollingNodeMac had a default-constructed
m_scrolledContentsLayer, i.e. its layer had never been set.
Setting the iframe to display:none destroys the parent's RenderIFrame, so the
parent's ScrollingStateFrameHostingNode goes away. On the next commit the UI
process sees that node as unvisited and destroys it, and willBeDestroyed()
calls removeHostedChildren(). Since updateTreeFromStateNodeRecursive() leaves
commitState.frameHostingNode set for the whole hosted recursion, every node of
the hosted subtree is registered as a hosted child, so that removes the
iframe's entire subtree from m_nodeMap.
Nothing tells the iframe's process about this; its ScrollingStateTree is
untouched. When the iframe is shown again and that process commits,
ScrollingStateTree::commit() clones the full node structure but the transaction
only encodes properties flagged as changed (the OptionalTupleBit entries in
RemoteScrollingCoordinatorTransaction.serialization.in).
updateTreeFromStateNodeRecursive() misses in m_nodeMap and creates fresh nodes,
and commitStateBeforeChildren() only assigns members whose properties changed,
so the layers stay unset and applyLayerPositions() dereferences one.
Whether this reproduces depends on whether the hosted process happens to
rebuild its scrolling state root - which marks every property changed - in the
same commit that reattaches the subtree, hence the intermittency.
Fix this by making the two sides resync rather than by changing what the UI
process keeps. removeHostedChildren() is the one place where the UI process
discards state that the hosted process still believes in, reached from
willBeDestroyed(), from setLayerHostingContextIdentifier() on a context change,
and from the empty-commit path in commitTreeStateInternal(). It now notifies
through a new ScrollingTree::hostedSubtreeNeedsFullCommit(FrameIdentifier).
RemoteScrollingTree forwards that to RemoteScrollingCoordinatorProxy, which
reuses the existing WebPageProxy::sendToProcessContainingFrame(), so no new
hosting-context-to-process map is needed. In the hosted process,
RemoteScrollingCoordinator::requestFullScrollingTreeCommit() calls
AsyncScrollingCoordinator::setAllScrollingStatePropertiesChangedForRootFrameID(),
which marks every applicable property of every node in that frame's state tree
as changed and schedules a commit. The next transaction is then self-contained,
so the recreated nodes get their layers.
Removal semantics are deliberately unchanged, so removeFromActiveNodes() still
runs for every removed node and 319425@main's fix for stale active nodes in
establishLayerTreeScrollingRelations() still holds. An earlier attempt that
detached the subtree instead of removing it regressed
http/tests/site-isolation/scrolling/remove-iframe-with-active-scroll-proxy.html
for exactly that reason, and would also have kept the iframe's CALayers alive
for the life of the page.
Notes on the details:
- The message carries a FrameIdentifier because AsyncScrollingCoordinator keeps
one ScrollingStateTree per root frame in m_scrollingStateTrees and a process
can host several (see
http/tests/site-isolation/scrolling/multiple-root-frames.html).
Addressing only the process would be ambiguous.
- The frame identifier is read from any hosted child, since they all come from
the same hosted commit and therefore share commitState.frameId. It is captured
before the removal loop because removeNode() drops the entries.
- Nested cross-process iframes fall out naturally: removing frame A's subtree
destroys the nested FrameHosting node for B, whose own removeHostedChildren()
notifies B.
- setAllPropertiesChanged() also walks m_unparentedNodes, since insertNode()
only marks the nodes it actually reparents.
- The dirty-bit walk this reuses was named after its only caller, so it is
renamed for its second one: nodeWasReattachedRecursive() becomes
setAllPropertiesChangedRecursive(), and the node-level
setPropertyChangesAfterReattach() becomes setAllApplicablePropertiesChanged(),
which is what it does. The "the ScrollingTree recreates the node from scratch"
comment moves to the reattach call site in insertNode(), since it explains
that caller rather than the walk itself.
One ordering gap remains: if the hosted process already had an incremental
commit in flight when the request was sent, that stale commit lands first and
briefly recreates layer-less nodes. Closing that requires a "this is a full
commit" bit on RemoteScrollingCoordinatorTransaction so the UI process can skip
hosted commits until the full one arrives, which is left for a follow-up.
Test:
imported/w3c/web-platform-tests/dom/events/scrolling/save-iframe-scroll-offset-when-display-none.html
* Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::setAllScrollingStatePropertiesChangedForRootFrameID):
* Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h:
* Source/WebCore/page/scrolling/ScrollingStateNode.cpp:
(WebCore::ScrollingStateNode::setAllApplicablePropertiesChanged):
(WebCore::ScrollingStateNode::setPropertyChangesAfterReattach): Deleted.
* Source/WebCore/page/scrolling/ScrollingStateNode.h:
* Source/WebCore/page/scrolling/ScrollingStateTree.cpp:
(WebCore::setAllPropertiesChangedRecursive):
(WebCore::ScrollingStateTree::insertNode):
(WebCore::ScrollingStateTree::setAllPropertiesChanged):
(WebCore::nodeWasReattachedRecursive): Deleted.
* Source/WebCore/page/scrolling/ScrollingStateTree.h:
* Source/WebCore/page/scrolling/ScrollingTree.h:
(WebCore::ScrollingTree::hostedSubtreeNeedsFullCommit):
* Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.cpp:
(WebCore::ScrollingTreeFrameHostingNode::removeHostedChildren):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:
(WebKit::RemoteScrollingCoordinatorProxy::requestFullScrollingTreeCommitForFrame):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h:
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.cpp:
(WebKit::RemoteScrollingTree::hostedSubtreeNeedsFullCommit):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingTree.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.h:
*
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.messages.in:
*
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteScrollingCoordinator.mm:
(WebKit::RemoteScrollingCoordinator::requestFullScrollingTreeCommit):
Canonical link: https://commits.webkit.org/320230@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
