Changeset 228666 in webkit


Ignore:
Timestamp:
Feb 19, 2018 5:17:13 AM (6 years ago)
Author:
Carlos Garcia Campos
Message:

Merge r228279 - AX: Defer attribute computation until needed.
https://bugs.webkit.org/show_bug.cgi?id=182386
<rdar://problem/37115277>

Reviewed by Zalan Bujtas.

Source/WebCore:

Accessibility is doing too much work when handling attribute changes. Here's how we can improve this:

1) Defer attribute changes while the tree is dirty (and coalesce them).
2) Don't create AXObjects when an attribute changes unnecessarily. If no client has requested an ax object, it's likely no work needs to be done

(with the exception of a few attributes like aria-modal)

3) Stop calculating the entire accessible ARIA label when trying to decide if an element should be ignored. That's generally wasteful and the

consequence of including more AX elements in the tree is very minimal.

  • accessibility/AXObjectCache.cpp:

(WebCore::rendererNeedsDeferredUpdate):
(WebCore::nodeAndRendererAreValid):
(WebCore::AXObjectCache::remove):
(WebCore::AXObjectCache::handleAriaExpandedChange):
(WebCore::AXObjectCache::handleAriaRoleChanged):
(WebCore::AXObjectCache::deferAttributeChangeIfNeeded):
(WebCore::AXObjectCache::shouldProcessAttributeChange):
(WebCore::AXObjectCache::handleAttributeChange):
(WebCore::AXObjectCache::prepareForDocumentDestruction):
(WebCore::AXObjectCache::performDeferredCacheUpdate):
(WebCore::AXObjectCache::deferRecomputeIsIgnoredIfNeeded):
(WebCore::AXObjectCache::deferRecomputeIsIgnored):
(WebCore::AXObjectCache::deferTextChangedIfNeeded):
(WebCore::AXObjectCache::deferSelectedChildrenChangedIfNeeded):
(WebCore::AXObjectCache::handleAttributeChanged): Deleted.

  • accessibility/AXObjectCache.h:

(WebCore::AXObjectCache::deferAttributeChangeIfNeeded):
(WebCore::AXObjectCache::handleAttributeChanged): Deleted.

  • accessibility/AccessibilityNodeObject.cpp:

(WebCore::AccessibilityNodeObject::hasAttributesRequiredForInclusion const):

  • accessibility/AccessibleNode.cpp:

(WebCore::AccessibleNode::notifyAttributeChanged):

  • dom/Element.cpp:

(WebCore::Element::attributeChanged):

LayoutTests:

Update tests to reflect new world of delayed attribute handling for accessibility.

  • accessibility/canvas-fallback-content.html:

Make test async so attributes can be checked after deferred handling.

  • accessibility/mac/aria-expanded-notifications.html:

Access elements through AX tree so attribute changes generate notifications.

  • accessibility/mac/aria-listbox-selectedchildren-change.html:

Make test async so attributes can be checked after deferred handling.

  • accessibility/mac/aria-menu-item-selected-notification.html:

Access menu item through AX tree so attribute changes generate notifications.

  • accessibility/mac/aria-modal-auto-focus.html:

Access buttons after delay so attributes have time to be deferred.

  • accessibility/mac/element-busy-changed.html:

Process second attribute change after delay so we generate two notifications.

  • accessibility/mac/expanded-notification.html:

Set attributes after a delay so they generate individual notifications.

  • accessibility/notification-listeners.html:

Access elements through AX tree so attribute changes generate notifications.

Location:
releases/WebKitGTK/webkit-2.20
Files:
15 edited

Legend:

