AX: AccessibilityRenderObject::updateRoleAfterChildrenCreation causes…

… unnecessary AXRoleChanged notifications for several subclasses

https://bugs.webkit.org/show_bug.cgi?id=262351
rdar://problem/116213145

Reviewed by Andres Gonzalez.

AccessibilityRenderObject::updateRoleAfterChildrenCreation is predicated on the assumption that the
class uses m_role. However, many subclasses don't, instead hard-coding a value like so:

AccessibilityRole roleValue() const final { return AccessibilityRole::Label; }

This means that m_role (not actually used) may differ from roleValue(), causing updateRoleAfterChildrenCreation
to post a spurious AXRoleChanged notification every time the object's children are cleared and re-added, in turn
causing lots of wasted work, as AXRoleChanged causes a full node update in the isolated tree.

With this patch, we move most subclasses towards using `m_role` rather than overriding roleValue(),
preventing spurious AXRoleChanged notifications (because now we will properly detect that the role has not changed).

This saves 10k node changes of 132k total node changes on an internal web page, as this webpage has thousands
of SVG elements that cleared and re-added their children a few times.

* Source/WebCore/accessibility/AXImage.cpp:
(WebCore::AXImage::determineAccessibilityRole):
(WebCore::AXImage::roleValue const): Deleted.
* Source/WebCore/accessibility/AXImage.h:
* Source/WebCore/accessibility/AccessibilityAttachment.h:
* Source/WebCore/accessibility/AccessibilityImageMapLink.cpp:
(WebCore::AccessibilityImageMapLink::setHTMLAreaElement):
(WebCore::AccessibilityImageMapLink::determineAccessibilityRole):
(WebCore::AccessibilityImageMapLink::detachFromParent):
(WebCore::AccessibilityImageMapLink::roleValue const): Deleted.
* Source/WebCore/accessibility/AccessibilityImageMapLink.h:
* Source/WebCore/accessibility/AccessibilityLabel.h:
* Source/WebCore/accessibility/AccessibilityList.cpp:
(WebCore::AccessibilityList::determineAccessibilityRole):
(WebCore::AccessibilityList::roleValue const): Deleted.
* Source/WebCore/accessibility/AccessibilityList.h:
* Source/WebCore/accessibility/AccessibilityListBox.h:
* Source/WebCore/accessibility/AccessibilityListBoxOption.h:
* Source/WebCore/accessibility/AccessibilityMenuList.h:
* Source/WebCore/accessibility/AccessibilityMenuListOption.h:
* Source/WebCore/accessibility/AccessibilityMenuListPopup.h:
* Source/WebCore/accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::init):
(WebCore::AccessibilityNodeObject::computeAccessibilityIsIgnored const):
(WebCore::AccessibilityNodeObject::updateRole): Deleted.
* Source/WebCore/accessibility/AccessibilityNodeObject.h:
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::init):
(WebCore::AccessibilityObject::updateRole):
* Source/WebCore/accessibility/AccessibilityObject.h:
(WebCore::AccessibilityObject::init): Deleted.
(WebCore::AccessibilityObject::updateRole): Deleted.
* Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:
(WebCore::AccessibilityProgressIndicator::determineAccessibilityRole):
(WebCore::AccessibilityProgressIndicator::roleValue const): Deleted.
* Source/WebCore/accessibility/AccessibilityProgressIndicator.h:
* Source/WebCore/accessibility/AccessibilityRenderObject.h:
* Source/WebCore/accessibility/AccessibilitySVGElement.h:
* Source/WebCore/accessibility/AccessibilitySVGRoot.cpp:
(WebCore::AccessibilitySVGRoot::determineAccessibilityRole):
(WebCore::AccessibilitySVGRoot::roleValue const): Deleted.
* Source/WebCore/accessibility/AccessibilitySVGRoot.h:
* Source/WebCore/accessibility/AccessibilityScrollView.h:
* Source/WebCore/accessibility/AccessibilityScrollbar.h:
* Source/WebCore/accessibility/AccessibilitySlider.h:
* Source/WebCore/accessibility/AccessibilitySpinButton.h:
* Source/WebCore/accessibility/AccessibilityTable.h:
* Source/WebCore/accessibility/AccessibilityTableColumn.h:
* Source/WebCore/accessibility/AccessibilityTableHeaderContainer.h:

Canonical link: https://commits.webkit.org/268789@main

Source/WebCore/accessibility/AXImage.cpp

@@ -47,12 +47,10 @@ Ref<AXImage> AXImage::create(RenderImage* renderer)
     return adoptRef(*new AXImage(renderer));
 }
 
