Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(241)

Issue 2640123004: Initial support for native accessibility in ARC (Closed)

Created:
3 years, 11 months ago by David Tseng
Modified:
3 years, 10 months ago
Reviewers:
yawano, Peng, oshima, nasko, dmazzoni
CC:
chromium-reviews, dmazzoni
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Initial support for native accessibility in ARC This is an initial implementation of native ARC++ accessibility. An accessibility tree is forwarded to Chrome and Chrome forwards tree data to extensions. TEST=manual - in ChromeVox, in a js-background page context, verify the contents of the accessibility tree from ARC show up; when events occur, verify events are received; when the ARC+++ surface is deactivated, verify tree gets destroyed/deatched - touch explore with ChromeVox; verify basic speech feedback - ChromeVox object navigation works BUG=683396 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2640123004 Cr-Commit-Position: refs/heads/master@{#452029} Committed: https://chromium.googlesource.com/chromium/src/+/be42343c9fd8b9fbb5733faf2e93abc3ff648b7f

Patch Set 1 : m #

Total comments: 10

Patch Set 2 : address feedback #

Total comments: 4

Patch Set 3 : address feedback #

Patch Set 4 : Add missing deps entry #

Patch Set 5 : Simplify. #

Patch Set 6 : Fix analyze #

Patch Set 7 : Export. #

Patch Set 8 : Fix Mac compile. #

Patch Set 9 : Rebase. #

Patch Set 10 : Rebase on top of yawano's changes. #

Patch Set 11 : Experimental changes #

Total comments: 2

Patch Set 12 : Ready for review #

Patch Set 13 : Add filter type off. #

Patch Set 14 : Add boolean properties field. #

Patch Set 15 : Individual settings for Talkback vs native ax. #

Patch Set 16 : Remove temp id. #

Patch Set 17 : Support focus! #

Patch Set 18 : |window| can be nullptr #

Total comments: 22

Patch Set 19 : Address various feedback. #

Total comments: 3

Patch Set 20 : Address some changes #

Patch Set 21 : Fix touch exploration (map view_hover_entered to hover on Chrome end); hover doesn't map well to fo… #

Patch Set 22 : Rebase. #

Patch Set 23 : override #

Total comments: 7

Patch Set 24 : Address nasko's comments. #

Total comments: 4

Patch Set 25 : Redundant include. #

Patch Set 26 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+706 lines, -200 lines) Patch
M chrome/browser/chromeos/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +2 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/arc/accessibility/OWNERS View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 3 chunks +22 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 2 chunks +117 lines, -18 lines 0 comments Download
A chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +77 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +249 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 3 chunks +10 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/automation_internal/automation_event_router.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +2 lines, -0 lines 0 comments Download
M chromeos/chromeos_switches.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -0 lines 0 comments Download
M chromeos/chromeos_switches.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +3 lines, -0 lines 0 comments Download
M components/arc/common/accessibility_helper.mojom View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +160 lines, -7 lines 0 comments Download
M content/browser/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +0 lines, -2 lines 0 comments Download
M content/browser/accessibility/ax_platform_position.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +2 lines, -2 lines 0 comments Download
D content/browser/accessibility/ax_tree_id_registry.h View 1 2 3 4 1 chunk +0 lines, -58 lines 0 comments Download
D content/browser/accessibility/ax_tree_id_registry.cc View 1 2 3 4 1 chunk +0 lines, -61 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_cocoa.mm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 2 chunks +4 lines, -4 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 5 chunks +5 lines, -4 lines 0 comments Download
M content/browser/accessibility/browser_accessibility_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 5 chunks +6 lines, -5 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 4 chunks +7 lines, -6 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 8 chunks +14 lines, -15 lines 0 comments Download
M ui/accessibility/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +2 lines, -0 lines 0 comments Download
A + ui/accessibility/ax_tree_id_registry.h View 1 2 3 4 5 6 3 chunks +11 lines, -7 lines 0 comments Download
A + ui/accessibility/ax_tree_id_registry.cc View 1 2 3 4 4 chunks +10 lines, -6 lines 0 comments Download

Messages