Unmodified
Added
Removed
  • releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog

    r228663 r228666  
     12018-02-08  Chris Fleizach  <cfleizach@apple.com>
     2
     3        AX: Defer attribute computation until needed.
     4        https://bugs.webkit.org/show_bug.cgi?id=182386
     5        <rdar://problem/37115277>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Update tests to reflect new world of delayed attribute handling for accessibility.
     10
     11        * accessibility/canvas-fallback-content.html:
     12             Make test async so attributes can be checked after deferred handling.
     13        * accessibility/mac/aria-expanded-notifications.html:
     14             Access elements through AX tree so attribute changes generate notifications.
     15        * accessibility/mac/aria-listbox-selectedchildren-change.html:
     16             Make test async so attributes can be checked after deferred handling.
     17        * accessibility/mac/aria-menu-item-selected-notification.html:
     18             Access menu item through AX tree so attribute changes generate notifications.
     19        * accessibility/mac/aria-modal-auto-focus.html:
     20             Access buttons after delay so attributes have time to be deferred.
     21        * accessibility/mac/element-busy-changed.html:
     22             Process second attribute change after delay so we generate two notifications.
     23        * accessibility/mac/expanded-notification.html:
     24             Set attributes after a delay so they generate individual notifications.
     25        * accessibility/notification-listeners.html:
     26              Access elements through AX tree so attribute changes generate notifications.
     27
    1282018-02-08  Philippe Normand  <pnormand@igalia.com>
    229
  • releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/canvas-fallback-content.html

    r201216 r228666  
    4141
    4242if (window.testRunner && window.accessibilityController) {
     43    window.jsTestIsAsync = true;
    4344    window.testRunner.dumpAsText();
    4445
     
    8182    // Check that the role is updated when the element changes.
    8283    document.getElementById('focusable1').setAttribute('role', 'button');
    83     check("focusable1", "AXRole: AXButton");
     84    setTimeout(function() {
     85        check("focusable1", "AXRole: AXButton");
     86    }, 1);
     87
    8488    document.getElementById('focusable2').setAttribute('role', 'button');
    85     check("focusable2", "AXRole: AXButton");
     89    setTimeout(function() {
     90        check("focusable2", "AXRole: AXButton");
     91        finishJSTest();
     92    }, 1);
    8693}
    8794
  • releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/aria-expanded-notifications.html

    r217171 r228666  
    5959        shouldBe("addedNotification", "true");
    6060
     61        accessibilityController.accessibleElementById("tree0");
     62        accessibilityController.accessibleElementById("tree0_item0");
     63
    6164        // the first aria-expanded should generate row count, row collapsed.
    6265        document.getElementById("tree0_item0").setAttribute("aria-expanded", "false");
  • releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/aria-listbox-selectedchildren-change.html

    r217171 r228666  
    66<body id="body">
    77
    8 <div role="group" tabindex=0 id="listbox" role="listbox">
     8<div tabindex=0 id="listbox" role="listbox">
    99<div id="option1" role="option" aria-selected="true">Option</div>
    1010<div id="option2" role="option">Option</div>
     
    3636        jsTestIsAsync = true;
    3737
    38         document.getElementById("listbox").focus();
    39         listbox = window.accessibilityController.focusedElement;
     38        listbox = accessibilityController.accessibleElementById("listbox");
    4039
    4140        var addedNotification = window.accessibilityController.addNotificationListener(ariaCallback);
     
    4443        // These should each trigger a notification that the selected children changed.
    4544        document.getElementById("option2").setAttribute("aria-selected", "true");
    46         document.getElementById("option2").setAttribute("aria-selected", "false");
     45        setTimeout(function() {
     46            document.getElementById("option2").setAttribute("aria-selected", "false");
     47        }, 1);
    4748    }
    4849
  • releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/aria-menu-item-selected-notification.html

    r187799 r228666  
    4040
    4141        var addedNotification = accessibilityController.addNotificationListener(ariaCallback);
    42         accessibilityController.rootElement;
     42        accessibilityController.accessibleElementById("menu");
    4343
    4444        shouldBe("addedNotification", "true");
  • releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/aria-modal-auto-focus.html

    r210265 r228666  
    5151            // 2. Click the new button, dialog2 shows and focus should move to the close button.
    5252            document.getElementById("new").click();
    53             closeBtn = accessibilityController.accessibleElementById("close");
    5453            setTimeout(function(){
     54                closeBtn = accessibilityController.accessibleElementById("close");
    5555                shouldBeTrue("closeBtn.isFocused");
    56                
     56                 
    5757                // 3. Click the close button, dialog2 closes and focus should go back to the
    5858                // first focusable child of dialog1.
    5959                document.getElementById("close").click();
    60                 okBtn = accessibilityController.accessibleElementById("ok");
    6160                setTimeout(function(){
     61                    okBtn = accessibilityController.accessibleElementById("ok");
    6262                    shouldBeTrue("okBtn.isFocused");
    6363                    finishJSTest();
    64                 }, 50);
    65             }, 50);
    66         }, 50);
     64                }, 100);
     65            }, 100);
     66        }, 100);
    6767    }
    6868   
  • releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/element-busy-changed.html

    r187799 r228666  
    3434        var busyElement = document.getElementById("body");
    3535        busyElement.setAttribute("aria-busy", "true");
    36         busyElement.setAttribute("aria-busy", "false");
     36
     37        setTimeout(function() {
     38            busyElement.setAttribute("aria-busy", "false");
     39        }, 1);
    3740    }
    3841</script>
  • releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/mac/expanded-notification.html

    r187799 r228666  
    3939
    4040        var addedNotification = accessibilityController.addNotificationListener(ariaCallback);
    41         debug("Initial expanded status: " + accessibilityController.accessibleElementById("button").isExpanded);
     41        var button = accessibilityController.accessibleElementById("button");
     42        debug("Initial expanded status: " + button.isExpanded);
     43
     44        document.getElementById("button").setAttribute("aria-expanded", "true");
    4245
    4346        setTimeout(function() {
    44             document.getElementById("button").setAttribute("aria-expanded", "true");
    45             setTimeout(function() {
    46                 document.getElementById("button").setAttribute("aria-expanded", "false");
    47             }, 10);
     47            document.getElementById("button").setAttribute("aria-expanded", "false");
    4848        }, 10);
    4949    }
  • releases/WebKitGTK/webkit-2.20/LayoutTests/accessibility/notification-listeners.html

    r211573 r228666  
    4646    }
    4747
     48    // Ensure these elements exist in the AX tree otherwise notifications won't be generated.
     49    accessibilityController.accessibleElementById("select");
     50    accessibilityController.accessibleElementById("slider");
     51
    4852    // This should trigger a "invalid status changed" notification on the select.
    4953    document.getElementById("select").setAttribute("aria-invalid", "true");
  • releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog

    r228665 r228666  
     12018-02-08  Chris Fleizach  <cfleizach@apple.com>
     2
     3        AX: Defer attribute computation until needed.
     4        https://bugs.webkit.org/show_bug.cgi?id=182386
     5        <rdar://problem/37115277>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Accessibility is doing too much work when handling attribute changes. Here's how we can improve this:
     10           1) Defer attribute changes while the tree is dirty (and coalesce them).
     11           2) Don't create AXObjects when an attribute changes unnecessarily. If no client has requested an ax object, it's likely no work needs to be done
     12                 (with the exception of a few attributes like aria-modal)
     13           3) Stop calculating the entire accessible ARIA label when trying to decide if an element should be ignored. That's generally wasteful and the
     14                 consequence of including more AX elements in the tree is very minimal.
     15
     16        * accessibility/AXObjectCache.cpp:
     17        (WebCore::rendererNeedsDeferredUpdate):
     18        (WebCore::nodeAndRendererAreValid):
     19        (WebCore::AXObjectCache::remove):
     20        (WebCore::AXObjectCache::handleAriaExpandedChange):
     21        (WebCore::AXObjectCache::handleAriaRoleChanged):
     22        (WebCore::AXObjectCache::deferAttributeChangeIfNeeded):
     23        (WebCore::AXObjectCache::shouldProcessAttributeChange):
     24        (WebCore::AXObjectCache::handleAttributeChange):
     25        (WebCore::AXObjectCache::prepareForDocumentDestruction):
     26        (WebCore::AXObjectCache::performDeferredCacheUpdate):
     27        (WebCore::AXObjectCache::deferRecomputeIsIgnoredIfNeeded):
     28        (WebCore::AXObjectCache::deferRecomputeIsIgnored):
     29        (WebCore::AXObjectCache::deferTextChangedIfNeeded):
     30        (WebCore::AXObjectCache::deferSelectedChildrenChangedIfNeeded):
     31        (WebCore::AXObjectCache::handleAttributeChanged): Deleted.
     32        * accessibility/AXObjectCache.h:
     33        (WebCore::AXObjectCache::deferAttributeChangeIfNeeded):
     34        (WebCore::AXObjectCache::handleAttributeChanged): Deleted.
     35        * accessibility/AccessibilityNodeObject.cpp:
     36        (WebCore::AccessibilityNodeObject::hasAttributesRequiredForInclusion const):
     37        * accessibility/AccessibleNode.cpp:
     38        (WebCore::AccessibleNode::notifyAttributeChanged):
     39        * dom/Element.cpp:
     40        (WebCore::Element::attributeChanged):
     41
    1422018-02-08  Zalan Bujtas  <zalan@apple.com>
    243
  • releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AXObjectCache.cpp

    r227858 r228666  
    121121static const Seconds accessibilityLiveRegionChangedNotificationInterval { 20_ms };
    122122static const Seconds accessibilityFocusModalNodeNotificationInterval { 50_ms };
    123 
     123   
     124static bool rendererNeedsDeferredUpdate(const RenderObject& renderer)
     125{
     126    ASSERT(!renderer.beingDestroyed());
     127    auto& document = renderer.document();
     128    return renderer.needsLayout() || document.needsStyleRecalc() || document.inRenderTreeUpdate() || (document.view() && document.view()->layoutContext().isInRenderTreeLayout());
     129}
     130
     131static bool nodeAndRendererAreValid(Node* node)
     132{
     133    if (!node)
     134        return false;
     135   
     136    auto* renderer = node->renderer();
     137    return renderer && !renderer->beingDestroyed();
     138}
     139   
    124140AccessibilityObjectInclusion AXComputedObjectAttributeCache::getIgnored(AXID id) const
    125141{
     
    724740        m_deferredSelectedChildredChangedList.remove(downcast<Element>(&node));
    725741        m_deferredTextFormControlValue.remove(downcast<Element>(&node));
     742        m_deferredAttributeChange.remove(downcast<Element>(&node));
    726743    }
    727744    m_deferredTextChangedList.remove(&node);
     
    14031420void AXObjectCache::handleAriaExpandedChange(Node* node)
    14041421{
    1405     if (AccessibilityObject* obj = getOrCreate(node))
     1422    if (AccessibilityObject* obj = get(node))
    14061423        obj->handleAriaExpandedChanged();
    14071424}
     
    14171434    stopCachingComputedObjectAttributes();
    14181435
    1419     if (AccessibilityObject* obj = getOrCreate(node)) {
     1436    // Don't make an AX object unless it's needed
     1437    if (AccessibilityObject* obj = get(node)) {
    14201438        obj->updateAccessibilityRole();
    14211439        obj->notifyIfIgnoredValueChanged();
     
    14231441}
    14241442
    1425 void AXObjectCache::handleAttributeChanged(const QualifiedName& attrName, Element* element)
    1426 {
     1443void AXObjectCache::deferAttributeChangeIfNeeded(const QualifiedName& attrName, Element* element)
     1444{
     1445    if (nodeAndRendererAreValid(element) && rendererNeedsDeferredUpdate(*element->renderer()))
     1446        m_deferredAttributeChange.add(element, attrName);
     1447    else
     1448        handleAttributeChange(attrName, element);
     1449}
     1450   
     1451bool AXObjectCache::shouldProcessAttributeChange(const QualifiedName& attrName, Element* element)
     1452{
     1453    if (!element)
     1454        return false;
     1455   
     1456    // aria-modal ends up affecting sub-trees that are being shown/hidden so it's likely that
     1457    // an AT would not have accessed this node yet.
     1458    if (attrName == aria_modalAttr)
     1459        return true;
     1460   
     1461    // If an AXObject has yet to be created, then there's no need to process attribute changes.
     1462    // Some of these notifications are processed on the parent, so allow that to proceed as well
     1463    if (get(element) || get(element->parentNode()))
     1464        return true;
     1465
     1466    return false;
     1467}
     1468   
     1469void AXObjectCache::handleAttributeChange(const QualifiedName& attrName, Element* element)
     1470{
     1471    if (!shouldProcessAttributeChange(attrName, element))
     1472        return;
     1473   
    14271474    if (attrName == roleAttr)
    14281475        handleAriaRoleChanged(element);
    14291476    else if (attrName == altAttr || attrName == titleAttr)
    1430         deferTextChangedIfNeeded(element);
     1477        textChanged(element);
    14311478    else if (attrName == forAttr && is<HTMLLabelElement>(*element))
    14321479        labelChanged(element);
     
    14421489        postNotification(element, AXObjectCache::AXValueChanged);
    14431490    else if (attrName == aria_labelAttr || attrName == aria_labeledbyAttr || attrName == aria_labelledbyAttr)
    1444         deferTextChangedIfNeeded(element);
     1491        textChanged(element);
    14451492    else if (attrName == aria_checkedAttr)
    14461493        checkedStateChanged(element);
     
    27612808    filterListForRemoval(m_deferredSelectedChildredChangedList, document, nodesToRemove);
    27622809    filterMapForRemoval(m_deferredTextFormControlValue, document, nodesToRemove);
     2810    filterMapForRemoval(m_deferredAttributeChange, document, nodesToRemove);
    27632811
    27642812    for (auto* node : nodesToRemove)
     
    28002848    }
    28012849    m_deferredTextFormControlValue.clear();
    2802 }
    2803 
    2804 static bool rendererNeedsDeferredUpdate(RenderObject& renderer)
    2805 {
    2806     ASSERT(!renderer.beingDestroyed());
    2807     auto& document = renderer.document();
    2808     return renderer.needsLayout() || document.needsStyleRecalc() || document.inRenderTreeUpdate() || (document.view() && document.view()->layoutContext().isInRenderTreeLayout());
    2809 }
    2810 
     2850
     2851    for (auto& deferredAttributeChangeContext : m_deferredAttributeChange)
     2852        handleAttributeChange(deferredAttributeChangeContext.value, deferredAttributeChangeContext.key);
     2853    m_deferredAttributeChange.clear();
     2854}
     2855   
    28112856void AXObjectCache::deferRecomputeIsIgnoredIfNeeded(Element* element)
    28122857{
    2813     if (!element)
    2814         return;
    2815 
    2816     auto* renderer = element->renderer();
    2817     if (!renderer || renderer->beingDestroyed())
    2818         return;
    2819 
    2820     if (rendererNeedsDeferredUpdate(*renderer)) {
     2858    if (!nodeAndRendererAreValid(element))
     2859        return;
     2860   
     2861    if (rendererNeedsDeferredUpdate(*element->renderer())) {
    28212862        m_deferredRecomputeIsIgnoredList.add(element);
    28222863        return;
    28232864    }
    2824     recomputeIsIgnored(renderer);
     2865    recomputeIsIgnored(element->renderer());
    28252866}
    28262867
    28272868void AXObjectCache::deferRecomputeIsIgnored(Element* element)
    28282869{
    2829     if (!element)
    2830         return;
    2831 
    2832     if (element->renderer() && element->renderer()->beingDestroyed())
     2870    if (!nodeAndRendererAreValid(element))
    28332871        return;
    28342872
     
    28382876void AXObjectCache::deferTextChangedIfNeeded(Node* node)
    28392877{
    2840     if (!node)
    2841         return;
    2842 
    2843     auto* renderer = node->renderer();
    2844     if (renderer && renderer->beingDestroyed())
    2845         return;
    2846 
    2847     if (renderer && rendererNeedsDeferredUpdate(*renderer)) {
     2878    if (!nodeAndRendererAreValid(node))
     2879        return;
     2880
     2881    if (rendererNeedsDeferredUpdate(*node->renderer())) {
    28482882        m_deferredTextChangedList.add(node);
    28492883        return;
     
    28542888void AXObjectCache::deferSelectedChildrenChangedIfNeeded(Element& selectElement)
    28552889{
    2856     auto* renderer = selectElement.renderer();
    2857     if (renderer && renderer->beingDestroyed())
    2858         return;
    2859    
    2860     if (renderer && rendererNeedsDeferredUpdate(*renderer)) {
     2890    if (!nodeAndRendererAreValid(&selectElement))
     2891        return;
     2892
     2893    if (rendererNeedsDeferredUpdate(*selectElement.renderer())) {
    28612894        m_deferredSelectedChildredChangedList.add(&selectElement);
    28622895        return;
  • releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AXObjectCache.h

    r225855 r228666  
    186186    Node* modalNode();
    187187
    188     void handleAttributeChanged(const QualifiedName& attrName, Element*);
     188    void deferAttributeChangeIfNeeded(const QualifiedName&, Element*);
    189189    void recomputeIsIgnored(RenderObject* renderer);
    190190
     
    410410    void handleLiveRegionCreated(Node*);
    411411    void handleMenuItemSelected(Node*);
     412    void handleAttributeChange(const QualifiedName&, Element*);
     413    bool shouldProcessAttributeChange(const QualifiedName&, Element*);
    412414   
    413415    // aria-modal related
     
    447449    ListHashSet<Element*> m_deferredSelectedChildredChangedList;
    448450    HashMap<Element*, String> m_deferredTextFormControlValue;
     451    HashMap<Element*, QualifiedName> m_deferredAttributeChange;
    449452    bool m_isSynchronizingSelection { false };
    450453    bool m_performingDeferredCacheUpdate { false };
     
    508511inline void AXObjectCache::handleModalChange(Node*) { }
    509512inline void AXObjectCache::handleAriaRoleChanged(Node*) { }
    510 inline void AXObjectCache::handleAttributeChanged(const QualifiedName&, Element*) { }
     513inline void AXObjectCache::deferAttributeChangeIfNeeded(const QualifiedName&, Element*) { }
     514inline void AXObjectCache::handleAttributeChange(const QualifiedName&, Element*) { }
     515inline bool AXObjectCache::shouldProcessAttributeChange(const QualifiedName&, Element*) { return false; }
    511516inline void AXObjectCache::handleFocusedUIElementChanged(Node*, Node*) { }
    512517inline void AXObjectCache::handleScrollbarUpdate(ScrollView*) { }
  • releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AccessibilityNodeObject.cpp

    r225680 r228666  
    20182018        return true;
    20192019
    2020     if (!ariaAccessibilityDescription().isEmpty())
     2020    // Avoid calculating the actual description here, which is expensive.
     2021    // This means there might be more accessible elements in the tree if the labelledBy points to invalid elements, but that shouldn't cause any real problems.
     2022    if (getAttribute(aria_labelledbyAttr).length() || getAttribute(aria_labeledbyAttr).length() || getAttribute(aria_labelAttr).length())
    20212023        return true;
    20222024
  • releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AccessibleNode.cpp

    r225511 r228666  
    380380{
    381381    if (AXObjectCache* cache = m_ownerElement.document().axObjectCache())
    382         cache->handleAttributeChanged(name, &m_ownerElement);
     382        cache->deferAttributeChangeIfNeeded(name, &m_ownerElement);
    383383}
    384384
  • releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Element.cpp

    r227983 r228666  
    13901390
    13911391    if (AXObjectCache* cache = document().existingAXObjectCache())
    1392         cache->handleAttributeChanged(name, this);
     1392        cache->deferAttributeChangeIfNeeded(name, this);
    13931393}
    13941394
Note: See TracChangeset for help on using the changeset viewer.