[webkit-reviews] review granted: [Bug 62289] ARIA live regions don't trigger notifications for elements that aren't in the AX tree : [Attachment 98019] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 62289: ARIA live regions don't trigger notifications for elements that
aren't in the AX tree
https://bugs.webkit.org/show_bug.cgi?id=62289

Attachment 98019: patch
https://bugs.webkit.org/attachment.cgi?id=98019&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=98019&action=review

> Source/WebCore/accessibility/AXObjectCache.cpp:444
> +    // Make a local copy in case childrenChanged() alters m_childrenToUpdate
(it shouldn't).

Just wondering about the "it shouldn't" comment. Doesn’t childrenChanged end up
calling out to the client? Couldn’t the client do any arbitrary work including
calls that would change the AX tree?

> Source/WebCore/accessibility/AXObjectCache.cpp:446
> +    HashSet<RenderObject*> updateChildren = m_childrenToUpdate;
> +    m_childrenToUpdate.clear();

A much more efficient, although less clear, way to do this is:

    HashSet<RenderObject*> updateChildren;
    m_childrenToUpdate.swap(updateChildren);

The code in the patch has to build a whole new hash table.

Another way to do it is to just copy the values of the set into a vector.
That's more efficient than making a new set, but not as efficient as swapping
to grab the existing set.