Changeset 241289 in webkit


Ignore:
Timestamp:
Feb 12, 2019 1:28:44 AM (5 years ago)
Author:
Chris Fleizach
Message:

AXObjectCache::childrenChanged shouldn't update layout or style during another style recalc
https://bugs.webkit.org/show_bug.cgi?id=182280
<rdar://problem/37018386>

Reviewed by Alan Bujtas.

Source/WebCore:

Remove the possibility that changing children calls back into updating layout by
handling children changes in a deferred manner.

This follows the same architecture as many other deferred changes, but also requires us to check deferred changes
in updateBackingStore, because things like aria-hidden changes won't trigger a layout, but will require us to update children.

A few tests had to be modified to no longer change the tree and then check the children immediately.

  • accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::remove):
(WebCore::AXObjectCache::childrenChanged):
(WebCore::AXObjectCache::prepareForDocumentDestruction):
(WebCore::AXObjectCache::performDeferredCacheUpdate):

  • accessibility/AXObjectCache.h:
  • accessibility/AccessibilityObject.cpp:

(WebCore::AccessibilityObject::updateBackingStore):

  • accessibility/mac/WebAccessibilityObjectWrapperBase.mm:

(convertToNSArray):
(-[WebAccessibilityObjectWrapperBase updateObjectBackingStore]):

