Implemented IAccessible2 reverse relations.

Implemented IAccessible2 reverse relations.

Only ones missing are IA2_RELATION_CHILD_OF and IA2_RELATION_PARENT_OF which are used for indicating aria-owns relationships and the parent-child relationships in trees.
Since the aria-owns requirement is obsolete, the remaining part is to implement parent - child relationships in trees.
BUG=619440
R=dmazzoni@chromium.org

Committed: https://crrev.com/70770adcfe2262b6886e19ac445d7c791892d011
Cr-Commit-Position: refs/heads/master@{#402485}

[dmazzoni, 2016-06-13 05:41:18]

https://codereview.chromium.org/2054393002/diff/1/content/browser/accessibili...
File content/browser/accessibility/browser_accessibility_win.cc (right):

https://codereview.chromium.org/2054393002/diff/1/content/browser/accessibili...
content/browser/accessibility/browser_accessibility_win.cc:4480:
target->AddRelation(reverse_relation_type, target_id);
I think this is going to keep adding this reverse relation to the target every
time this node is updated.

For example, suppose you have an <input> with aria-labelledby, and you focus it
and type into it. Every single time the <input> element is edited and its node
is updated, it's going to add a IA2_RELATION_LABEL_FOR relation to its target
label, since there's nothing to clear it.

One solution would be to clear all previous target relations first when a node
is updated. I'm wondering if a better solution might be to have some hash maps
of (source id, dest id) in a BrowserAccessibilityManager, that way we could more
easily update all of them when a node gets updated.

https://codereview.chromium.org/2054393002/diff/1/ui/accessibility/ax_tree_co...
File ui/accessibility/ax_tree_combiner.cc (right):

https://codereview.chromium.org/2054393002/diff/1/ui/accessibility/ax_tree_co...
ui/accessibility/ax_tree_combiner.cc:75: case AX_ATTR_LABELLEDBY_IDS:
This was sorted correctly before

[blink-reviews, 2016-06-13 21:18:08]

On 6/13/2016 1:41 AM, dmazzoni@chromium.org wrote:
>
>
https://codereview.chromium.org/2054393002/diff/1/content/browser/accessibili...
> File content/browser/accessibility/browser_accessibility_win.cc (right):
>
>
https://codereview.chromium.org/2054393002/diff/1/content/browser/accessibili...
> content/browser/accessibility/browser_accessibility_win.cc:4480:
> target->AddRelation(reverse_relation_type, target_id);
> I think this is going to keep adding this reverse relation to the target
> every time this node is updated.

This is now fixed and a unit test was added.
Every time that IA2 relations need to be re-computed I first go through 
and remove all the forward relations on the current object and all the 
reverse relations that are associated with them and which are stored on 
other objects. I don't touch my own reverse relations.
I didn't implement a dirty flag in |OnNodeWillChange| because I don't 
think that these relations are present on too many nodes so that it 
would be a performance hit, but happy to revisit.

Also:
AXTreeCombiner was reverted.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-06-13 21:18:08]

On 6/13/2016 1:41 AM, dmazzoni@chromium.org wrote:
>
>
https://codereview.chromium.org/2054393002/diff/1/content/browser/accessibili...
> File content/browser/accessibility/browser_accessibility_win.cc (right):
>
>
https://codereview.chromium.org/2054393002/diff/1/content/browser/accessibili...
> content/browser/accessibility/browser_accessibility_win.cc:4480:
> target->AddRelation(reverse_relation_type, target_id);
> I think this is going to keep adding this reverse relation to the target
> every time this node is updated.

This is now fixed and a unit test was added.
Every time that IA2 relations need to be re-computed I first go through 
and remove all the forward relations on the current object and all the 
reverse relations that are associated with them and which are stored on 
other objects. I don't touch my own reverse relations.
I didn't implement a dirty flag in |OnNodeWillChange| because I don't 
think that these relations are present on too many nodes so that it 
would be a performance hit, but happy to revisit.