Total messages: 142 (105 generated)
David Tseng
Ready for a first look. reveman: exo changes yawano: ARC++ AXTreeSourceArc stub/integration with accessibility helper. ...
3 years, 11 months ago (2017-01-19 17:48:22 UTC) #4
dmazzoni
https://codereview.chromium.org/2640123004/diff/20001/chrome/browser/chromeos/arc/accessibility_helper/ax_tree_source_arc.h File chrome/browser/chromeos/arc/accessibility_helper/ax_tree_source_arc.h (right): https://codereview.chromium.org/2640123004/diff/20001/chrome/browser/chromeos/arc/accessibility_helper/ax_tree_source_arc.h#newcode16 chrome/browser/chromeos/arc/accessibility_helper/ax_tree_source_arc.h:16: // This class represents the accessibility tree from the ...
3 years, 11 months ago (2017-01-19 19:48:16 UTC) #7
yawano
Do we have a bug for this? It would be better to file a bug ...
3 years, 11 months ago (2017-01-20 07:47:49 UTC) #12
David Tseng
https://codereview.chromium.org/2640123004/diff/20001/chrome/browser/chromeos/arc/accessibility_helper/ax_tree_source_arc.cc File chrome/browser/chromeos/arc/accessibility_helper/ax_tree_source_arc.cc (right): https://codereview.chromium.org/2640123004/diff/20001/chrome/browser/chromeos/arc/accessibility_helper/ax_tree_source_arc.cc#newcode72 chrome/browser/chromeos/arc/accessibility_helper/ax_tree_source_arc.cc:72: surface->set_ax_tree_source(nullptr); On 2017/01/20 07:47:49, yawano wrote: > Might we ...
3 years, 11 months ago (2017-01-20 23:00:56 UTC) #14
yawano
Thank you! c/b/chromeos/arc/accessibility_helper lgtm with a nit (please consider to rename accessibility_helper to accessibility as ...
3 years, 11 months ago (2017-01-23 10:41:53 UTC) #19
David Tseng
https://codereview.chromium.org/2640123004/diff/40001/chrome/browser/chromeos/BUILD.gn File chrome/browser/chromeos/BUILD.gn (right): https://codereview.chromium.org/2640123004/diff/40001/chrome/browser/chromeos/BUILD.gn#newcode211 chrome/browser/chromeos/BUILD.gn:211: "arc/accessibility_helper/ax_tree_source_arc.cc", On 2017/01/23 10:41:53, yawano wrote: > Could you ...
3 years, 11 months ago (2017-01-23 20:56:00 UTC) #20
David Tseng
Friendly ping reveman@.
3 years, 11 months ago (2017-01-23 20:57:07 UTC) #21
David Tseng
PTAL @dmazzoni; moved around the tree registry; and minimized exo dependency.
3 years, 11 months ago (2017-01-24 22:35:54 UTC) #39
reveman
I think I need a high level description of how this will be used. I'm ...
3 years, 10 months ago (2017-01-27 18:27:55 UTC) #56
David Tseng
reveman@chromium.org writes: > I think I need a high level description of how this will ...
3 years, 10 months ago (2017-01-27 23:55:38 UTC) #57
yawano
One question about the new interface. https://codereview.chromium.org/2640123004/diff/220001/components/arc/common/accessibility_helper.mojom File components/arc/common/accessibility_helper.mojom (right): https://codereview.chromium.org/2640123004/diff/220001/components/arc/common/accessibility_helper.mojom#newcode37 components/arc/common/accessibility_helper.mojom:37: array<AccessibilityNodeInfoData> nodeData; What ...
3 years, 10 months ago (2017-02-08 11:11:27 UTC) #58
David Tseng
PTAL. This is now ready for a full review. Note that subsequent cl's will optimize ...
3 years, 10 months ago (2017-02-08 18:49:26 UTC) #61
dmazzoni
https://codereview.chromium.org/2640123004/diff/360001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc File chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc (right): https://codereview.chromium.org/2640123004/diff/360001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc#newcode71 chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc:71: } else { nit: maybe just return inside the ...
3 years, 10 months ago (2017-02-09 01:18:10 UTC) #70
David Tseng
Replying to one question from dmazzoni. +reveman, can you please answer the question below? https://codereview.chromium.org/2640123004/diff/360001/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.h ...
3 years, 10 months ago (2017-02-09 03:10:09 UTC) #72
yawano
Thank you for working on this! https://codereview.chromium.org/2640123004/diff/360001/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc File chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc (right): https://codereview.chromium.org/2640123004/diff/360001/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc#newcode90 chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc:90: root_id_ = id; ...
3 years, 10 months ago (2017-02-09 10:55:30 UTC) #73
David Tseng
dmazzoni@chromium.org writes: > https://codereview.chromium.org/2640123004/diff/360001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc > File > chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc > (right): > > https://codereview.chromium.org/2640123004/diff/360001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc#newcode71 > chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc:71: ...
3 years, 10 months ago (2017-02-13 18:56:49 UTC) #74
chromium-reviews
From: David Tseng <dtseng@google.com> Date: Mon, 13 Feb 2017 15:16:21 -0800 Message-ID: <xq5defz1adh6.fsf@google.com> MIME-Version: 1.0 ...
3 years, 10 months ago (2017-02-13 23:16:27 UTC) #75
dmazzoni
lgtm This looks great. Still hoping for a cleaner solution to associating accessibility events with ...
3 years, 10 months ago (2017-02-14 00:31:15 UTC) #76
yawano
mostly lgtm, just a few comments. https://codereview.chromium.org/2640123004/diff/380001/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc File chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc (right): https://codereview.chromium.org/2640123004/diff/380001/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc#newcode236 chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc:236: current_tree_serializer_.reset(new AXTreeArcSerializer(this)); Do ...
3 years, 10 months ago (2017-02-14 01:35:02 UTC) #77
David Tseng
FYI. I removed the window property changes in favor of the original approach of adding ...
3 years, 10 months ago (2017-02-14 20:46:04 UTC) #78
chromium-reviews
From: David Tseng <dtseng@google.com> Date: Tue, 14 Feb 2017 12:49:07 -0800 Message-ID: <xq5dshng33cs.fsf@google.com> MIME-Version: 1.0 ...
3 years, 10 months ago (2017-02-14 20:49:14 UTC) #79
David Tseng
+ oshima for chrome/browser/chromeos/ + nasko for content/* chromecomponents/arc/common/
3 years, 10 months ago (2017-02-14 21:35:01 UTC) #81
nasko
https://codereview.chromium.org/2640123004/diff/460001/components/arc/common/accessibility_helper.mojom File components/arc/common/accessibility_helper.mojom (right): https://codereview.chromium.org/2640123004/diff/460001/components/arc/common/accessibility_helper.mojom#newcode21 components/arc/common/accessibility_helper.mojom:21: VIEW_SELECTED, The ordering doesn't seem to quite match the ...
3 years, 10 months ago (2017-02-15 19:49:40 UTC) #92
David Tseng
https://codereview.chromium.org/2640123004/diff/460001/components/arc/common/accessibility_helper.mojom File components/arc/common/accessibility_helper.mojom (right): https://codereview.chromium.org/2640123004/diff/460001/components/arc/common/accessibility_helper.mojom#newcode21 components/arc/common/accessibility_helper.mojom:21: VIEW_SELECTED, On 2017/02/15 19:49:40, nasko wrote: > The ordering ...
3 years, 10 months ago (2017-02-15 20:41:02 UTC) #93
oshima
https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h File chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h (right): https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h#newcode13 chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h:13: #include "components/exo/wm_helper.h" Hmm, I think you're not supposed to ...
3 years, 10 months ago (2017-02-16 04:16:45 UTC) #95
nasko
LGTM
3 years, 10 months ago (2017-02-16 18:01:54 UTC) #96
David Tseng
https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h File chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h (right): https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h#newcode13 chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h:13: #include "components/exo/wm_helper.h" On 2017/02/16 04:16:45, oshima wrote: > Hmm, ...
3 years, 10 months ago (2017-02-16 18:36:40 UTC) #97
Peng
https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h File chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h (right): https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h#newcode13 chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h:13: #include "components/exo/wm_helper.h" On 2017/02/16 04:16:45, oshima wrote: > Hmm, ...
3 years, 10 months ago (2017-02-16 18:51:04 UTC) #98
David Tseng
https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h File chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h (right): https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h#newcode13 chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h:13: #include "components/exo/wm_helper.h" On 2017/02/16 18:51:04, Peng wrote: > On ...
3 years, 10 months ago (2017-02-16 19:14:38 UTC) #99
oshima
On 2017/02/16 19:14:38, David Tseng wrote: > https://codereview.chromium.org/2640123004/diff/480001/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h > File chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h > (right): > > ...
3 years, 10 months ago (2017-02-16 22:49:19 UTC) #100
oshima
On 2017/02/16 22:49:19, oshima wrote: > On 2017/02/16 19:14:38, David Tseng wrote: > > > ...
3 years, 10 months ago (2017-02-16 22:49:52 UTC) #101
David Tseng
On 2017/02/16 22:49:52, oshima wrote: > On 2017/02/16 22:49:19, oshima wrote: > > On 2017/02/16 ...
3 years, 10 months ago (2017-02-17 15:26:44 UTC) #102
oshima
On 2017/02/17 15:26:44, David Tseng wrote: > On 2017/02/16 22:49:52, oshima wrote: > > On ...
3 years, 10 months ago (2017-02-17 16:55:48 UTC) #103
David Tseng
On 2017/02/17 16:55:48, oshima wrote: > On 2017/02/17 15:26:44, David Tseng wrote: > > On ...
3 years, 10 months ago (2017-02-21 16:54:22 UTC) #104
oshima
per offline chat, lgtm'ing this CL because it already depends on exo/wm_helper. We should remove ...
3 years, 10 months ago (2017-02-21 23:42:29 UTC) #124
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2640123004/620001
3 years, 10 months ago (2017-02-22 14:01:14 UTC) #139
commit-bot: I haz the power
3 years, 10 months ago (2017-02-22 14:07:22 UTC) #142
Message was sent while issue was closed.
Committed patchset #26 (id:620001) as
https://chromium.googlesource.com/chromium/src/+/be42343c9fd8b9fbb5733faf2e93...

Powered by Google App Engine
This is Rietveld 408576698