LayoutTests:

  • accessibility/aria-hidden-update.html:
  • accessibility/aria-hidden-updates-alldescendants.html:
  • accessibility/image-load-on-delay.html:
  • accessibility/mac/aria-hidden-changes-for-non-ignored-elements.html:
  • accessibility/removed-anonymous-block-child-causes-crash.html:
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r241288 r241289  
     12019-02-08  Chris Fleizach  <cfleizach@apple.com>
     2
     3        AXObjectCache::childrenChanged shouldn't update layout or style during another style recalc
     4        https://bugs.webkit.org/show_bug.cgi?id=182280
     5        <rdar://problem/37018386>
     6
     7        Reviewed by Alan Bujtas.
     8
     9        * accessibility/aria-hidden-update.html:
     10        * accessibility/aria-hidden-updates-alldescendants.html:
     11        * accessibility/image-load-on-delay.html:
     12        * accessibility/mac/aria-hidden-changes-for-non-ignored-elements.html:
     13        * accessibility/removed-anonymous-block-child-causes-crash.html:
     14
    1152019-02-11  Myles C. Maxfield  <mmaxfield@apple.com>
    216
  • trunk/LayoutTests/accessibility/aria-hidden-update.html

    r155274 r241289  
    1919    <script>
    2020        if (window.accessibilityController) {
     21            jsTestIsAsync = true;
    2122            description("This test makes sure that when aria-hidden changes, the AX hierarchy is updated.");
    2223
     
    4041            // Make the 2nd button hidden. Only 1 and 3 should be present.
    4142            document.getElementById("button2").setAttribute("aria-hidden", "true");
    42             shouldBeTrue("parent.childAtIndex(0).isEqual(button1)");
    43             shouldBeTrue("parent.childAtIndex(1).isEqual(button3)");
     43            setTimeout(function() {
     44                shouldBeTrue("parent.childAtIndex(0).isEqual(button1)");
     45                shouldBeTrue("parent.childAtIndex(1).isEqual(button3)");
    4446           
    45             // Make the 1st button hidden. Only 3 should be present.
    46             document.getElementById("button1").setAttribute("aria-hidden", "true");
    47             shouldBeTrue("parent.childAtIndex(0).isEqual(button3)");
     47                // Make the 1st button hidden. Only 3 should be present.
     48                document.getElementById("button1").setAttribute("aria-hidden", "true");
     49                setTimeout(function() {
     50                    shouldBeTrue("parent.childAtIndex(0).isEqual(button3)");
    4851
    49             // Make the 2nd button not hidden. 2 and 3 should be present.
    50             document.getElementById("button2").setAttribute("aria-hidden", "false");
    51             shouldBeTrue("parent.childAtIndex(0).isEqual(button2)");
    52             shouldBeTrue("parent.childAtIndex(1).isEqual(button3)");
    53 
     52                    // Make the 2nd button not hidden. 2 and 3 should be present.
     53                    document.getElementById("button2").setAttribute("aria-hidden", "false");
     54                    setTimeout(function() {
     55                        shouldBeTrue("parent.childAtIndex(0).isEqual(button2)");
     56                        shouldBeTrue("parent.childAtIndex(1).isEqual(button3)");
     57                        finishJSTest();
     58                    }, 0);
     59                }, 0);
     60            }, 0);
    5461        }
    5562    </script>
  • trunk/LayoutTests/accessibility/aria-hidden-updates-alldescendants.html

    r155274 r241289  
    2727
    2828    if (window.accessibilityController) {
     29          jsTestIsAsync = true;
    2930          document.getElementById("main").focus();
    3031         
     
    3839          var items = group.getElementsByTagName('div');         
    3940          items[0].removeAttribute('aria-hidden');
    40 
    41           // After removing aria-hidden, the new count should be 2.
    42           shouldBe("main.childrenCount", "2");         
     41          setTimeout(function() {
     42              // After removing aria-hidden, the new count should be 2.
     43              shouldBe("main.childrenCount", "2");         
    4344         
    44           // And most importantly, the DIV that was made non-hidden should have one child now.
    45           shouldBe("main.childAtIndex(1).childrenCount", "1");
     45              // And most importantly, the DIV that was made non-hidden should have one child now.
     46              shouldBe("main.childAtIndex(1).childrenCount", "1");
     47              finishJSTest();
     48          }, 0);
    4649    }
    4750
  • trunk/LayoutTests/accessibility/image-load-on-delay.html

    r222546 r241289  
    2626          document.getElementById("image").onload = function() {
    2727              document.body.offsetHeight;
    28               debug("AFTER: Group count: " + content.childrenCount);
    29               finishJSTest();
     28              setTimeout(function() {
     29                  debug("AFTER: Group count: " + content.childrenCount);
     30                  finishJSTest();
     31              }, 0);
    3032          };
    3133
  • trunk/LayoutTests/accessibility/mac/aria-hidden-changes-for-non-ignored-elements.html

    r187799 r241289  
    2626
    2727    if (window.accessibilityController) {
    28 
     28        jsTestIsAsync = true;
    2929        document.getElementById("body").focus();
    3030        var body = accessibilityController.focusedElement;
     
    3535        // Toggle aria-hidden, and we should not be able to access the same elements anymore.
    3636        document.getElementById("main").setAttribute("aria-hidden", "true");
    37         shouldBe("body.childAtIndex(0).role", "'AXRole: AXCheckBox'");
     37        setTimeout(function() {
     38            shouldBe("body.childAtIndex(0).role", "'AXRole: AXCheckBox'");
    3839
    39         // Toggle aria-hidden off again and we should again be able to access elements inside the tab panel.
    40         document.getElementById("main").setAttribute("aria-hidden", "false");
    41         shouldBe("body.childAtIndex(3).childAtIndex(0).role", "'AXRole: AXButton'");
     40            // Toggle aria-hidden off again and we should again be able to access elements inside the tab panel.
     41            document.getElementById("main").setAttribute("aria-hidden", "false");
     42            setTimeout(function() {
     43                shouldBe("body.childAtIndex(3).childAtIndex(0).role", "'AXRole: AXButton'");
     44                finishJSTest();
     45            }, 0);
     46        }, 0);
    4247
    4348    }
  • trunk/LayoutTests/accessibility/removed-anonymous-block-child-causes-crash.html

    r155274 r241289  
    99
    1010    function queryIsEnabledOnDecendants(accessibilityObject) {
     11        if (!accessibilityObject)
     12            return;
    1113        accessibilityObject.isEnabled
    1214
  • trunk/Source/WebCore/ChangeLog

    r241288 r241289  
     12019-02-08  Chris Fleizach  <cfleizach@apple.com>
     2
     3        AXObjectCache::childrenChanged shouldn't update layout or style during another style recalc
     4        https://bugs.webkit.org/show_bug.cgi?id=182280
     5        <rdar://problem/37018386>
     6
     7        Reviewed by Alan Bujtas.
     8
     9        Remove the possibility that changing children calls back into updating layout by
     10        handling children changes in a deferred manner.
     11
     12        This follows the same architecture as many other deferred changes, but also requires us to check deferred changes
     13        in updateBackingStore, because things like aria-hidden changes won't trigger a layout, but will require us to update children.
     14
     15        A few tests had to be modified to no longer change the tree and then check the children immediately.
     16
     17        * accessibility/AXObjectCache.cpp:
     18        (WebCore::AXObjectCache::remove):
     19        (WebCore::AXObjectCache::childrenChanged):
     20        (WebCore::AXObjectCache::prepareForDocumentDestruction):
     21        (WebCore::AXObjectCache::performDeferredCacheUpdate):
     22        * accessibility/AXObjectCache.h:
     23        * accessibility/AccessibilityObject.cpp:
     24        (WebCore::AccessibilityObject::updateBackingStore):
     25        * accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
     26        (convertToNSArray):
     27        (-[WebAccessibilityObjectWrapperBase updateObjectBackingStore]):
     28
    1292019-02-11  Myles C. Maxfield  <mmaxfield@apple.com>
    230
  • trunk/Source/WebCore/accessibility/AXObjectCache.cpp

    r240552 r241289  
    748748        m_deferredRecomputeIsIgnoredList.remove(downcast<Element>(&node));
    749749        m_deferredSelectedChildredChangedList.remove(downcast<Element>(&node));
     750        m_deferredChildrenChangedNodeList.remove(&node);
    750751        m_deferredTextFormControlValue.remove(downcast<Element>(&node));
    751752        m_deferredAttributeChange.remove(downcast<Element>(&node));
     
    860861void AXObjectCache::childrenChanged(Node* node, Node* newChild)
    861862{
    862     if (newChild) {
    863         handleMenuOpened(newChild);
    864         handleLiveRegionCreated(newChild);
    865     }
     863    if (newChild)
     864        m_deferredChildrenChangedNodeList.add(newChild);
    866865
    867866    childrenChanged(get(node));
     
    873872        return;
    874873
    875     // FIXME: Refactor the code to avoid calling updateLayout in this call stack.
    876     ScriptDisallowedScope::LayoutAssertionDisableScope disableScope;
    877    
    878     if (newChild) {
    879         handleMenuOpened(newChild->node());
    880         handleLiveRegionCreated(newChild->node());
    881     }
    882    
     874    if (newChild && newChild->node())
     875        m_deferredChildrenChangedNodeList.add(newChild->node());
     876
    883877    childrenChanged(get(renderer));
    884878}
     
    889883        return;
    890884
    891     obj->childrenChanged();
     885    m_deferredChildredChangedList.add(obj);
    892886}
    893887   
     
    28552849    filterListForRemoval(m_deferredTextChangedList, document, nodesToRemove);
    28562850    filterListForRemoval(m_deferredSelectedChildredChangedList, document, nodesToRemove);
     2851    filterListForRemoval(m_deferredChildrenChangedNodeList, document, nodesToRemove);
    28572852    filterMapForRemoval(m_deferredTextFormControlValue, document, nodesToRemove);
    28582853    filterMapForRemoval(m_deferredAttributeChange, document, nodesToRemove);
     
    28872882
    28882883    SetForScope<bool> performingDeferredCacheUpdate(m_performingDeferredCacheUpdate, true);
     2884
     2885    for (auto* nodeChild : m_deferredChildrenChangedNodeList) {
     2886        handleMenuOpened(nodeChild);
     2887        handleLiveRegionCreated(nodeChild);
     2888    }
     2889    m_deferredChildrenChangedNodeList.clear();
     2890
     2891    for (auto& child : m_deferredChildredChangedList)
     2892        child->childrenChanged();
     2893    m_deferredChildredChangedList.clear();
     2894
    28892895    for (auto* node : m_deferredTextChangedList)
    28902896        textChanged(node);
  • trunk/Source/WebCore/accessibility/AXObjectCache.h

    r241190 r241289  
    470470    ListHashSet<Node*> m_deferredTextChangedList;
    471471    ListHashSet<Element*> m_deferredSelectedChildredChangedList;
     472    ListHashSet<RefPtr<AccessibilityObject>> m_deferredChildredChangedList;
     473    ListHashSet<Node*> m_deferredChildrenChangedNodeList;
    472474    HashMap<Element*, String> m_deferredTextFormControlValue;
    473475    HashMap<Element*, QualifiedName> m_deferredAttributeChange;
  • trunk/Source/WebCore/accessibility/AccessibilityObject.cpp

    r238454 r241289  
    17871787            document->updateLayoutIgnorePendingStylesheets();
    17881788    }
     1789
     1790    if (auto cache = axObjectCache())
     1791        cache->performDeferredCacheUpdate();
     1792   
    17891793    updateChildrenIfNecessary();
    17901794}
  • trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm

    r240552 r241289  
    259259    for (const auto& child : vector) {
    260260        auto wrapper = (WebAccessibilityObjectWrapperBase *)child->wrapper();
    261         ASSERT(wrapper);
    262         if (wrapper) {
    263             // We want to return the attachment view instead of the object representing the attachment,
    264             // otherwise, we get palindrome errors in the AX hierarchy.
    265             if (child->isAttachment() && [wrapper attachmentView])
    266                 [array addObject:[wrapper attachmentView]];
    267             else
    268                 [array addObject:wrapper];
    269         }
     261        if (!wrapper)
     262            continue;
     263
     264        // We want to return the attachment view instead of the object representing the attachment,
     265        // otherwise, we get palindrome errors in the AX hierarchy.
     266        if (child->isAttachment() && [wrapper attachmentView])
     267            [array addObject:[wrapper attachmentView]];
     268        else
     269            [array addObject:wrapper];
    270270    }
    271271    return [[array copy] autorelease];
Note: See TracChangeset for help on using the changeset viewer.