Improve the notifications sent from AXTree updates.

Improve the notifications sent from AXTree updates.

These notifications will be useful in order to fire the right Windows
accessibility events for certain tree changes, but this patch
just adds the notifications and refactors existing code to use them.

BUG=447962
Committed: https://crrev.com/a4b4891dec07229b618c7e8c6101cf8ec8b95038
Cr-Commit-Position: refs/heads/master@{#312973}

[dmazzoni, 2015-01-15 22:43:47]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org

[dmazzoni, 2015-01-21 00:07:12]

Friendly ping

[David Tseng, 2015-01-23 17:25:39]

lgtm

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree.h
File ui/accessibility/ax_tree.h (right):

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
ui/accessibility/ax_tree.h:23: // don't walk the parents and children at this
time:
At least for |OnSubtreeWillBeDeleted|, it appears to be safe to walk parents and
children.

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
ui/accessibility/ax_tree.h:31: // changed, for final postprocessing.
Do you mean |OnAtomicUpdateFinished|?

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
ui/accessibility/ax_tree.h:117: void DestroySubtree(AXNode* node);
Maybe call this NotifySubtreeWillBeDeleted?

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
File ui/accessibility/ax_tree_unittest.cc (right):

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
ui/accessibility/ax_tree_unittest.cc:39: if (root_changed)
I think you always want to assign this value (i.e. remove this line and assign
to the incoming param).

[dmazzoni, 2015-01-23 22:59:43]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2015-01-23 23:00:17]

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree.h
File ui/accessibility/ax_tree.h (right):

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
ui/accessibility/ax_tree.h:23: // don't walk the parents and children at this
time:
On 2015/01/23 17:25:39, David Tseng wrote:
> At least for |OnSubtreeWillBeDeleted|, it appears to be safe to walk parents
and
> children.

Technically you're right that the parents and children will be valid, but the
parent might have pointers to other children that are invalid. My preference is
to make this statement simpler and more strict, rather than allowing code to
depend on it being safe to walk the tree.

If we end up needing code to walk the tree when there's a node to be deleted, we
should probably refactor the code and create a separate callback for that.

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
ui/accessibility/ax_tree.h:31: // changed, for final postprocessing.
On 2015/01/23 17:25:39, David Tseng wrote:
> Do you mean |OnAtomicUpdateFinished|?

Oops, yes - I meant to include that in the comment. Done.

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
ui/accessibility/ax_tree.h:117: void DestroySubtree(AXNode* node);
On 2015/01/23 17:25:39, David Tseng wrote:
> Maybe call this NotifySubtreeWillBeDeleted?

It notifies and destroys

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
File ui/accessibility/ax_tree_unittest.cc (right):

https://codereview.chromium.org/830943004/diff/20001/ui/accessibility/ax_tree...
ui/accessibility/ax_tree_unittest.cc:39: if (root_changed)
On 2015/01/23 17:25:39, David Tseng wrote:
> I think you always want to assign this value (i.e. remove this line and assign
> to the incoming param).

Done.

[commit-bot: I haz the power, 2015-01-23 23:00:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/830943004/40001

[commit-bot: I haz the power, 2015-01-24 00:09:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-24 00:10:17]

Patchset 3 (id:??) landed as
https://crrev.com/a4b4891dec07229b618c7e8c6101cf8ec8b95038
Cr-Commit-Position: refs/heads/master@{#312973}