AXTreeCombiner no longer needs to convert to global coordinates.

AXTreeCombiner no longer needs to convert to global coordinates.

AXTreeCombiner is used to combine accessibility trees from multiple frames
in order to create a single accessibility tree, used to provide information
about a web page to the Android assist API, used by features like Now-on-Tap.

Prior to change https://codereview.chromium.org/2217363002, only the root
frame of an accessibility tree was allowed to have a transformation matrix.
So in order to combine frames, AXTreeCombiner had to apply transformation
matrices manually, and make all coordinates global, otherwise the downstream
consumer of the accessibility tree wouldn't apply transformation matrices
from subframes.

That worked, but then in change 2217363002, we made all accessibility
coordinates relative, by allowing transformation matrices anywhere in the
tree. This eliminated the need for AXTreeCombiner to make all
coordinates global, but AXTreeCombiner was improperly updated in
this change, and in many circumstances it was pretending to return
global coordinates but not actually fully implement the new local-to-global
protocol as documented in ax_relative_bounds.h.

The proper fix is to simplify AXTreeCombiner. It's no longer necessary for it
to modify coordinates and apply transformation matrices at all. Just leaving
the coordinates alone now works correctly.

More end-to-end testing is needed. ax_tree_combiner_unittest only tests the
tree combining, and now that coordinates are passed through unmodified, there's
nothing really to test. We need a test of
Java_WebContentsImpl_onAccessibilitySnapshot that tests the resulting
coordinates passed to Java.

BUG=673935
Committed: https://crrev.com/cbc8b96da2d7987a42ededb8017b2f0aca445553
Cr-Commit-Position: refs/heads/master@{#438572}

[dmazzoni, 2016-12-14 00:03:05]

dmazzoni@chromium.org changed reviewers:
+ aboxhall@chromium.org, sgurun@chromium.org

[dmazzoni, 2016-12-14 00:03:06]

The CQ bit was checked by dmazzoni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 00:03:49]

Dry run: 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-12-14 00:42:47]

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

[commit-bot: I haz the power, 2016-12-14 00:42:48]

Dry run: 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_...)

[sgurun-gerrit only, 2016-12-14 03:43:19]

On 2016/12/14 00:03:05, dmazzoni wrote:

this is failing tests.  I will look after tests are fixed.

[dmazzoni, 2016-12-14 16:30:21]

The change caused a harmless arbitrary difference
in the test results. I fixed it so that the test results
are identical to before.

[dmazzoni, 2016-12-14 16:33:34]

The CQ bit was checked by dmazzoni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 16:33:59]

Dry run: 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-12-14 17:35:20]

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

[commit-bot: I haz the power, 2016-12-14 17:35:20]

Dry run: This issue passed the CQ dry run.

[sgurun-gerrit only, 2016-12-14 18:15:14]

On 2016/12/14 16:30:21, dmazzoni wrote:
> The change caused a harmless arbitrary difference
> in the test results. I fixed it so that the test results
> are identical to before.

lgtm

[aboxhall, 2016-12-14 18:58:24]

LGTM 
🎉

[dmazzoni, 2016-12-14 19:01:49]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2016-12-14 19:03:51]

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-12-14 19:10:17]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481742109834620,
"parent_rev": "7d69f7322acdb17b933346853698408c8300e96b", "commit_rev":
"7c494023b2caa9a7aff33d9bb5b456a49c85fe21"}

[commit-bot: I haz the power, 2016-12-14 19:10:43]

Description was changed from

==========
AXTreeCombiner no longer needs to convert to global coordinates.

AXTreeCombiner is used to combine accessibility trees from multiple frames
in order to create a single accessibility tree, used to provide information
about a web page to the Android assist API, used by features like Now-on-Tap.

Prior to change https://codereview.chromium.org/2217363002, only the root
frame of an accessibility tree was allowed to have a transformation matrix.
So in order to combine frames, AXTreeCombiner had to apply transformation
matrices manually, and make all coordinates global, otherwise the downstream
consumer of the accessibility tree wouldn't apply transformation matrices
from subframes.

That worked, but then in change 2217363002, we made all accessibility
coordinates relative, by allowing transformation matrices anywhere in the
tree. This eliminated the need for AXTreeCombiner to make all
coordinates global, but AXTreeCombiner was improperly updated in
this change, and in many circumstances it was pretending to return
global coordinates but not actually fully implement the new local-to-global
protocol as documented in ax_relative_bounds.h.

The proper fix is to simplify AXTreeCombiner. It's no longer necessary for it
to modify coordinates and apply transformation matrices at all. Just leaving
the coordinates alone now works correctly.

More end-to-end testing is needed. ax_tree_combiner_unittest only tests the
tree combining, and now that coordinates are passed through unmodified, there's
nothing really to test. We need a test of
Java_WebContentsImpl_onAccessibilitySnapshot that tests the resulting
coordinates passed to Java.