-AccessibilityRole AXImage::roleValue() const
+AccessibilityRole AXImage::determineAccessibilityRole()
 {
-    auto ariaRole = ariaRoleAttribute();
-    if (ariaRole != AccessibilityRole::Unknown)
-        return ariaRole;
-
+    if ((m_ariaRole = determineAriaRoleAttribute()) != AccessibilityRole::Unknown)
+        return m_ariaRole;
     return AccessibilityRole::Image;
 }
 

Source/WebCore/accessibility/AXImage.h

@@ -41,7 +41,7 @@ class AXImage : public AccessibilityRenderObject {
 private:
     explicit AXImage(RenderImage*);
 
-    AccessibilityRole roleValue() const override;
+    AccessibilityRole determineAccessibilityRole() final;
     std::optional<AccessibilityChildrenVector> imageOverlayElements() override;
 };
 

Source/WebCore/accessibility/AccessibilityAttachment.h

@@ -42,7 +42,8 @@ class AccessibilityAttachment final : public AccessibilityRenderObject {
     bool hasProgress(float* progress = nullptr) const;
 
 private:
-    AccessibilityRole roleValue() const override { return AccessibilityRole::Button; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::Button; }
+
     bool isAttachmentElement() const override { return true; }
 
     String roleDescription() const override;

Source/WebCore/accessibility/AccessibilityImageMapLink.cpp

@@ -52,6 +52,15 @@ Ref<AccessibilityImageMapLink> AccessibilityImageMapLink::create()
     return adoptRef(*new AccessibilityImageMapLink());
 }
 
+void AccessibilityImageMapLink::setHTMLAreaElement(HTMLAreaElement* element)
+{
+    if (element == m_areaElement)
+        return;
+    m_areaElement = element;
+    // AccessibilityImageMapLink::determineAccessibilityRole() depends on m_areaElement, so re-compute it now.
+    updateRole();
+}
+
 AccessibilityObject* AccessibilityImageMapLink::parentObject() const
 {
     if (m_parent)
@@ -63,7 +72,7 @@ AccessibilityObject* AccessibilityImageMapLink::parentObject() const
     return m_mapElement->document().axObjectCache()->getOrCreate(m_mapElement->renderer());
 }
 
-AccessibilityRole AccessibilityImageMapLink::roleValue() const
+AccessibilityRole AccessibilityImageMapLink::determineAccessibilityRole()
 {
     if (!m_areaElement)
         return AccessibilityRole::WebCoreLink;

Source/WebCore/accessibility/AccessibilityImageMapLink.h

@@ -39,15 +39,15 @@ class AccessibilityImageMapLink final : public AccessibilityMockObject {
     static Ref<AccessibilityImageMapLink> create();
     virtual ~AccessibilityImageMapLink();
 
-    void setHTMLAreaElement(HTMLAreaElement* element) { m_areaElement = element; }
+    void setHTMLAreaElement(HTMLAreaElement*);
     HTMLAreaElement* areaElement() const { return m_areaElement.get(); }
 
     void setHTMLMapElement(HTMLMapElement* element) { m_mapElement = element; }    
     HTMLMapElement* mapElement() const { return m_mapElement.get(); }
 
     Node* node() const override { return m_areaElement.get(); }
 
-    AccessibilityRole roleValue() const override;
+    AccessibilityRole determineAccessibilityRole() final;
     bool isEnabled() const override { return true; }
 
     Element* anchorElement() const override;
@@ -70,8 +70,8 @@ class AccessibilityImageMapLink final : public AccessibilityMockObject {
     bool isImageMapLink() const final { return true; }
     bool supportsPath() const override { return true; }
 
-    RefPtr<HTMLAreaElement> m_areaElement;
-    RefPtr<HTMLMapElement> m_mapElement;
+    WeakPtr<HTMLAreaElement, WeakPtrImplWithEventTargetData> m_areaElement;
+    WeakPtr<HTMLMapElement, WeakPtrImplWithEventTargetData> m_mapElement;
 };
 
 } // namespace WebCore

Source/WebCore/accessibility/AccessibilityLabel.h

@@ -43,7 +43,9 @@ class AccessibilityLabel final : public AccessibilityRenderObject {
 private:
     explicit AccessibilityLabel(RenderObject*);
     bool computeAccessibilityIsIgnored() const final;
-    AccessibilityRole roleValue() const final { return AccessibilityRole::Label; }
+
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::Label; }
+
     bool isLabel() const final { return true; }
     String stringValue() const final;
     void updateChildrenIfNecessary() final;

Source/WebCore/accessibility/AccessibilityList.cpp

@@ -145,7 +145,7 @@ AccessibilityRole AccessibilityList::determineAccessibilityRole()
     auto role = AccessibilityRole::List;
 
     // Temporarily set role so that we can query children (otherwise canHaveChildren returns false).
-    m_role = role;
+    SetForScope temporaryRole(m_role, role);
 
     unsigned listItemCount = 0;
     bool hasVisibleMarkers = false;
@@ -193,11 +193,5 @@ AccessibilityRole AccessibilityList::determineAccessibilityRole()
 
     return role;
 }
