https://codereview.chromium.org/2572923002/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbc8b96da2d7987a42ededb8017b2f0aca445553

commit cbc8b96da2d7987a42ededb8017b2f0aca445553
Author: dmazzoni <dmazzoni@chromium.org>
Date: Wed Dec 14 19:10:14 2016

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
Cr-Commit-Position: refs/heads/master@{#438572}

[modify] https://crrev.com/cbc8b96da2d7987a42ededb8017b2f0aca445553/ui/accessibility/ax_tree_combiner.cc
[modify] https://crrev.com/cbc8b96da2d7987a42ededb8017b2f0aca445553/ui/accessibility/ax_tree_combiner.h
[modify] https://crrev.com/cbc8b96da2d7987a42ededb8017b2f0aca445553/ui/accessibility/ax_tree_combiner_unittest.cc

Your change meets the bar and is auto-approved for M56. Please go ahead and merge the CL manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d6ddb0ef53db84aa3f192ad6c42578179a422a0

commit 8d6ddb0ef53db84aa3f192ad6c42578179a422a0
Author: Selim Gurun <sgurun@chromium.org>
Date: Wed Jan 04 21:55:25 2017

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
Cr-Commit-Position: refs/heads/master@{#438572}
(cherry picked from commit cbc8b96da2d7987a42ededb8017b2f0aca445553)

Review-Url: https://codereview.chromium.org/2618513003 .
Cr-Commit-Position: refs/branch-heads/2924@{#671}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/8d6ddb0ef53db84aa3f192ad6c42578179a422a0/ui/accessibility/ax_tree_combiner.cc
[modify] https://crrev.com/8d6ddb0ef53db84aa3f192ad6c42578179a422a0/ui/accessibility/ax_tree_combiner.h
[modify] https://crrev.com/8d6ddb0ef53db84aa3f192ad6c42578179a422a0/ui/accessibility/ax_tree_combiner_unittest.cc

Verified this fix as per #0 steps with latest M56 on Nexus 5X/MOB31R, Pixel C/ NRD91R, Pixel/NMF26X and Pixel L/N.

Thanks!

[Bulk edit]

Greetings from the release team!  This issue is marked as a stable release blocker for M56 on Android.  We're planning to ship our final beta release on Jan 25, so you have less than *two weeks* to fix this issue on trunk and merge the change back to the M56 branch.  Please prioritize working on this ASAP!

Sure this bug shouldn't block the release?  Remove the ReleaseBlock-Stable tag.

Don't think this bug should block the release, but not 100% sure?  CC me to the bug and let me know!

Cheers,
Alex