⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 286780 in webkit


Ignore:
Timestamp:
Dec 9, 2021, 7:40:58 AM (5 years ago)
Author:
Tyler Wilcock
Message:

AX: Use RefPtr<AXCoreObject> instead of raw AXCoreObject* pointers in Accessibility::findMatchingObjects and downstream functions
​https://bugs.webkit.org/show_bug.cgi?id=233888

Reviewed by Chris Fleizach.

Move usages of raw AXCoreObject* pointers to RefPtr<AXCoreObject> in
Accessibility::findMatchingObjects and downstream functions.

This fixes isolated tree mode only crashes for tests:

  • accessibility/mac/search-predicate-element-count.html
  • accessibility/mac/search-predicate-visible-button.html

These crashed because:

  1. The secondary thread starts a search and stores raw pointers on its stack.
  2. The main thread performs some operation to queue isolated tree changes.
  3. The search continues on the secondary thread, eventually calling AXIsolatedObject::children. This in turns calls AXIsolatedTree::pendingChanges.
  4. The object(s) which we held pointers to are destroyed.
  • accessibility/AccessibilityObject.cpp:

(WebCore::appendAccessibilityObject):
(WebCore::Accessibility::isRadioButtonInDifferentAdhocGroup):
(WebCore::Accessibility::isAccessibilityObjectSearchMatchAtIndex):
(WebCore::Accessibility::isAccessibilityObjectSearchMatch):
(WebCore::Accessibility::isAccessibilityTextSearchMatch):
(WebCore::Accessibility::objectMatchesSearchCriteriaWithResultLimit):
(WebCore::Accessibility::appendChildrenToArray):
(WebCore::Accessibility::findMatchingObjects):
Use RefPtr<AXCoreObject> instead of raw AXCoreObject* pointers.

Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r286776 r286780  
     12021-12-09  Tyler Wilcock  <tyler_w@apple.com>
     2
     3        AX: Use RefPtr<AXCoreObject> instead of raw AXCoreObject* pointers in Accessibility::findMatchingObjects and downstream functions
     4        https://bugs.webkit.org/show_bug.cgi?id=233888
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Move usages of raw AXCoreObject* pointers to RefPtr<AXCoreObject> in
     9        Accessibility::findMatchingObjects and downstream functions.
     10
     11        This fixes isolated tree mode only crashes for tests:
     12          - accessibility/mac/search-predicate-element-count.html
     13          - accessibility/mac/search-predicate-visible-button.html
     14
     15        These crashed because:
     16
     17          1. The secondary thread starts a search and stores raw pointers on
     18             its stack.
     19          2. The main thread performs some operation to queue isolated tree
     20             changes.
     21          3. The search continues on the secondary thread, eventually calling
     22             AXIsolatedObject::children. This in turns calls AXIsolatedTree::pendingChanges.
     23          4. The object(s) which we held pointers to are destroyed.
     24
     25        * accessibility/AccessibilityObject.cpp:
     26        (WebCore::appendAccessibilityObject):
     27        (WebCore::Accessibility::isRadioButtonInDifferentAdhocGroup):
     28        (WebCore::Accessibility::isAccessibilityObjectSearchMatchAtIndex):
     29        (WebCore::Accessibility::isAccessibilityObjectSearchMatch):
     30        (WebCore::Accessibility::isAccessibilityTextSearchMatch):
     31        (WebCore::Accessibility::objectMatchesSearchCriteriaWithResultLimit):
     32        (WebCore::Accessibility::appendChildrenToArray):
     33        (WebCore::Accessibility::findMatchingObjects):
     34        Use RefPtr<AXCoreObject> instead of raw AXCoreObject* pointers.
     35
    1362021-12-09  Manuel Rego Casasnovas  <rego@igalia.com>
    237

  • trunk/Source/WebCore/accessibility/AccessibilityObject.cpp

    r286406 r286780  
    534534}
    535535
    536 static void appendAccessibilityObject(AXCoreObject* object, AccessibilityObject::AccessibilityChildrenVector& results)
     536static void appendAccessibilityObject(RefPtr<AXCoreObject> object, AccessibilityObject::AccessibilityChildrenVector& results)
    537537{
    538538    // Find the next descendant of this attachment object so search can continue through frames.
    … …  
    39083908// This function determines if the given `axObject` is a radio button part of a different ad-hoc radio group
    39093909// than `referenceObject`, where ad-hoc radio group membership is determined by comparing `name` attributes.
    3910 static bool isRadioButtonInDifferentAdhocGroup(AXCoreObject* axObject, AXCoreObject* referenceObject)
     3910static bool isRadioButtonInDifferentAdhocGroup(RefPtr<AXCoreObject> axObject, AXCoreObject* referenceObject)
    39113911{
    39123912    if (!axObject || !axObject->isRadioButton())
    … …  
    39213921}
    39223922
    3923 static bool isAccessibilityObjectSearchMatchAtIndex(AXCoreObject* axObject, AccessibilitySearchCriteria const& criteria, size_t index)
     3923static bool isAccessibilityObjectSearchMatchAtIndex(RefPtr<AXCoreObject> axObject, AccessibilitySearchCriteria const& criteria, size_t index)
    39243924{
    39253925    switch (criteria.searchKeys[index]) {
    … …  
    40294029}
    40304030
    4031 static bool isAccessibilityObjectSearchMatch(AXCoreObject* axObject, AccessibilitySearchCriteria const& criteria)
     4031static bool isAccessibilityObjectSearchMatch(RefPtr<AXCoreObject> axObject, AccessibilitySearchCriteria const& criteria)
    40324032{
    40334033    if (!axObject)
    … …  
    40454045}
    40464046
    4047 static bool isAccessibilityTextSearchMatch(AXCoreObject* axObject, AccessibilitySearchCriteria const& criteria)
     4047static bool isAccessibilityTextSearchMatch(RefPtr<AXCoreObject> axObject, AccessibilitySearchCriteria const& criteria)
    40484048{
    40494049    if (!axObject)
    … …  
    40594059}
    40604060
    4061 static bool objectMatchesSearchCriteriaWithResultLimit(AXCoreObject* object, AccessibilitySearchCriteria const& criteria, AXCoreObject::AccessibilityChildrenVector& results)
     4061static bool objectMatchesSearchCriteriaWithResultLimit(RefPtr<AXCoreObject> object, AccessibilitySearchCriteria const& criteria, AXCoreObject::AccessibilityChildrenVector& results)
    40624062{
    40634063    if (isAccessibilityObjectSearchMatch(object, criteria) && isAccessibilityTextSearchMatch(object, criteria)) {
    … …  
    40724072}
    40734073
    4074 static void appendChildrenToArray(AXCoreObject* object, bool isForward, AXCoreObject* startObject, AccessibilityObject::AccessibilityChildrenVector& results)
     4074static void appendChildrenToArray(RefPtr<AXCoreObject> object, bool isForward, RefPtr<AXCoreObject> startObject, AccessibilityObject::AccessibilityChildrenVector& results)
    40754075{
    40764076    // A table's children includes elements whose own children are also the table's children (due to the way the Mac exposes tables).
    … …  
    40844084
    40854085    // If the startObject is ignored, we should use an accessible sibling as a start element instead.
    4086     if (startObject && startObject->accessibilityIsIgnored() && startObject->isDescendantOfObject(object)) {
    4087         AXCoreObject* parentObject = startObject->parentObject();
     4086    if (startObject && startObject->accessibilityIsIgnored() && startObject->isDescendantOfObject(object.get())) {
     4087        RefPtr<AXCoreObject> parentObject = startObject->parentObject();
    40884088        // Go up the parent chain to find the highest ancestor that's also being ignored.
    40894089        while (parentObject && parentObject->accessibilityIsIgnored()) {
    … …  
    41104110    if (isForward) {
    41114111        for (size_t i = startIndex; i > endIndex; i--)
    4112             appendAccessibilityObject(searchChildren.at(i - 1).get(), results);
     4112            appendAccessibilityObject(searchChildren.at(i - 1), results);
    41134113    } else {
    41144114        for (size_t i = startIndex; i < endIndex; i++)
    4115             appendAccessibilityObject(searchChildren.at(i).get(), results);
     4115            appendAccessibilityObject(searchChildren.at(i), results);
    41164116    }
    41174117}
    … …  
    41264126
    41274127    // If there's no start object, it means we want to search everything.
    4128     AXCoreObject* startObject = criteria.startObject;
     4128    RefPtr<AXCoreObject> startObject = criteria.startObject;
    41294129    if (!startObject)
    41304130        startObject = criteria.anchorObject;
    … …  
    41354135    // iterating backwards, the start object children should not be considered, so the loop is skipped ahead. We make an
    41364136    // exception when no start object was specified because we want to search everything regardless of search direction.
    4137     AXCoreObject* previousObject = nullptr;
     4137    RefPtr<AXCoreObject> previousObject;
    41384138    if (!isForward && startObject != criteria.anchorObject) {
    41394139        previousObject = startObject;
    … …  
    41514151        // This now does a DFS at the current level of the parent.
    41524152        while (!searchStack.isEmpty()) {
    4153             AXCoreObject* searchObject = searchStack.last().get();
     4153            auto searchObject = searchStack.last();
    41544154            searchStack.removeLast();
    41554155

  • trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h

    r286406 r286780  
    16831683{
    16841684    return axObject && Accessibility::findAncestor<AXCoreObject>(*this, false, [axObject] (const AXCoreObject& object) {
    1685             return &object == axObject;
    1686         }) != nullptr;
     1685        return &object == axObject;
     1686    }) != nullptr;
    16871687}
    16881688
Note: See TracChangeset for help on using the changeset viewer.