-
-AccessibilityRole AccessibilityList::roleValue() const
-{
-    ASSERT(m_role != AccessibilityRole::Unknown);
-    return m_role;
-}
-
+
 } // namespace WebCore

Source/WebCore/accessibility/AccessibilityList.h

@@ -38,7 +38,6 @@ class AccessibilityList final : public AccessibilityRenderObject {
     static Ref<AccessibilityList> create(Node&);
     virtual ~AccessibilityList();
 
-    AccessibilityRole roleValue() const override;
 private:
     explicit AccessibilityList(RenderObject*);
     explicit AccessibilityList(Node&);

Source/WebCore/accessibility/AccessibilityListBox.h

@@ -39,8 +39,9 @@ class AccessibilityListBox final : public AccessibilityRenderObject {
 
     bool canSetSelectedChildren() const override;
     WEBCORE_EXPORT void setSelectedChildren(const AccessibilityChildrenVector&) override;
-    AccessibilityRole roleValue() const override { return AccessibilityRole::ListBox; }
-
+
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::ListBox; }
+
     AccessibilityChildrenVector selectedChildren() final;
     AccessibilityChildrenVector visibleChildren() final;
 

Source/WebCore/accessibility/AccessibilityListBoxOption.h

@@ -46,7 +46,7 @@ class AccessibilityListBoxOption final : public AccessibilityObject {
 private:
     explicit AccessibilityListBoxOption(HTMLElement&);
 
-    AccessibilityRole roleValue() const final { return AccessibilityRole::ListBoxOption; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::ListBoxOption; }
     bool isEnabled() const final;
     bool isSelectedOptionActive() const final;
     String stringValue() const final;

Source/WebCore/accessibility/AccessibilityMenuList.h

@@ -44,7 +44,8 @@ class AccessibilityMenuList final : public AccessibilityRenderObject {
     explicit AccessibilityMenuList(RenderMenuList*);
 
     bool isMenuList() const final { return true; }
-    AccessibilityRole roleValue() const override { return AccessibilityRole::PopUpButton; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::PopUpButton; }
+
     bool canSetFocusAttribute() const override;
 
     void addChildren() override;

Source/WebCore/accessibility/AccessibilityMenuListOption.h

@@ -41,7 +41,7 @@ class AccessibilityMenuListOption final : public AccessibilityObject {
 
     bool isMenuListOption() const final { return true; }
 
-    AccessibilityRole roleValue() const final { return AccessibilityRole::MenuListOption; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::MenuListOption; }
     bool canHaveChildren() const final { return false; }
     AccessibilityObject* parentObject() const final { return m_parent.get(); }
 

Source/WebCore/accessibility/AccessibilityMenuListPopup.h

@@ -49,7 +49,7 @@ class AccessibilityMenuListPopup final : public AccessibilityMockObject {
     bool isMenuListPopup() const final { return true; }
 
     LayoutRect elementRect() const final { return LayoutRect(); }
-    AccessibilityRole roleValue() const override { return AccessibilityRole::MenuListPopup; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::MenuListPopup; }
 
     bool isVisible() const override;
     bool press() override;

Source/WebCore/accessibility/AccessibilityNodeObject.cpp

@@ -108,7 +108,7 @@ void AccessibilityNodeObject::init()
     ASSERT(!m_initialized);
     m_initialized = true;
 #endif
-    m_role = determineAccessibilityRole();
+    AccessibilityObject::init();
 }
 
 Ref<AccessibilityNodeObject> AccessibilityNodeObject::create(Node& node)
@@ -123,16 +123,6 @@ void AccessibilityNodeObject::detachRemoteParts(AccessibilityDetachmentType deta
     m_node = nullptr;
 }
 
-void AccessibilityNodeObject::updateRole()
-{
-    auto previousRole = m_role;
-    m_role = determineAccessibilityRole();
-    if (previousRole != m_role) {
-        if (auto* cache = axObjectCache())
-            cache->handleRoleChanged(this);
-    }
-}
-
 AccessibilityObject* AccessibilityNodeObject::firstChild() const
 {
     auto* currentChild = node() ? node()->firstChild() : nullptr;
@@ -669,10 +659,8 @@ bool AccessibilityNodeObject::computeAccessibilityIsIgnored() const
     if (isDescendantOfBarrenParent())
         return true;
 
-    if (roleValue() == AccessibilityRole::Ignored)
-        return true;
-
-    return m_role == AccessibilityRole::Unknown;
+    auto role = roleValue();
+    return role == AccessibilityRole::Ignored || role == AccessibilityRole::Unknown;
 }
 
 bool AccessibilityNodeObject::canvasHasFallbackContent() const

Source/WebCore/accessibility/AccessibilityNodeObject.h

@@ -137,7 +137,6 @@ class AccessibilityNodeObject : public AccessibilityObject {
     AccessibilityObject* parentObject() const override;
     AccessibilityObject* parentObjectIfExists() const override;
 
-    void updateRole() override;
     bool matchesTextAreaRole() const;
 
     void increment() override;
@@ -156,7 +155,7 @@ class AccessibilityNodeObject : public AccessibilityObject {
 
     bool isDetached() const override { return !m_node; }
 
-    virtual AccessibilityRole determineAccessibilityRole();
+    AccessibilityRole determineAccessibilityRole() override;
     enum class TreatStyleFormatGroupAsInline : bool { No, Yes };
     AccessibilityRole determineAccessibilityRoleFromNode(TreatStyleFormatGroupAsInline = TreatStyleFormatGroupAsInline::No) const;
     AccessibilityRole ariaRoleAttribute() const override { return m_ariaRole; }

Source/WebCore/accessibility/AccessibilityObject.cpp

@@ -107,6 +107,11 @@ AccessibilityObject::~AccessibilityObject()
     ASSERT(isDetached());
 }
 
+void AccessibilityObject::init()
+{
+    m_role = determineAccessibilityRole();
+}
+
 inline ProcessID AccessibilityObject::processID() const
 {
     return presentingApplicationPID();
@@ -2770,6 +2775,16 @@ String AccessibilityObject::computedRoleString() const
     return reverseAriaRoleMap().get(static_cast<int>(role));
 }
 
+void AccessibilityObject::updateRole()
+{
+    auto previousRole = m_role;
+    m_role = determineAccessibilityRole();
+    if (previousRole != m_role) {
+        if (auto* cache = axObjectCache())
+            cache->handleRoleChanged(this);
+    }
+}
+
 bool AccessibilityObject::hasHighlighting() const
 {
     for (Node* node = this->node(); node; node = node->parentNode()) {

Source/WebCore/accessibility/AccessibilityObject.h

@@ -79,7 +79,7 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
     // After constructing an AccessibilityObject, it must be given a
     // unique ID, then added to AXObjectCache, and finally init() must
     // be called last.
-    virtual void init() { }
+    virtual void init();
 
     // Prefer using the dedicated functions over consuming these flag values directly, as the flags can sometimes be uninitialized.
     // Also, the dedicated functions traverse for you if the flags aren't yet initialized.
@@ -416,7 +416,9 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
     // Only if isColorWell()
     SRGBA<uint8_t> colorValue() const override;
 
+    // FIXME: This should be made final after AccessibilityTable is fixed to use m_role rather than computing its own roleValue().
     AccessibilityRole roleValue() const override { return m_role; }
+    virtual AccessibilityRole determineAccessibilityRole() = 0;
     String rolePlatformString() const override;
     String roleDescription() const override;
     String subrolePlatformString() const override;
@@ -499,7 +501,7 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
     void increment() override { }
     void decrement() override { }
 
-    virtual void updateRole() { }
+    virtual void updateRole();
     bool childrenInitialized() const { return m_childrenInitialized; }
     const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded = true) override;
     virtual void addChildren() { }

Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp

@@ -118,7 +118,7 @@ float AccessibilityProgressIndicator::minValueForRange() const
     return 0.0;
 }
 
-AccessibilityRole AccessibilityProgressIndicator::roleValue() const
+AccessibilityRole AccessibilityProgressIndicator::determineAccessibilityRole()
 {
     if (meterElement())
         return AccessibilityRole::Meter;

Source/WebCore/accessibility/AccessibilityProgressIndicator.h

@@ -37,7 +37,7 @@ class AccessibilityProgressIndicator final : public AccessibilityRenderObject {
     bool isIndeterminate() const final;
 
 private:
-    AccessibilityRole roleValue() const override;
+    AccessibilityRole determineAccessibilityRole() final;
 
     String valueDescription() const override;
     String gaugeRegionValueDescription() const;

Source/WebCore/accessibility/AccessibilityRenderObject.h

@@ -189,8 +189,8 @@ class AccessibilityRenderObject : public AccessibilityNodeObject {
 #endif
     String expandedTextValue() const override;
     bool supportsExpandedTextValue() const override;
-    void updateRoleAfterChildrenCreation();
-    
+    virtual void updateRoleAfterChildrenCreation();
+
     bool inheritsPresentationalRole() const override;
 
     bool shouldGetTextFromNode(AccessibilityTextUnderElementMode) const;

Source/WebCore/accessibility/AccessibilitySVGElement.h

@@ -39,13 +39,13 @@ class AccessibilitySVGElement : public AccessibilityRenderObject {
 protected:
     explicit AccessibilitySVGElement(RenderObject*, AXObjectCache*);
     AXObjectCache* axObjectCache() const override { return m_axObjectCache.get(); }
+    AccessibilityRole determineAriaRoleAttribute() const final;
 
 private:
     String description() const final;
     String helpText() const final;
     void accessibilityText(Vector<AccessibilityText>&) const final;
-    AccessibilityRole determineAccessibilityRole() final;
-    AccessibilityRole determineAriaRoleAttribute() const final;
+    AccessibilityRole determineAccessibilityRole() override;
     bool inheritsPresentationalRole() const final;
     bool computeAccessibilityIsIgnored() const final;
 

Source/WebCore/accessibility/AccessibilitySVGRoot.cpp

@@ -66,12 +66,10 @@ AccessibilityObject* AccessibilitySVGRoot::parentObject() const
     return AccessibilitySVGElement::parentObject();
 }
 
-AccessibilityRole AccessibilitySVGRoot::roleValue() const
+AccessibilityRole AccessibilitySVGRoot::determineAccessibilityRole()
 {
-    AccessibilityRole ariaRole = ariaRoleAttribute();
-    if (ariaRole != AccessibilityRole::Unknown)
-        return ariaRole;
-
+    if ((m_ariaRole = determineAriaRoleAttribute()) != AccessibilityRole::Unknown)
+        return m_ariaRole;
     return AccessibilityRole::Group;
 }
 

Source/WebCore/accessibility/AccessibilitySVGRoot.h

@@ -45,7 +45,8 @@ class AccessibilitySVGRoot final : public AccessibilitySVGElement {
 
     AccessibilityObject* parentObject() const override;
     bool isAccessibilitySVGRoot() const override { return true; }
-    AccessibilityRole roleValue() const override;
+
+    AccessibilityRole determineAccessibilityRole() final;
 
     WeakPtr<AccessibilityRenderObject> m_parent;
 };

Source/WebCore/accessibility/AccessibilityScrollView.h

@@ -37,7 +37,7 @@ class ScrollView;
 class AccessibilityScrollView final : public AccessibilityObject {
 public:
     static Ref<AccessibilityScrollView> create(ScrollView*);
-    AccessibilityRole roleValue() const override { return AccessibilityRole::ScrollArea; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::ScrollArea; }
     ScrollView* scrollView() const override { return currentScrollView(); }
 
     virtual ~AccessibilityScrollView();

Source/WebCore/accessibility/AccessibilityScrollbar.h

@@ -48,7 +48,7 @@ class AccessibilityScrollbar final : public AccessibilityMockObject {
     bool isAccessibilityScrollbar() const override { return true; }
     LayoutRect elementRect() const override;
 
-    AccessibilityRole roleValue() const override { return AccessibilityRole::ScrollBar; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::ScrollBar; }
     AccessibilityOrientation orientation() const override;
     Document* document() const override;
     bool isEnabled() const override;

Source/WebCore/accessibility/AccessibilitySlider.h

@@ -47,7 +47,8 @@ class AccessibilitySlider : public AccessibilityRenderObject {
     HTMLInputElement* inputElement() const;
     AXCoreObject* elementAccessibilityHitTest(const IntPoint&) const override;
 
-    AccessibilityRole roleValue() const override { return AccessibilityRole::Slider; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::Slider; }
+
     bool isControl() const override { return true; }
 
     void addChildren() override;
@@ -67,7 +68,7 @@ class AccessibilitySliderThumb final : public AccessibilityMockObject {
     static Ref<AccessibilitySliderThumb> create();
     virtual ~AccessibilitySliderThumb() = default;
 
-    AccessibilityRole roleValue() const override { return AccessibilityRole::SliderThumb; }
+    AccessibilityRole determineAccessibilityRole() final { return AccessibilityRole::SliderThumb; }
     LayoutRect elementRect() const override;
 
 private: