Skip to content

Commit

Permalink
Cherry-pick c047b91. rdar://problem/111341681
Browse files Browse the repository at this point in the history
    AX: Improve smart pointer hygiene in AXObjectCache and AXIsolatedObject::updateBackingStore
    rdar://111341681

    Reviewed by Chris Fleizach.

    Per https://github.com/WebKit/WebKit/wiki/Smart-Pointer-Usage-Guidelines, continue refactoring
    to replace raw pointer usage with smart pointers where appropriate.

    * Source/WebCore/accessibility/AXObjectCache.cpp:
    (WebCore::AXObjectCache::remove):
    (WebCore::AXObjectCache::deferNodeAddedOrRemoved):
    (WebCore::AXObjectCache::prepareForDocumentDestruction):
    (WebCore::AXObjectCache::performDeferredCacheUpdate):
    * Source/WebCore/accessibility/AXObjectCache.h:
    * Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:

    Canonical link: https://commits.webkit.org/259548.864@safari-7615-branch
Identifier: 259548.865@safari-7615.3.12.10-branch
  • Loading branch information
twilco authored and MyahCobbs committed Jun 29, 2023
1 parent c9372c6 commit 28f24db
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 9 deletions.
1 change: 1 addition & 0 deletions Source/WebCore/accessibility/AXLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ void AXLogger::log(const String& collectionName, const AXObjectCache::DeferredCo
[&size] (const Vector<std::pair<Node*, Node*>>& typedCollection) { size = typedCollection.size(); },
[&size] (const WeakHashSet<Element, WeakPtrImplWithEventTargetData>& typedCollection) { size = typedCollection.computeSize(); },
[&size] (const WeakHashSet<HTMLTableElement, WeakPtrImplWithEventTargetData>& typedCollection) { size = typedCollection.computeSize(); },
[&size] (const WeakListHashSet<Node, WeakPtrImplWithEventTargetData>& typedCollection) { size = typedCollection.computeSize(); },
[] (auto&) {
ASSERT_NOT_REACHED();
return;
Expand Down
19 changes: 13 additions & 6 deletions Source/WebCore/accessibility/AXObjectCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ void AXObjectCache::remove(Node& node)
m_deferredModalChangedList.remove(downcast<Element>(node));
m_deferredMenuListChange.remove(downcast<Element>(node));
}
m_deferredNodeAddedOrRemovedList.remove(&node);
m_deferredNodeAddedOrRemovedList.remove(node);
m_deferredTextChangedList.remove(&node);
// Remove the entry if the new focused node is being removed.
m_deferredFocusedNodeChange.removeAllMatching([&node](auto& entry) -> bool {
Expand Down Expand Up @@ -1167,7 +1167,7 @@ void AXObjectCache::deferNodeAddedOrRemoved(Node* node)
if (!node)
return;

m_deferredNodeAddedOrRemovedList.add(node);
m_deferredNodeAddedOrRemovedList.add(*node);

if (is<Element>(node)) {
auto* changedElement = downcast<Element>(node);
Expand Down Expand Up @@ -3411,12 +3411,19 @@ static void filterWeakHashSetForRemoval(WeakHashSet& weakHashSet, const Document
});
}

template<typename WeakListHashSetType>
static void filterWeakListHashSetForRemoval(WeakListHashSetType& list, const Document& document, HashSet<Ref<Node>>& nodesToRemove)
{
for (auto& node : list)
conditionallyAddNodeToFilterList(&node, document, nodesToRemove);
}

void AXObjectCache::prepareForDocumentDestruction(const Document& document)
{
HashSet<Ref<Node>> nodesToRemove;
filterListForRemoval(m_textMarkerNodes, document, nodesToRemove);
filterListForRemoval(m_deferredTextChangedList, document, nodesToRemove);
filterListForRemoval(m_deferredNodeAddedOrRemovedList, document, nodesToRemove);
filterWeakListHashSetForRemoval(m_deferredNodeAddedOrRemovedList, document, nodesToRemove);
filterWeakHashSetForRemoval(m_deferredRecomputeIsIgnoredList, document, nodesToRemove);
filterWeakHashSetForRemoval(m_deferredRecomputeTableIsExposedList, document, nodesToRemove);
filterWeakHashSetForRemoval(m_deferredSelectedChildredChangedList, document, nodesToRemove);
Expand Down Expand Up @@ -3479,9 +3486,9 @@ void AXObjectCache::performDeferredCacheUpdate()
m_deferredRecomputeTableIsExposedList.clear();

AXLOGDeferredCollection("NodeAddedOrRemovedList"_s, m_deferredNodeAddedOrRemovedList);
for (auto* nodeChild : m_deferredNodeAddedOrRemovedList) {
handleMenuOpened(nodeChild);
handleLiveRegionCreated(nodeChild);
for (auto& nodeChild : m_deferredNodeAddedOrRemovedList) {
handleMenuOpened(&nodeChild);
handleLiveRegionCreated(&nodeChild);
}
m_deferredNodeAddedOrRemovedList.clear();

Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/accessibility/AXObjectCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>
, Vector<AttributeChange>
, Vector<std::pair<Node*, Node*>>
, WeakHashSet<Element, WeakPtrImplWithEventTargetData>
, WeakHashSet<HTMLTableElement, WeakPtrImplWithEventTargetData>>;
, WeakHashSet<HTMLTableElement, WeakPtrImplWithEventTargetData>
, WeakListHashSet<Node, WeakPtrImplWithEventTargetData>>;
void deferFocusedUIElementChangeIfNeeded(Node* oldFocusedNode, Node* newFocusedNode);
void deferModalChange(Element*);
void deferMenuListValueChange(Element*);
Expand Down Expand Up @@ -595,7 +596,7 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>
ListHashSet<Node*> m_deferredTextChangedList;
WeakHashSet<Element, WeakPtrImplWithEventTargetData> m_deferredSelectedChildredChangedList;
ListHashSet<RefPtr<AccessibilityObject>> m_deferredChildrenChangedList;
ListHashSet<Node*> m_deferredNodeAddedOrRemovedList;
WeakListHashSet<Node, WeakPtrImplWithEventTargetData> m_deferredNodeAddedOrRemovedList;
WeakHashSet<Element, WeakPtrImplWithEventTargetData> m_deferredModalChangedList;
WeakHashSet<Element, WeakPtrImplWithEventTargetData> m_deferredMenuListChange;
HashMap<Element*, String> m_deferredTextFormControlValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -943,7 +943,7 @@ void AXIsolatedObject::fillChildrenVectorForProperty(AXPropertyName propertyName
void AXIsolatedObject::updateBackingStore()
{
// This method can be called on either the main or the AX threads.
if (auto tree = this->tree())
if (RefPtr tree = this->tree())
tree->applyPendingChanges();
// AXIsolatedTree::applyPendingChanges can cause this object and / or the AXIsolatedTree to be destroyed.
// Make sure to protect `this` with a Ref before adding more logic to this function.
Expand Down

0 comments on commit 28f24db

Please sign in to comment.