Improve the usage of smart pointers in AXObjectCache
<rdar://problem/114090467>
rdar://113803539
Created attachment 467328 [details] Patch
Comment on attachment 467328 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=467328&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:2291 > + RefPtr protectedElement { element }; > + > if (nodeAndRendererAreValid(element) && rendererNeedsDeferredUpdate(*element->renderer())) { > - m_deferredAttributeChange.append({ element, attrName, oldValue, newValue }); > + m_deferredAttributeChange.append({ protectedElement, attrName, oldValue, newValue }); > if (!m_performCacheUpdateTimer.isActive()) > m_performCacheUpdateTimer.startOneShot(0_s); > AXLOG(makeString("Deferring handling of attribute ", attrName.localName().string(), " for element ", element->debugDescription())); > return; > } > - handleAttributeChange(element, attrName, oldValue, newValue); > + handleAttributeChange(protectedElement.get(), attrName, oldValue, newValue); I don't think we need to protect this element with a RefPtr if !rendererNeedsDeferredUpdate(*element->renderer()) (because otherwise we append it to m_deferredAttributeChange which stores it as a WeakPtr). I think we only need to protect it if we're going to directly call handleAttributeChange with it.
(In reply to Tyler Wilcock from comment #4) > Comment on attachment 467328 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=467328&action=review > > > - handleAttributeChange(element, attrName, oldValue, newValue); > > + handleAttributeChange(protectedElement.get(), attrName, oldValue, newValue); > > I don't think we need to protect this element with a RefPtr if > !rendererNeedsDeferredUpdate(*element->renderer()) (because otherwise we > append it to m_deferredAttributeChange which stores it as a WeakPtr). I > think we only need to protect it if we're going to directly call > handleAttributeChange with it. Gotcha—yeah that makes sense. I can move the refptr protection to before the handleAttributeChange call.
Created attachment 467330 [details] Patch
Committed 267064@main (4e6fd1fb5461): <https://commits.webkit.org/267064@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 467330 [details].
(In reply to Joshua Hoffman from comment #6) > Created attachment 467330 [details] > Patch --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ b/Source/WebCore/accessibility/AXObjectCache.cpp @@ -2178,10 +2178,10 @@ void AXObjectCache::handleActiveDescendantChange(Element& element, const AtomStr } // Handle active-descendant changes when the target allows for it, or the controlled object allows for it. - AccessibilityObject* target = nullptr; + RefPtr<AccessibilityObject> target { nullptr }; The RefPtr default constructor initializes the pointer to nullptr so the initializer list is unnecessary here.