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

Issue 2779243002: [ash-md] Enable Ash shelf coloring by default. (Closed)

Created:
3 years, 8 months ago by bruthig
Modified:
3 years, 8 months ago
Reviewers:
James Cook, bshe
CC:
chromium-reviews, kalyank, sadrul, asvitkine+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[ash-md] Enable Ash shelf coloring by default. - Changes the --ash-shelf-color switch to enable/disable the feature - Introduces new --ash-shelf-color-scheme switch to define the color extraction scheme - Uses the 'Dark Muted' --ash-shelf-color-scheme by default. TBR=bshe@chromium.org BUG=595010 TEST=ash_unittests, components_unittests Review-Url: https://codereview.chromium.org/2779243002 Cr-Commit-Position: refs/heads/master@{#461183} Committed: https://chromium.googlesource.com/chromium/src/+/d8b95960a8f2bec0e0eb48e636ee57204804bad9

Patch Set 1 #

Patch Set 2 : Added CHECKs() to debug trybot failures. #

Patch Set 3 : Attempted to fix crashes. #

Patch Set 4 : Added logging. #

Patch Set 5 : Observed SessionController. #

Total comments: 9

Patch Set 6 : Addressed patch set 5 comments. #

Total comments: 8

Patch Set 7 : Fixed histograms.xml and nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+229 lines, -74 lines) Patch
M ash/common/ash_switches.h View 1 2 3 4 5 1 chunk +9 lines, -6 lines 0 comments Download
M ash/common/ash_switches.cc View 1 2 3 4 5 1 chunk +11 lines, -6 lines 0 comments Download
M ash/common/shelf/shelf_background_animator.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M ash/common/wallpaper/wallpaper_controller.h View 1 2 3 4 5 6 chunks +21 lines, -9 lines 0 comments Download
M ash/common/wallpaper/wallpaper_controller.cc View 1 2 3 4 5 6 7 chunks +59 lines, -19 lines 0 comments Download
M ash/common/wallpaper/wallpaper_controller_unittest.cc View 1 2 3 4 5 6 5 chunks +63 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 2 chunks +28 lines, -12 lines 0 comments Download
M chrome/browser/flag_descriptions.h View 1 2 3 4 5 1 chunk +20 lines, -12 lines 0 comments Download
M chrome/browser/flag_descriptions.cc View 1 2 3 4 5 1 chunk +15 lines, -8 lines 0 comments Download
M components/wallpaper/wallpaper_color_calculator.h View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M components/wallpaper/wallpaper_color_calculator.cc View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 52 (37 generated)
bruthig
James, can you PTAL?
3 years, 8 months ago (2017-03-30 00:31:16 UTC) #16
James Cook
Please update the CL description. It wasn't clear to me that this enables the feature ...
3 years, 8 months ago (2017-03-30 14:31:47 UTC) #19
James Cook
https://codereview.chromium.org/2779243002/diff/80001/ash/common/wallpaper/wallpaper_controller.cc File ash/common/wallpaper/wallpaper_controller.cc (right): https://codereview.chromium.org/2779243002/diff/80001/ash/common/wallpaper/wallpaper_controller.cc#newcode57 ash/common/wallpaper/wallpaper_controller.cc:57: if (switch_value.find("black") != std::string::npos) On 2017/03/30 14:31:47, James Cook ...
3 years, 8 months ago (2017-03-30 14:38:53 UTC) #20
bruthig
James, can you take another look? > Please update the CL description. Done. > Also, ...
3 years, 8 months ago (2017-03-31 00:48:34 UTC) #25
James Cook
Code looks good, but a question about how many times we run the calculation. https://codereview.chromium.org/2779243002/diff/100001/ash/common/wallpaper/wallpaper_controller.cc ...
3 years, 8 months ago (2017-03-31 05:08:51 UTC) #28
bruthig
James, can you take another look? Biao, I've TBR'ed you on this simple include change. ...
3 years, 8 months ago (2017-03-31 17:38:51 UTC) #33
James Cook
LGTM. As we discussed I'd like to see some numbers from ARM devices and a ...
3 years, 8 months ago (2017-03-31 17:56:18 UTC) #34
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/2779243002/120001
3 years, 8 months ago (2017-03-31 18:25:47 UTC) #37
commit-bot: I haz the power
CQ experienced an internal error when committing your CL and the maintainers were notified. Sorry ...
3 years, 8 months ago (2017-03-31 18:48:33 UTC) #39
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/2779243002/120001
3 years, 8 months ago (2017-03-31 18:49:47 UTC) #41
commit-bot: I haz the power
CQ experienced an internal error when committing your CL and the maintainers were notified. Sorry ...
3 years, 8 months ago (2017-03-31 18:59:06 UTC) #43
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/2779243002/120001
3 years, 8 months ago (2017-03-31 19:00:09 UTC) #45
commit-bot: I haz the power
CQ experienced an internal error when committing your CL and the maintainers were notified. Sorry ...
3 years, 8 months ago (2017-03-31 19:05:36 UTC) #47
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/2779243002/120001
3 years, 8 months ago (2017-03-31 19:23:38 UTC) #49
commit-bot: I haz the power
3 years, 8 months ago (2017-03-31 19:39:40 UTC) #52
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/d8b95960a8f2bec0e0eb48e636ee...

Powered by Google App Engine
This is Rietveld 408576698