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

Issue 2824703006: Add MetalayerMode to the palette. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 3 days ago by Vladislav Kaznacheev
Modified:
1 week, 1 day ago
CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, sadrul, yusukes+watch_chromium.org, viettrungluu+watch_chromium.org, hidehiko+watch_chromium.org, oshima+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, lhchavez+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

Add MetalayerMode to the palette. This tool shows/hides the "metalayer" overlay on the containter side. The tool name string is temporary, as well as the palette icons (reusing the icons from "Capture region"). Still keeping around the temporary Shift-Search-A shortcut and the repurposed "Capture region" tool. BUG=b:37165189 TEST="Metalayer mode" stylus tool toggles the metalayer. Review-Url: https://codereview.chromium.org/2824703006 Cr-Commit-Position: refs/heads/master@{#466167} Committed: https://chromium.googlesource.com/chromium/src/+/466e48c8e7f8fbfc3ba625000881094dd864a8da

Patch Set 1 #

Total comments: 20

Patch Set 2 : Addressed comments #

Total comments: 16

Patch Set 3 : Addressed comments #

Total comments: 2

Patch Set 4 : Fixed copyright year #

Unified diffs Side-by-side diffs Delta from patch set Stats (+259 lines, -9 lines) Patch
M ash/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M ash/ash_strings.grd View 1 chunk +3 lines, -0 lines 0 comments Download
M ash/palette_delegate.h View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
M ash/shell/shell_delegate_impl.cc View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M ash/system/palette/palette_ids.h View 2 chunks +2 lines, -0 lines 0 comments Download
M ash/system/palette/palette_ids.cc View 2 chunks +4 lines, -0 lines 0 comments Download
M ash/system/palette/palette_tool.cc View 2 chunks +2 lines, -0 lines 0 comments Download
A ash/system/palette/tools/metalayer_mode.h View 1 2 3 1 chunk +42 lines, -0 lines 0 comments Download
A ash/system/palette/tools/metalayer_mode.cc View 1 2 3 1 chunk +65 lines, -0 lines 0 comments Download
M ash/test/test_palette_delegate.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M ash/test/test_palette_delegate.cc View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.h View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc View 1 2 4 chunks +60 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/palette_delegate_chromeos.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/ash/palette_delegate_chromeos.cc View 1 2 2 chunks +32 lines, -2 lines 0 comments Download
M components/arc/common/voice_interaction_framework.mojom View 1 2 3 chunks +8 lines, -5 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 26 (10 generated)
Luis Héctor Chávez
https://codereview.chromium.org/2824703006/diff/1/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/2824703006/diff/1/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc#newcode82 chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:82: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); if (!metalayer_closed_callback_.is_null()) base::ResetAndReturn(&metalayer_closed_callback_).Run(); instance_ready_ = false; // if ...
1 week, 2 days ago (2017-04-19 15:48:29 UTC) #3
Vladislav Kaznacheev
PTAL https://codereview.chromium.org/2824703006/diff/1/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/2824703006/diff/1/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc#newcode82 chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:82: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); On 2017/04/19 15:48:28, Luis Héctor Chávez wrote: ...
1 week, 2 days ago (2017-04-19 23:42:59 UTC) #4
estark
Sorry, what am I supposed to be reviewing in this CL?
1 week, 2 days ago (2017-04-20 00:06:45 UTC) #5
Vladislav Kaznacheev
On 2017/04/20 00:06:45, estark wrote: > Sorry, what am I supposed to be reviewing in ...
1 week, 2 days ago (2017-04-20 00:53:01 UTC) #6
Luis Héctor Chávez
https://codereview.chromium.org/2824703006/diff/1/components/arc/common/voice_interaction_framework.mojom File components/arc/common/voice_interaction_framework.mojom (right): https://codereview.chromium.org/2824703006/diff/1/components/arc/common/voice_interaction_framework.mojom#newcode22 components/arc/common/voice_interaction_framework.mojom:22: [MinVersion=2]OnMetalayerComplete@2(); On 2017/04/19 23:42:59, Vladislav Kaznacheev wrote: > On ...
1 week, 1 day ago (2017-04-20 16:03:26 UTC) #7
Vladislav Kaznacheev
PTAL https://codereview.chromium.org/2824703006/diff/1/components/arc/common/voice_interaction_framework.mojom File components/arc/common/voice_interaction_framework.mojom (right): https://codereview.chromium.org/2824703006/diff/1/components/arc/common/voice_interaction_framework.mojom#newcode22 components/arc/common/voice_interaction_framework.mojom:22: [MinVersion=2]OnMetalayerComplete@2(); On 2017/04/20 16:03:26, Luis Héctor Chávez wrote: ...
1 week, 1 day ago (2017-04-20 17:54:56 UTC) #8
Luis Héctor Chávez
lgtm thanks!
1 week, 1 day ago (2017-04-20 18:17:54 UTC) #9
xiaohuic
lgtm
1 week, 1 day ago (2017-04-20 18:54:04 UTC) #11
oshima
lgtm https://codereview.chromium.org/2824703006/diff/40001/ash/system/palette/tools/metalayer_mode.h File ash/system/palette/tools/metalayer_mode.h (right): https://codereview.chromium.org/2824703006/diff/40001/ash/system/palette/tools/metalayer_mode.h#newcode1 ash/system/palette/tools/metalayer_mode.h:1: // Copyright 2016 The Chromium Authors. All rights ...
1 week, 1 day ago (2017-04-20 19:56:58 UTC) #12
estark
mojom changes lgtm
1 week, 1 day ago (2017-04-20 20:07:32 UTC) #13
estark
mojom changes lgtm
1 week, 1 day ago (2017-04-20 20:07:33 UTC) #14
Vladislav Kaznacheev
https://codereview.chromium.org/2824703006/diff/40001/ash/system/palette/tools/metalayer_mode.h File ash/system/palette/tools/metalayer_mode.h (right): https://codereview.chromium.org/2824703006/diff/40001/ash/system/palette/tools/metalayer_mode.h#newcode1 ash/system/palette/tools/metalayer_mode.h:1: // Copyright 2016 The Chromium Authors. All rights reserved. ...
1 week, 1 day ago (2017-04-20 20:09:55 UTC) #15
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/2824703006/60001
1 week, 1 day ago (2017-04-20 20:10:56 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/276708)
1 week, 1 day ago (2017-04-20 21:25:36 UTC) #20
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/2824703006/60001
1 week, 1 day ago (2017-04-20 21:39:47 UTC) #22
commit-bot: I haz the power
1 week, 1 day ago (2017-04-20 22:35:37 UTC) #25
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/466e48c8e7f8fbfc3ba625000881...
Sign in to reply to this message.

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