Changeset 215975 in webkit


Ignore:
Timestamp:
Apr 29, 2017 11:55:18 AM (7 years ago)
Author:
n_wang@apple.com
Message:

AX: Improve performance of addChildren()/childrenChanged()
​https://bugs.webkit.org/show_bug.cgi?id=171443

Reviewed by Chris Fleizach.

There's a lot of unnecessary work happening when childrenChanged() is being called.
Some of the improvements here:

  1. When childrenChanged() is being called on some element, we are marking its parent chain dirty. However, in the addChild() method we are then clearing each child of that 'dirty' parent, eventually making the entire tree dirty. Added a m_subTreeDirty flag and using the needsToUpdateChildren() check to make sure we are only clearing the right chain without updating the unchanged siblings.
  2. In the addChild() method we are calling accessibilityIsIgnored() on each child and that might lead to going up the parent chain again to get necessary information. Since we are already traversing the tree from top to bottom to add children, added a struct to store the information and pass it to the child so to avoid unnecessary traversal.
  3. Reduced the amount work of ARIA text controls perform when updating its parents in childrenChanged() so that we don't update a big portion of the tree while typing.

No new tests since this didn't change any functionality.

  • accessibility/AccessibilityNodeObject.cpp:

(WebCore::AccessibilityNodeObject::AccessibilityNodeObject):
(WebCore::AccessibilityNodeObject::childrenChanged):
(WebCore::AccessibilityNodeObject::insertChild):
(WebCore::AccessibilityNodeObject::addChildren):
(WebCore::AccessibilityNodeObject::isDescendantOfBarrenParent):

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

(WebCore::AccessibilityObject::AccessibilityObject):
(WebCore::AccessibilityObject::defaultObjectInclusion):
(WebCore::AccessibilityObject::setIsIgnoredFromParentDataForChild):

  • accessibility/AccessibilityObject.h:

(WebCore::AccessibilityIsIgnoredFromParentData::AccessibilityIsIgnoredFromParentData):
(WebCore::AccessibilityIsIgnoredFromParentData::isNull):
(WebCore::AccessibilityObject::setNeedsToUpdateSubTree):
(WebCore::AccessibilityObject::needsToUpdateChildren):
(WebCore::AccessibilityObject::setIsIgnoredFromParentData):

  • accessibility/AccessibilityRenderObject.cpp:

(WebCore::AccessibilityRenderObject::updateChildrenIfNecessary):
(WebCore::AccessibilityRenderObject::addChildren):

  • accessibility/AccessibilityRenderObject.h:

(WebCore::AccessibilityRenderObject::setRenderObject):
(WebCore::AccessibilityRenderObject::needsToUpdateChildren): Deleted.

Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r215973 r215975  
     12017-04-29  Nan Wang  <n_wang@apple.com>
     2
     3        AX: Improve performance of addChildren()/childrenChanged()
     4        https://bugs.webkit.org/show_bug.cgi?id=171443
     5
     6        Reviewed by Chris Fleizach.
     7
     8        There's a lot of unnecessary work happening when childrenChanged() is being called.
     9        Some of the improvements here:
     10        1. When childrenChanged() is being called on some element, we are marking its parent
     11           chain dirty. However, in the addChild() method we are then clearing each child of
     12           that 'dirty' parent, eventually making the entire tree dirty.
     13           Added a m_subTreeDirty flag and using the needsToUpdateChildren() check to make sure
     14           we are only clearing the right chain without updating the unchanged siblings.
     15        2. In the addChild() method we are calling accessibilityIsIgnored() on each child and that
     16           might lead to going up the parent chain again to get necessary information.
     17           Since we are already traversing the tree from top to bottom to add children, added a
     18           struct to store the information and pass it to the child so to avoid unnecessary traversal.
     19        3. Reduced the amount work of ARIA text controls perform when updating its parents in childrenChanged()
     20           so that we don't update a big portion of the tree while typing.
     21
     22        No new tests since this didn't change any functionality.
     23
     24        * accessibility/AccessibilityNodeObject.cpp:
     25        (WebCore::AccessibilityNodeObject::AccessibilityNodeObject):
     26        (WebCore::AccessibilityNodeObject::childrenChanged):
     27        (WebCore::AccessibilityNodeObject::insertChild):
     28        (WebCore::AccessibilityNodeObject::addChildren):
     29        (WebCore::AccessibilityNodeObject::isDescendantOfBarrenParent):
     30        * accessibility/AccessibilityNodeObject.h:
     31        * accessibility/AccessibilityObject.cpp:
     32        (WebCore::AccessibilityObject::AccessibilityObject):
     33        (WebCore::AccessibilityObject::defaultObjectInclusion):
     34        (WebCore::AccessibilityObject::setIsIgnoredFromParentDataForChild):
     35        * accessibility/AccessibilityObject.h:
     36        (WebCore::AccessibilityIsIgnoredFromParentData::AccessibilityIsIgnoredFromParentData):
     37        (WebCore::AccessibilityIsIgnoredFromParentData::isNull):
     38        (WebCore::AccessibilityObject::setNeedsToUpdateSubTree):
     39        (WebCore::AccessibilityObject::needsToUpdateChildren):
     40        (WebCore::AccessibilityObject::setIsIgnoredFromParentData):
     41        * accessibility/AccessibilityRenderObject.cpp:
     42        (WebCore::AccessibilityRenderObject::updateChildrenIfNecessary):
     43        (WebCore::AccessibilityRenderObject::addChildren):
     44        * accessibility/AccessibilityRenderObject.h:
     45        (WebCore::AccessibilityRenderObject::setRenderObject):
     46        (WebCore::AccessibilityRenderObject::needsToUpdateChildren): Deleted.
     47
    1482017-04-29  Yusuke Suzuki  <utatane.tea@gmail.com>
    249

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

    r215968 r215975  
    8686    , m_ariaRole(UnknownRole)
    8787    , m_childrenDirty(false)
     88    , m_subtreeDirty(false)
    8889    , m_roleForMSAA(UnknownRole)
    8990#ifndef NDEBUG
    … …  
    130131        return;
    131132    cache->postNotification(this, document(), AXObjectCache::AXChildrenChanged);
     133   
     134    // Should make the sub tree dirty so that everything below will be updated correctly.
     135    this->setNeedsToUpdateSubtree();
     136    bool shouldStopUpdatingParent = false;
    132137
    133138    // Go up the accessibility parent chain, but only if the element already exists. This method is
    … …  
    136141    // At the same time, process ARIA live region changes.
    137142    for (AccessibilityObject* parent = this; parent; parent = parent->parentObjectIfExists()) {
    138         parent->setNeedsToUpdateChildren();
     143        if (!shouldStopUpdatingParent)
     144            parent->setNeedsToUpdateChildren();
     145       
    139146
    140147        // These notifications always need to be sent because screenreaders are reliant on them to perform.
    … …  
    149156       
    150157        // If this element is an ARIA text control, notify the AT of changes.
    151         if (parent->isNonNativeTextControl())
     158        if (parent->isNonNativeTextControl()) {
    152159            cache->postNotification(parent, parent->document(), AXObjectCache::AXValueChanged);
     160           
     161            // Do not let the parent that's above the editable ancestor update its children
     162            // since we already notify the AT of changes.
     163            shouldStopUpdatingParent = true;
     164        }
    153165    }
    154166}
    … …  
    334346    // or its visibility has changed. In the latter case, this child may have a stale child cached.
    335347    // This can prevent aria-hidden changes from working correctly. Hence, whenever a parent is getting children, ensure data is not stale.
    336     child->clearChildren();
    337    
     348    // Only clear the child's children when we know it's in the updating chain in order to avoid unnecessary work.
     349    if (child->needsToUpdateChildren() || m_subtreeDirty) {
     350        child->clearChildren();
     351        // Pass m_subtreeDirty flag down to the child so that children cache gets reset properly.
     352        if (m_subtreeDirty)
     353            child->setNeedsToUpdateSubtree();
     354    } else {
     355        // For some reason the grand children might be detached so that we need to regenerate the
     356        // children list of this child.
     357        for (const auto& grandChild : child->children(false)) {
     358            if (grandChild->isDetachedFromParent()) {
     359                child->clearChildren();
     360                break;
     361            }
     362        }
     363    }
     364   
     365    setIsIgnoredFromParentDataForChild(child);
    338366    if (child->accessibilityIsIgnored()) {
    339367        const auto& children = child->children();
    … …  
    345373        m_children.insert(index, child);
    346374    }
     375   
     376    // Reset the child's m_isIgnoredFromParentData since we are done adding that child and its children.
     377    child->clearIsIgnoredFromParentData();
    347378}
    348379
    … …  
    369400    for (Node* child = m_node->firstChild(); child; child = child->nextSibling())
    370401        addChild(axObjectCache()->getOrCreate(child));
     402   
     403    m_subtreeDirty = false;
    371404}
    372405
    … …  
    10601093bool AccessibilityNodeObject::isDescendantOfBarrenParent() const
    10611094{
     1095    if (!m_isIgnoredFromParentData.isNull())
     1096        return m_isIgnoredFromParentData.isDescendantOfBarrenParent;
     1097   
    10621098    for (AccessibilityObject* object = parentObject(); object; object = object->parentObject()) {
    10631099        if (!object->canHaveChildren())

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

    r208179 r215975  
    147147    AccessibilityRole m_ariaRole;
    148148    bool m_childrenDirty;
     149    bool m_subtreeDirty;
    149150    mutable AccessibilityRole m_roleForMSAA;
    150151#ifndef NDEBUG

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

    r215968 r215975  
    8787    , m_role(UnknownRole)
    8888    , m_lastKnownIsIgnoredValue(DefaultBehavior)
     89    , m_isIgnoredFromParentData(AccessibilityIsIgnoredFromParentData())
    8990#if PLATFORM(GTK)
    9091    , m_wrapper(nullptr)
    … …  
    30983099AccessibilityObjectInclusion AccessibilityObject::defaultObjectInclusion() const
    30993100{
    3100     if (isARIAHidden())
     3101    bool useParentData = !m_isIgnoredFromParentData.isNull();
     3102   
     3103    if (useParentData ? m_isIgnoredFromParentData.isARIAHidden : isARIAHidden())
    31013104        return IgnoreObject;
    31023105   
    … …  
    31043107        return IgnoreObject;
    31053108   
    3106     if (isPresentationalChildOfAriaRole())
     3109    if (useParentData ? m_isIgnoredFromParentData.isPresentationalChildOfAriaRole : isPresentationalChildOfAriaRole())
    31073110        return IgnoreObject;
    31083111   
    … …  
    32963299}
    32973300
     3301void AccessibilityObject::setIsIgnoredFromParentDataForChild(AccessibilityObject* child)
     3302{
     3303    if (!child || child->parentObject() != this)
     3304        return;
     3305   
     3306    AccessibilityIsIgnoredFromParentData result = AccessibilityIsIgnoredFromParentData(this);
     3307    if (!m_isIgnoredFromParentData.isNull()) {
     3308        result.isARIAHidden = m_isIgnoredFromParentData.isARIAHidden || equalLettersIgnoringASCIICase(child->getAttribute(aria_hiddenAttr), "true");
     3309        result.isPresentationalChildOfAriaRole = m_isIgnoredFromParentData.isPresentationalChildOfAriaRole || ariaRoleHasPresentationalChildren();
     3310        result.isDescendantOfBarrenParent = m_isIgnoredFromParentData.isDescendantOfBarrenParent || !canHaveChildren();
     3311    } else {
     3312        result.isARIAHidden = child->isARIAHidden();
     3313        result.isPresentationalChildOfAriaRole = child->isPresentationalChildOfAriaRole();
     3314        result.isDescendantOfBarrenParent = child->isDescendantOfBarrenParent();
     3315    }
     3316   
     3317    child->setIsIgnoredFromParentData(result);
     3318}
     3319
    32983320} // namespace WebCore

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

    r215968 r215975  
    286286        { }
    287287};
     288
     289// Use this struct to store the isIgnored data that depends on the parents, so that in addChildren()
     290// we avoid going up the parent chain for each element while traversing the tree with useful information already.
     291struct AccessibilityIsIgnoredFromParentData {
     292    AccessibilityObject* parent;
     293    bool isARIAHidden;
     294    bool isPresentationalChildOfAriaRole;
     295    bool isDescendantOfBarrenParent;
     296   
     297    AccessibilityIsIgnoredFromParentData(AccessibilityObject* parent = nullptr)
     298        : parent(parent)
     299        , isARIAHidden(false)
     300        , isPresentationalChildOfAriaRole(false)
     301        , isDescendantOfBarrenParent(false)
     302        { }
     303   
     304    bool isNull() const { return !parent; }
     305};
    288306   
    289307enum AccessibilityOrientation {
    … …  
    823841    virtual void updateChildrenIfNecessary();
    824842    virtual void setNeedsToUpdateChildren() { }
     843    virtual void setNeedsToUpdateSubtree() { }
    825844    virtual void clearChildren();
     845    virtual bool needsToUpdateChildren() const { return false; }
    826846#if PLATFORM(COCOA)
    827847    virtual void detachFromParent();
    … …  
    10791099    static const AccessibilityObject* matchedParent(const AccessibilityObject&, bool includeSelf, const std::function<bool(const AccessibilityObject&)>&);
    10801100   
     1101    void clearIsIgnoredFromParentData() { m_isIgnoredFromParentData = AccessibilityIsIgnoredFromParentData(); }
     1102    void setIsIgnoredFromParentDataForChild(AccessibilityObject*);
     1103   
    10811104protected:
    10821105    AXID m_id;
    … …  
    10851108    AccessibilityRole m_role;
    10861109    AccessibilityObjectInclusion m_lastKnownIsIgnoredValue;
     1110    AccessibilityIsIgnoredFromParentData m_isIgnoredFromParentData;
     1111   
     1112    void setIsIgnoredFromParentData(AccessibilityIsIgnoredFromParentData& data) { m_isIgnoredFromParentData = data; }
    10871113
    10881114    virtual bool computeAccessibilityIsIgnored() const { return true; }

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

    r215971 r215975  
    29832983{
    29842984    if (needsToUpdateChildren())
    2985         clearChildren();       
     2985        clearChildren();
    29862986   
    29872987    AccessibilityObject::updateChildrenIfNecessary();
    … …  
    32073207    for (RefPtr<AccessibilityObject> obj = firstChild(); obj; obj = obj->nextSibling())
    32083208        addChild(obj.get());
     3209   
     3210    m_subtreeDirty = false;
    32093211   
    32103212    addHiddenChildren();

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

    r208929 r215975  
    203203    explicit AccessibilityRenderObject(RenderObject*);
    204204    void setRenderObject(RenderObject* renderer) { m_renderer = renderer; }
    205     bool needsToUpdateChildren() const { return m_childrenDirty; }
    206205    ScrollableArea* getScrollableAreaIfScrollable() const override;
    207206    void scrollTo(const IntPoint&) const override;
    … …  
    229228    bool nodeIsTextControl(const Node*) const;
    230229    void setNeedsToUpdateChildren() override { m_childrenDirty = true; }
     230    bool needsToUpdateChildren() const override { return m_childrenDirty; }
     231    void setNeedsToUpdateSubtree() override { m_subtreeDirty = true; }
    231232    Path elementPath() const override;
    232233   
Note: See TracChangeset for help on using the changeset viewer.