Also:
AXTreeCombiner was reverted.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2016-06-14 05:45:52]

lgtm

Agreed, efficiency should be okay if these simply aren't used often.

Could you have the unit test actually update a node and confirm
it ends up in the same state?

[nektarios, 2016-06-22 22:51:22]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-06-22 22:51:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2054393002/#ps60001
(title: "Added test for updating a node.")

[commit-bot: I haz the power, 2016-06-22 22:51:51]

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

[commit-bot: I haz the power, 2016-06-22 23:01:11]

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

[commit-bot: I haz the power, 2016-06-22 23:01:12]

Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[nektarios, 2016-06-22 23:28:43]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-06-22 23:28:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2054393002/#ps80001
(title: "Added member_of_id to the list of printed attributes.")

[commit-bot: I haz the power, 2016-06-22 23:29:09]

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

[commit-bot: I haz the power, 2016-06-22 23:37:39]

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

[commit-bot: I haz the power, 2016-06-22 23:37:40]

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

[nektarios, 2016-06-22 23:42:05]

nektar@chromium.org changed reviewers:
+ dglazkov@chromium.org

[nektarios, 2016-06-22 23:43:04]

@dglazkov
For WebAXObject.

[blink-reviews, 2016-06-23 15:55:03]

Could you have the unit test actually update a node and confirm
> it ends up in the same state?
>
Done.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-06-23 15:55:04]

Could you have the unit test actually update a node and confirm
> it ends up in the same state?
>
Done.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dglazkov, 2016-06-27 20:18:39]

lgtm

[nektarios, 2016-06-27 20:26:19]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-06-27 20:27:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-27 20:37:54]

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

[commit-bot: I haz the power, 2016-06-27 20:37:55]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nektarios, 2016-06-27 21:17:56]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-06-27 21:17:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, dglazkov@chromium.org
Link to the patchset: https://codereview.chromium.org/2054393002/#ps100001
(title: "Added member_of_id to AXTreeCombiner.")

[commit-bot: I haz the power, 2016-06-27 21:18:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-27 22:42:38]

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

[commit-bot: I haz the power, 2016-06-27 22:42:40]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[nektarios, 2016-06-28 00:56:19]

The CQ bit was checked by nektar@chromium.org

[nektarios, 2016-06-28 00:56:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, dglazkov@chromium.org
Link to the patchset: https://codereview.chromium.org/2054393002/#ps120001
(title: "Fixed content_browsertests.")

[commit-bot: I haz the power, 2016-06-28 00:57:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 02:08:09]

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

[commit-bot: I haz the power, 2016-06-28 02:08:11]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nektarios, 2016-06-28 15:20:20]

The CQ bit was checked by nektar@chromium.org

[commit-bot: I haz the power, 2016-06-28 15:20:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 16:15:47]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-06-28 16:17:22]

Description was changed from

==========
Implemented IAccessible2 reverse relations.

Only ones missing are IA2_RELATION_CHILD_OF and IA2_RELATION_PARENT_OF which are
used for indicating aria-owns relationships and the parent-child relationships
in trees.
Since the aria-owns requirement is obsolete, the remaining part is to implement
parent - child relationships in trees.
BUG=619440
R=dmazzoni@chromium.org
==========

to

==========
Implemented IAccessible2 reverse relations.

Only ones missing are IA2_RELATION_CHILD_OF and IA2_RELATION_PARENT_OF which are
used for indicating aria-owns relationships and the parent-child relationships
in trees.
Since the aria-owns requirement is obsolete, the remaining part is to implement
parent - child relationships in trees.
BUG=619440
R=dmazzoni@chromium.org

Committed: https://crrev.com/70770adcfe2262b6886e19ac445d7c791892d011
Cr-Commit-Position: refs/heads/master@{#402485}
==========

[commit-bot: I haz the power, 2016-06-28 16:17:24]

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