Update the accessibility tree when a modal dialog is opened/closed.

Update the accessibility tree when a modal dialog is opened/closed.

When a node's inertness changes, we must notify the AX tree since
the node may need to be added or removed from the tree.

This patch does the following.
1) When an modal dialog opens/closes, essentially we traverse the AX
tree and call notifyIfIgnoredValueChanged on each object. However, once
an object is found whose ignored value changed, it's not necessary to
descend into its subtree by virtue of the childrenCleared/addChild
mechanism.

An earlier attempt called notifyIfIgnoredValueChanged only on immediate
children of the HTMLBodyElement. This is insufficient because siblings
of <dialog> are roots of newly (or formerly) inert subtrees, and some
objects are not descendants of body due to renderer reparenting (e.g.
modal dialogs).

2) Modifies AXObject::isInertOrAriaHidden to climb the AX tree until an
object with non-null node() is found, when evaluating inertness. This is
necessary because some objects such as one created with
MenuListPopupRole don't have a node, but should be considered inert.

The test for this patch is Chrome-side:
https://codereview.chromium.org/64273003

BUG=304779
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161595

[falken, 2013-11-07 13:29:45]

Dominic,

Could you please review? This is spun off of
https://codereview.chromium.org/53423003/

[dmazzoni, 2013-11-07 16:35:58]

lgtm

I think this will cover all of the cases. Thanks!

https://codereview.chromium.org/59663005/diff/1/Source/core/accessibility/AXO...
File Source/core/accessibility/AXObject.cpp (right):

https://codereview.chromium.org/59663005/diff/1/Source/core/accessibility/AXO...
Source/core/accessibility/AXObject.cpp:292: bool canBeInInertSubtree = true;
Do you mean "is allowed to be in inert subtreee" or "might possibly be in inert
subtree"?

If the latter, how about mightBeInInsertSubtree?

https://codereview.chromium.org/59663005/diff/1/Source/core/accessibility/AXO...
File Source/core/accessibility/AXObjectCache.h (right):

https://codereview.chromium.org/59663005/diff/1/Source/core/accessibility/AXO...
Source/core/accessibility/AXObjectCache.h:125: void
updateCacheAfterInertSubtreesChanged();
Just call this handleInertSubtreesChanged(), and add a comment indicating that
this happens when a dialog opens or closes, which may cause nodes all over the
tree to suddenly become inert or not inert.

https://codereview.chromium.org/59663005/diff/1/Source/core/html/HTMLDialogEl...
File Source/core/html/HTMLDialogElement.cpp (right):

https://codereview.chromium.org/59663005/diff/1/Source/core/html/HTMLDialogEl...
Source/core/html/HTMLDialogElement.cpp:131: inertSubtreesChanged(document());
Might be nice to do this at the end - after setting "open" to true and after
layout, since those may cause AX events too. In particular, layout will clear
the cache and it'd be a waste to clear the cache immediately after touching the
whole tree.

[falken, 2013-11-07 23:54:57]

Thanks! Will make the suggested changes, just one a comment for now.

https://codereview.chromium.org/59663005/diff/1/Source/core/html/HTMLDialogEl...
File Source/core/html/HTMLDialogElement.cpp (right):

https://codereview.chromium.org/59663005/diff/1/Source/core/html/HTMLDialogEl...
Source/core/html/HTMLDialogElement.cpp:131: inertSubtreesChanged(document());
On 2013/11/07 16:35:59, Dominic Mazzoni wrote:
> Might be nice to do this at the end - after setting "open" to true and after
> layout, since those may cause AX events too. In particular, layout will clear
> the cache and it'd be a waste to clear the cache immediately after touching
the
> whole tree.

Yeah I actually had tried that but somehow the AX tree wasn't updated correctly
for http://demo.agektmr.com/dialog/, which suggests there may be a bug with the
updating logic. I'll try to track that down, maybe in a separate patch.

[falken, 2013-11-08 05:08:35]

https://codereview.chromium.org/59663005/diff/1/Source/core/accessibility/AXO...
File Source/core/accessibility/AXObject.cpp (right):

https://codereview.chromium.org/59663005/diff/1/Source/core/accessibility/AXO...
Source/core/accessibility/AXObject.cpp:292: bool canBeInInertSubtree = true;
On 2013/11/07 16:35:59, Dominic Mazzoni wrote:
> Do you mean "is allowed to be in inert subtreee" or "might possibly be in
inert
> subtree"?
> 
> If the latter, how about mightBeInInsertSubtree?

Done.

https://codereview.chromium.org/59663005/diff/1/Source/core/accessibility/AXO...
File Source/core/accessibility/AXObjectCache.h (right):

https://codereview.chromium.org/59663005/diff/1/Source/core/accessibility/AXO...
Source/core/accessibility/AXObjectCache.h:125: void
updateCacheAfterInertSubtreesChanged();
On 2013/11/07 16:35:59, Dominic Mazzoni wrote:
> Just call this handleInertSubtreesChanged(), and add a comment indicating that
> this happens when a dialog opens or closes, which may cause nodes all over the
> tree to suddenly become inert or not inert.

Done.

[commit-bot: I haz the power, 2013-11-08 05:09:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/59663005/90001

[commit-bot: I haz the power, 2013-11-08 06:09:52]

Retried try job too often on linux_blink_rel for step(s) weborigin_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-11-08 06:22:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/59663005/90001

[commit-bot: I haz the power, 2013-11-08 07:27:27]

Retried try job too often on linux_blink_rel for step(s) weborigin_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-11-08 08:13:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/59663005/90001

[commit-bot: I haz the power, 2013-11-08 09:14:58]

Retried try job too often on linux_blink_rel for step(s) weborigin_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-11-08 09:52:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/59663005/90001

[commit-bot: I haz the power, 2013-11-08 10:02:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/59663005/90001

[commit-bot: I haz the power, 2013-11-08 10:04:51]

Change committed as 161595