Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 52a7fd41f9066841d26530ac96f5c0310d42e1c0
https://github.com/WebKit/WebKit/commit/52a7fd41f9066841d26530ac96f5c0310d42e1c0
Author: Tyler Wilcock <[email protected]>
Date: 2025-01-23 (Thu, 23 Jan 2025)
Changed paths:
M Source/WebCore/accessibility/AXObjectCache.cpp
M Source/WebCore/accessibility/AXObjectCache.h
M Source/WebCore/dom/Node.cpp
Log Message:
-----------
AX: Make m_nodeObjectMapping a WeakHashMap to avoid WeakRef crashes
https://bugs.webkit.org/show_bug.cgi?id=286398
rdar://143446394
Reviewed by Chris Fleizach.
We made an attempt to fix this crash in https://commits.webkit.org/289060@main,
but have since found a website
where this bug reproduces consistently, confirming that the speculative fix
from that commit did not work. The crash
is reproducible by turning VoiceOver on and starting a Speedometer run at
https://browserbench.org/Speedometer3.0.
Fundamentally, we are failing to remove nodes from m_nodeObjectMapping, which
prior to this commit was of type:
UncheckedKeyHashMap<WeakRef<Node, WeakPtrImplWithEventTargetData>, AXID>
This happens due to this sequence:
1. We add a node to the map
2. The document that node belongs to gets its m_frame set to nullptr via
Document::willDetachFrame().
3. When that node is destroyed, we can no longer get an
existingAXObjectCache() to clean it up, since
the node's document has no m_frame. The WeakRef for that node in
m_nodeObjectMapping becomes null.
4. Something else tries to access m_nodeObjectMapping, causing a lookup.
During the lookup, when we try
to compare against the null WeakRef, we crash.
One option would be to clear out AXObjectCache::m_nodeObjectMapping when
willDetachFrame() is called. But its not clear
that's always the right thing to do, as the document and frame lifecycle is
complicated when considering things like
the back-forward cache.
As a stop-gap fix to prevent the crashes and ensure at least
m_nodeObjectMapping is cleaned up (even though we're still
missing clean-up for other things in AXObjectCache::remove(Node&)), make
m_nodeObjectMapping a WeakHashMap. This is
an improvement over status quo — we stop crashing, and we leak less. This seems
fine for now, as this scenario seems
very rare.
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::get const):
(WebCore::AXObjectCache::remove):
(WebCore::AXObjectCache::onRendererCreated):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::~Node):
Canonical link: https://commits.webkit.org/289293@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
