Implement Chrome OS accessibility features to highlight focus, caret & cursor.

Implement Chrome OS accessibility features to highlight focus, caret & cursor.

This extends the existing code that handles drawing a focus ring in an overlay
layer for ChromeVox and for the keyboard-driven OOBE, and implements support
for standalone features to highlight the focused object, the mouse cursor,
and the text caret.

The preferences to enable highlighting focus, caret, and cursor are behind
a flag, so the details of the implementation may be tweaked before this ships.
Currently each one has a distinct color, and the cursor highlight fades out
when the cursor isn't moving.

BUG=314889
Committed: https://crrev.com/b93107c14278a62f0a139fa6db64af89a7f20924
Cr-Commit-Position: refs/heads/master@{#382693}

[dmazzoni, 2016-03-21 21:09:24]

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

[xiyuan, 2016-03-21 22:59:31]

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/acc...
File chrome/browser/chromeos/accessibility/accessibility_highlight_manager.cc
(right):

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/acc...
chrome/browser/chromeos/accessibility/accessibility_highlight_manager.cc:93:
cursor_point_ = event->root_location();
Think we should use event->location() instead of root_location since
AccessibilityFocusRingController expects screen coordinates.

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/acc...
File chrome/browser/chromeos/accessibility/accessibility_highlight_manager.h
(right):

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/acc...
chrome/browser/chromeos/accessibility/accessibility_highlight_manager.h:61: };
nit: DISALLOW_COPY_AND_ASSIGN(AccessibilityHighlightManager);

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/ui/...
File chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc (right):

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/ui/...
chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc:8: 
nit: nuke empty line?

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/ui/...
chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc:79: if
(rings_.size() == 0)
nit: rings_.empty()

https://codereview.chromium.org/1822823002/diff/1/chrome/chrome_browser_chrom...
File chrome/chrome_browser_chromeos.gypi (right):

https://codereview.chromium.org/1822823002/diff/1/chrome/chrome_browser_chrom...
chrome/chrome_browser_chromeos.gypi:1010:
'browser/chromeos/ui/accessibility_cursor_ring_layer.h',
Where are these files?

[dmazzoni, 2016-03-22 04:09:30]

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/acc...
File chrome/browser/chromeos/accessibility/accessibility_highlight_manager.cc
(right):

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/acc...
chrome/browser/chromeos/accessibility/accessibility_highlight_manager.cc:93:
cursor_point_ = event->root_location();
On 2016/03/21 22:59:31, xiyuan wrote:
> Think we should use event->location() instead of root_location since
> AccessibilityFocusRingController expects screen coordinates. 

Done. I'll do some more testing with multi-monitor too.

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/acc...
File chrome/browser/chromeos/accessibility/accessibility_highlight_manager.h
(right):

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/acc...
chrome/browser/chromeos/accessibility/accessibility_highlight_manager.h:61: };
On 2016/03/21 22:59:31, xiyuan wrote:
> nit: DISALLOW_COPY_AND_ASSIGN(AccessibilityHighlightManager);

Done.

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/ui/...
File chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc (right):

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/ui/...
chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc:8: 
On 2016/03/21 22:59:31, xiyuan wrote:
> nit: nuke empty line?

Done.

https://codereview.chromium.org/1822823002/diff/1/chrome/browser/chromeos/ui/...
chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc:79: if
(rings_.size() == 0)
On 2016/03/21 22:59:31, xiyuan wrote:
> nit: rings_.empty()

Done.

https://codereview.chromium.org/1822823002/diff/1/chrome/chrome_browser_chrom...
File chrome/chrome_browser_chromeos.gypi (right):

https://codereview.chromium.org/1822823002/diff/1/chrome/chrome_browser_chrom...
chrome/chrome_browser_chromeos.gypi:1010:
'browser/chromeos/ui/accessibility_cursor_ring_layer.h',
On 2016/03/21 22:59:31, xiyuan wrote:
> Where are these files?

Ack, forgot to add them to git.

Uploaded now.

[xiyuan, 2016-03-22 04:34:27]

lgtm

https://codereview.chromium.org/1822823002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/ui/accessibility_cursor_ring_layer.h (right):

https://codereview.chromium.org/1822823002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/ui/accessibility_cursor_ring_layer.h:17: explicit
AccessibilityCursorRingLayer(FocusRingLayerDelegate* delegate,
nit: explicit not needed when ctor has >1 args.

https://codereview.chromium.org/1822823002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc (right):

https://codereview.chromium.org/1822823002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc:8: #include
<algorithm>
nit: alpha sort the headers?

[dmazzoni, 2016-03-22 20:23:02]

Thanks.

I'm going to land without the change to the views delegate
for now, I'll address that in a follow-up.

https://codereview.chromium.org/1822823002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/ui/accessibility_cursor_ring_layer.h (right):

https://codereview.chromium.org/1822823002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/ui/accessibility_cursor_ring_layer.h:17: explicit
AccessibilityCursorRingLayer(FocusRingLayerDelegate* delegate,
On 2016/03/22 04:34:27, xiyuan wrote:
> nit: explicit not needed when ctor has >1 args.

Done.

https://codereview.chromium.org/1822823002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc (right):

https://codereview.chromium.org/1822823002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/ui/accessibility_focus_ring_controller.cc:8: #include
<algorithm>
On 2016/03/22 04:34:27, xiyuan wrote:
> nit: alpha sort the headers?

Actually I think this is right - Chrome's style is to put C
standard headers first, then C++ standard headers,
then project headers. See base/cpu.cc or url/gurl.cc for
some examples.

[dmazzoni, 2016-03-22 20:23:19]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-03-22 20:23:20]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/1822823002/#ps60001
(title: "Landing without views_delegate change for now")

[commit-bot: I haz the power, 2016-03-22 20:23:39]

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

[commit-bot: I haz the power, 2016-03-22 21:51:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-22 21:52:54]

Description was changed from

==========
Implement Chrome OS accessibility features to highlight focus, caret & cursor.

This extends the existing code that handles drawing a focus ring in an overlay
layer for ChromeVox and for the keyboard-driven OOBE, and implements support
for standalone features to highlight the focused object, the mouse cursor,
and the text caret.

The preferences to enable highlighting focus, caret, and cursor are behind
a flag, so the details of the implementation may be tweaked before this ships.
Currently each one has a distinct color, and the cursor highlight fades out
when the cursor isn't moving.

BUG=314889
==========

to

==========
Implement Chrome OS accessibility features to highlight focus, caret & cursor.

This extends the existing code that handles drawing a focus ring in an overlay
layer for ChromeVox and for the keyboard-driven OOBE, and implements support
for standalone features to highlight the focused object, the mouse cursor,
and the text caret.

The preferences to enable highlighting focus, caret, and cursor are behind
a flag, so the details of the implementation may be tweaked before this ships.
Currently each one has a distinct color, and the cursor highlight fades out
when the cursor isn't moving.

BUG=314889

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

[commit-bot: I haz the power, 2016-03-22 21:52:55]

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