Chromium Code Reviews
Help | Chromium Project | Sign in
(8)

Issue 2803403002: Support region selection for voice interaction session (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks, 5 days ago by Vladislav Kaznacheev
Modified:
2 weeks ago
CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, oshima+watch_chromium.org, yusukes+watch_chromium.org, viettrungluu+watch_chromium.org, hidehiko+watch_chromium.org, sadrul, abarth-chromium, Aaron Boodman, lhchavez+watch_chromium.org, yzshen+watch_chromium.org, victorhsieh+watch_chromium.org, kalyank, darin (slow to review), davemoore+watch_chromium.org, qsr+mojo_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Support region selection for voice interaction session Introducing two new ways to invoke a voice interaction session for a specific region: 1. If kEnableVoiceInteraction is enabled, the "Capture region" stylus tool does not take a screenshot but starts a voice interaction session for that region. 2. Shift+Search+A invokes the "metalayer" - an alternative region selection tool. BUG=b:37165189 TEST="Capture region" stylus tool start voice Interaction session. Shift+Search+A invokes the "metalayer". Review-Url: https://codereview.chromium.org/2803403002 Cr-Commit-Position: refs/heads/master@{#464070} Committed: https://chromium.googlesource.com/chromium/src/+/91b3f7e7b1f592052614ffd6eb61d60f87dd2571

Patch Set 1 #

Total comments: 1

Patch Set 2 : Removed 'auto' #

Total comments: 10

Patch Set 3 : Addressed comments #

Total comments: 6

Patch Set 4 : Addressed comments #

Patch Set 5 : Added comments re experimental mode #

Total comments: 2

Patch Set 6 : Addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+91 lines, -11 lines) Patch
M chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc View 1 2 3 4 2 chunks +21 lines, -6 lines 0 comments Download
M chrome/browser/ui/ash/palette_delegate_chromeos.h View 1 2 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/ui/ash/palette_delegate_chromeos.cc View 1 2 3 4 5 3 chunks +52 lines, -3 lines 0 comments Download
M components/arc/common/voice_interaction_framework.mojom View 1 2 3 2 chunks +11 lines, -2 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 41 (29 generated)
Vladislav Kaznacheev
2 weeks, 5 days ago (2017-04-07 23:15:03 UTC) #2
xc
lgtm https://codereview.chromium.org/2803403002/diff/1/chrome/browser/ui/ash/palette_delegate_chromeos.cc File chrome/browser/ui/ash/palette_delegate_chromeos.cc (right): https://codereview.chromium.org/2803403002/diff/1/chrome/browser/ui/ash/palette_delegate_chromeos.cc#newcode46 chrome/browser/ui/ash/palette_delegate_chromeos.cc:46: auto framework = I was asked to use ...
2 weeks, 2 days ago (2017-04-10 17:53:45 UTC) #3
Vladislav Kaznacheev
Please review: dcheng@ *.mojom lhchavez@ components/arc
2 weeks, 2 days ago (2017-04-10 22:54:29 UTC) #6
dcheng (OOO through May 2)
mojo lgtm (I can't help but wonder what voice interaction has to do with screenshots ...
2 weeks, 2 days ago (2017-04-11 07:39:47 UTC) #7
Luis Héctor Chávez
*/arc/* lgtm with nits. https://codereview.chromium.org/2803403002/diff/40001/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc File chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc (right): https://codereview.chromium.org/2803403002/diff/40001/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc#newcode84 chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:84: if (accelerator.modifiers() & ui::EF_SHIFT_DOWN) { ...
2 weeks, 1 day ago (2017-04-11 16:11:16 UTC) #8
Vladislav Kaznacheev
Please review: oshima@ chrome/browser/ui/ash/palette_delegate_chromeos.* https://codereview.chromium.org/2803403002/diff/40001/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc File chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc (right): https://codereview.chromium.org/2803403002/diff/40001/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc#newcode84 chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:84: if (accelerator.modifiers() & ui::EF_SHIFT_DOWN) { ...
2 weeks, 1 day ago (2017-04-11 18:59:33 UTC) #14
Luis Héctor Chávez
https://codereview.chromium.org/2803403002/diff/60001/chrome/browser/ui/ash/palette_delegate_chromeos.cc File chrome/browser/ui/ash/palette_delegate_chromeos.cc (right): https://codereview.chromium.org/2803403002/diff/60001/chrome/browser/ui/ash/palette_delegate_chromeos.cc#newcode44 chrome/browser/ui/ash/palette_delegate_chromeos.cc:44: if (!framework) { nit: you can elide braces https://codereview.chromium.org/2803403002/diff/60001/components/arc/common/voice_interaction_framework.mojom ...
2 weeks, 1 day ago (2017-04-11 20:02:04 UTC) #19
Vladislav Kaznacheev
https://codereview.chromium.org/2803403002/diff/60001/chrome/browser/ui/ash/palette_delegate_chromeos.cc File chrome/browser/ui/ash/palette_delegate_chromeos.cc (right): https://codereview.chromium.org/2803403002/diff/60001/chrome/browser/ui/ash/palette_delegate_chromeos.cc#newcode44 chrome/browser/ui/ash/palette_delegate_chromeos.cc:44: if (!framework) { On 2017/04/11 20:02:04, Luis Héctor Chávez ...
2 weeks, 1 day ago (2017-04-11 20:49:42 UTC) #20
oshima
lgtm with nits https://codereview.chromium.org/2803403002/diff/100001/chrome/browser/ui/ash/palette_delegate_chromeos.cc File chrome/browser/ui/ash/palette_delegate_chromeos.cc (right): https://codereview.chromium.org/2803403002/diff/100001/chrome/browser/ui/ash/palette_delegate_chromeos.cc#newcode34 chrome/browser/ui/ash/palette_delegate_chromeos.cc:34: private: nit: please define public ctor/dtor ...
2 weeks, 1 day ago (2017-04-12 01:09:22 UTC) #29
Vladislav Kaznacheev
https://codereview.chromium.org/2803403002/diff/100001/chrome/browser/ui/ash/palette_delegate_chromeos.cc File chrome/browser/ui/ash/palette_delegate_chromeos.cc (right): https://codereview.chromium.org/2803403002/diff/100001/chrome/browser/ui/ash/palette_delegate_chromeos.cc#newcode34 chrome/browser/ui/ash/palette_delegate_chromeos.cc:34: private: On 2017/04/12 01:09:22, oshima wrote: > nit: please ...
2 weeks ago (2017-04-12 16:08:47 UTC) #32
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/2803403002/120001
2 weeks ago (2017-04-12 16:39:47 UTC) #37
commit-bot: I haz the power
2 weeks ago (2017-04-12 17:40:48 UTC) #41
Message was sent while issue was closed.
Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/91b3f7e7b1f592052614ffd6eb61...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46