BUG=673935
==========

to

==========
AXTreeCombiner no longer needs to convert to global coordinates.

AXTreeCombiner is used to combine accessibility trees from multiple frames
in order to create a single accessibility tree, used to provide information
about a web page to the Android assist API, used by features like Now-on-Tap.

Prior to change https://codereview.chromium.org/2217363002, only the root
frame of an accessibility tree was allowed to have a transformation matrix.
So in order to combine frames, AXTreeCombiner had to apply transformation
matrices manually, and make all coordinates global, otherwise the downstream
consumer of the accessibility tree wouldn't apply transformation matrices
from subframes.

That worked, but then in change 2217363002, we made all accessibility
coordinates relative, by allowing transformation matrices anywhere in the
tree. This eliminated the need for AXTreeCombiner to make all
coordinates global, but AXTreeCombiner was improperly updated in
this change, and in many circumstances it was pretending to return
global coordinates but not actually fully implement the new local-to-global
protocol as documented in ax_relative_bounds.h.

The proper fix is to simplify AXTreeCombiner. It's no longer necessary for it
to modify coordinates and apply transformation matrices at all. Just leaving
the coordinates alone now works correctly.

More end-to-end testing is needed. ax_tree_combiner_unittest only tests the
tree combining, and now that coordinates are passed through unmodified, there's
nothing really to test. We need a test of
Java_WebContentsImpl_onAccessibilitySnapshot that tests the resulting
coordinates passed to Java.

BUG=673935

Review-Url: https://codereview.chromium.org/2572923002
==========

[commit-bot: I haz the power, 2016-12-14 19:10:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-14 19:14:07]

Description was changed from

==========
AXTreeCombiner no longer needs to convert to global coordinates.

AXTreeCombiner is used to combine accessibility trees from multiple frames
in order to create a single accessibility tree, used to provide information
about a web page to the Android assist API, used by features like Now-on-Tap.

Prior to change https://codereview.chromium.org/2217363002, only the root
frame of an accessibility tree was allowed to have a transformation matrix.
So in order to combine frames, AXTreeCombiner had to apply transformation
matrices manually, and make all coordinates global, otherwise the downstream
consumer of the accessibility tree wouldn't apply transformation matrices
from subframes.

That worked, but then in change 2217363002, we made all accessibility
coordinates relative, by allowing transformation matrices anywhere in the
tree. This eliminated the need for AXTreeCombiner to make all
coordinates global, but AXTreeCombiner was improperly updated in
this change, and in many circumstances it was pretending to return
global coordinates but not actually fully implement the new local-to-global
protocol as documented in ax_relative_bounds.h.

The proper fix is to simplify AXTreeCombiner. It's no longer necessary for it
to modify coordinates and apply transformation matrices at all. Just leaving
the coordinates alone now works correctly.

More end-to-end testing is needed. ax_tree_combiner_unittest only tests the
tree combining, and now that coordinates are passed through unmodified, there's
nothing really to test. We need a test of
Java_WebContentsImpl_onAccessibilitySnapshot that tests the resulting
coordinates passed to Java.

BUG=673935

Review-Url: https://codereview.chromium.org/2572923002
==========

to

==========
AXTreeCombiner no longer needs to convert to global coordinates.

AXTreeCombiner is used to combine accessibility trees from multiple frames
in order to create a single accessibility tree, used to provide information
about a web page to the Android assist API, used by features like Now-on-Tap.

Prior to change https://codereview.chromium.org/2217363002, only the root
frame of an accessibility tree was allowed to have a transformation matrix.
So in order to combine frames, AXTreeCombiner had to apply transformation
matrices manually, and make all coordinates global, otherwise the downstream
consumer of the accessibility tree wouldn't apply transformation matrices
from subframes.

That worked, but then in change 2217363002, we made all accessibility
coordinates relative, by allowing transformation matrices anywhere in the
tree. This eliminated the need for AXTreeCombiner to make all
coordinates global, but AXTreeCombiner was improperly updated in
this change, and in many circumstances it was pretending to return
global coordinates but not actually fully implement the new local-to-global
protocol as documented in ax_relative_bounds.h.

The proper fix is to simplify AXTreeCombiner. It's no longer necessary for it
to modify coordinates and apply transformation matrices at all. Just leaving
the coordinates alone now works correctly.

More end-to-end testing is needed. ax_tree_combiner_unittest only tests the
tree combining, and now that coordinates are passed through unmodified, there's
nothing really to test. We need a test of
Java_WebContentsImpl_onAccessibilitySnapshot that tests the resulting
coordinates passed to Java.

BUG=673935

Committed: https://crrev.com/cbc8b96da2d7987a42ededb8017b2f0aca445553
Cr-Commit-Position: refs/heads/master@{#438572}
==========

[commit-bot: I haz the power, 2016-12-14 19:14:09]

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