Fire AX text inserted event when embedded obj char changes.

Fire AX text inserted event when embedded obj char changes.

See bug for details. This patch makes it so that we fire
the events  IA2_EVENT_TEXT_INSERTED and IA2_EVENT_TEXT_REMOVED
when the children referenced by the embedded object characters
changed, not just when the text changed.

This patch also avoids incorrectly firing these same two
events on a newly-created node.

BUG=425861
Committed: https://crrev.com/57831c0d68b35843942e19e30d0c9c86d2005099
Cr-Commit-Position: refs/heads/master@{#314280}

[dmazzoni, 2015-01-30 21:53:10]

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

[dmazzoni, 2015-01-30 21:53:11]

Ready for review!

[David Tseng, 2015-01-30 23:57:25]

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_manager_win.cc (right):

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_manager_win.cc:311: //
concatenatenation of all of its child text nodes, so it can't run until
nit: contatenation

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_manager_win.cc:338:
obj->ToBrowserAccessibilityWin()->UpdateStep4DeleteOldWinAttributes();
Could you combine this last step with the above one?

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3241: if
(is_new_object)
nit: no need for this local var.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3291: if (GetParent()
&&
nit: store parent in a local variable.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3301: // Fire
hypertext-related events.
This would mean this object's parent fires hypertext related events first; does
that matter? Also, this seems like it would trigger a lot of events (along an
ancestry chain).

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3304: if (old_len) {
nit: old_len > 0?

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3309: if (new_len) {
ditto

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3310: // In-process
screen readers may call IAccessibleText::get_newText
in reaction to this event?

[dmazzoni, 2015-01-31 07:45:21]

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_manager_win.cc (right):

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_manager_win.cc:311: //
concatenatenation of all of its child text nodes, so it can't run until
On 2015/01/30 23:57:25, David Tseng wrote:
> nit: contatenation

Done.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_manager_win.cc:338:
obj->ToBrowserAccessibilityWin()->UpdateStep4DeleteOldWinAttributes();
On 2015/01/30 23:57:25, David Tseng wrote:
> Could you combine this last step with the above one?

Sure. It doesn't really need its own step.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3241: if
(is_new_object)
On 2015/01/30 23:57:25, David Tseng wrote:
> nit: no need for this local var.

Done.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3291: if (GetParent()
&&
On 2015/01/30 23:57:25, David Tseng wrote:
> nit: store parent in a local variable.

Done.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3301: // Fire
hypertext-related events.
On 2015/01/30 23:57:25, David Tseng wrote:
> This would mean this object's parent fires hypertext related events first;
does
> that matter? Also, this seems like it would trigger a lot of events (along an
> ancestry chain).

No, I checked with NV Access. The order in which we fire the hypertext events
doesn't matter, but as soon as we fire one they might explore the tree so we
have to have finished computing the hypertext in the previous pass.

Changing one node doesn't trigger events up the ancestry chain, just that node's
parent will have its hypertext change.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3304: if (old_len) {
On 2015/01/30 23:57:25, David Tseng wrote:
> nit: old_len > 0?

Done.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3309: if (new_len) {
On 2015/01/30 23:57:25, David Tseng wrote:
> ditto

Done.

https://codereview.chromium.org/859133003/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility_win.cc:3310: // In-process
screen readers may call IAccessibleText::get_newText
On 2015/01/30 23:57:25, David Tseng wrote:
> in reaction to this event?

Done.

[David Tseng, 2015-02-02 16:26:43]

lgtm

[dmazzoni, 2015-02-02 16:27:24]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2015-02-02 16:28:08]

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

[commit-bot: I haz the power, 2015-02-02 16:34:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-02 16:34:26]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dmazzoni, 2015-02-02 23:33:13]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2015-02-02 23:34:07]

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

[commit-bot: I haz the power, 2015-02-03 01:35:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-03 01:35:11]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win (None)

[dmazzoni, 2015-02-03 06:54:41]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2015-02-03 06:55:46]

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

[commit-bot: I haz the power, 2015-02-03 06:56:48]

Committed patchset #7 (id:100001)

[commit-bot: I haz the power, 2015-02-03 06:57:47]

Patchset 7 (id:??) landed as
https://crrev.com/57831c0d68b35843942e19e30d0c9c86d2005099
Cr-Commit-Position: refs/heads/master@{#314280}