[ash-md] Wired in the Shelf color to be derived from the Wallpaper.

[ash-md] Wired in the Shelf color to be derived from the Wallpaper.

This CL adds a flag, which is disabled by default, that will color the shelf
based on a color extracted from the current wallpaper.

BUG=595010
TEST=ash_unittest, components_unittests

Review-Url: https://codereview.chromium.org/2679133003
Cr-Commit-Position: refs/heads/master@{#453101}
Committed: https://chromium.googlesource.com/chromium/src/+/33189d2b610025a3f8595e3cf6cc1347ad6a9d41

[bruthig, 2017-02-14 21:52:15]

Description was changed from

==========
[ash-md] WIP Wired in the Shelf color to be derived from the Wallpaper.

BUG=595010
==========

to

==========
[ash-md] WIP Wired in the Shelf color to be derived from the Wallpaper.

Depends on: https://codereview.chromium.org/2690513002/

BUG=595010
==========

[bruthig, 2017-02-22 22:48:39]

The CQ bit was checked by bruthig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 22:49:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bruthig, 2017-02-22 23:08:28]

The CQ bit was checked by bruthig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 23:09:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 23:53:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 23:53:13]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[bruthig, 2017-02-23 22:37:00]

Description was changed from

==========
[ash-md] WIP Wired in the Shelf color to be derived from the Wallpaper.

Depends on: https://codereview.chromium.org/2690513002/

BUG=595010
==========

to

==========
[ash-md] Wired in the Shelf color to be derived from the Wallpaper.

This CL adds a flag, which is disabled by default, that will color the shelf
based on a color extracted from the current wallpaper.

BUG=595010
TEST=ash_unittest, components_unittests
==========

[bruthig, 2017-02-23 22:40:49]

The CQ bit was checked by bruthig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 22:41:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bruthig, 2017-02-23 22:45:17]

bruthig@chromium.org changed reviewers:
+ bshe@chromium.org, sky@chromium.org

[bruthig, 2017-02-23 22:45:18]

bshe@, can you please review changes in:
- ash/common/wallpaper*
- components/wallpaper/*

sky@, can you please review changes in:
- Everything else

[sky, 2017-02-24 00:56:32]

sky@chromium.org changed reviewers:
+ msw@chromium.org

[sky, 2017-02-24 00:56:32]

+msw to review the shelf changes.

[commit-bot: I haz the power, 2017-02-24 02:25:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 02:25:09]

Dry run: This issue passed the CQ dry run.

[msw, 2017-02-24 02:28:12]

lgtm with a few nits

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/app_l...
File ash/common/shelf/app_list_button.h (right):

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/app_l...
ash/common/shelf/app_list_button.h:77: // Color used to paint the background;
nit: period

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/overf...
File ash/common/shelf/overflow_button.h (right):

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/overf...
ash/common/shelf/overflow_button.h:69: // Color used to paint the background;
nit: period

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/shelf...
File ash/common/shelf/shelf_background_animator.h (right):

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/shelf...
ash/common/shelf/shelf_background_animator.h:48: static const int kMaxAlpha =
255;
optional nit: use or inline SK_AlphaOPAQUE or 0xFF?

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/shelf...
ash/common/shelf/shelf_background_animator.h:110: void UpdateCurrentColor(double
t);
optional nit: Color -> Value for consistency with other function names.

[sky, 2017-02-24 04:13:51]

chrome LGTM (I'm rubber stamping the shelf changes as msw reviewed them)

[bruthig, 2017-02-24 09:04:30]

bruthig@chromium.org changed reviewers:
+ mpearson@chromium.org

[bruthig, 2017-02-24 09:04:31]

Mark, can you review histograms.xml?

[Mark P, 2017-02-24 17:42:08]

tentative histograms.xml lgtm assuming if you fix the one issue

https://codereview.chromium.org/2679133003/diff/140001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2679133003/diff/140001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:2146: +  <summary>The time taken to
extract colors from wallpapers.</summary>
Please state when this is recorded.
https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histo...

[bshe, 2017-02-24 18:30:36]

wallpaper lgtm with nits

https://codereview.chromium.org/2679133003/diff/100001/ash/common/wallpaper/w...
File ash/common/wallpaper/wallpaper_controller.cc (right):

https://codereview.chromium.org/2679133003/diff/100001/ash/common/wallpaper/w...
ash/common/wallpaper/wallpaper_controller.cc:44: ash::switches::kAshShelfColor))
{
nit: ash:: namespace is not necessary for this.

https://codereview.chromium.org/2679133003/diff/100001/ash/common/wallpaper/w...
ash/common/wallpaper/wallpaper_controller.cc:53: ash::switches::kAshShelfColor);
ditto

https://codereview.chromium.org/2679133003/diff/100001/components/wallpaper/w...
File components/wallpaper/wallpaper_color_calculator.h (right):

https://codereview.chromium.org/2679133003/diff/100001/components/wallpaper/w...
components/wallpaper/wallpaper_color_calculator.h:5: #ifndef
COMPONENTS_WALLPAPER_COLOR_CALCULATOR_H_
nit: should this be COMPONENTS_WALLPAPER_WALLPAPER_COLOR_CALCULATOR_H_?

[bruthig, 2017-02-25 17:51:18]

Thanks all, sending to CQ.

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/app_l...
File ash/common/shelf/app_list_button.h (right):

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/app_l...
ash/common/shelf/app_list_button.h:77: // Color used to paint the background;
On 2017/02/24 02:28:12, msw wrote:
> nit: period

Woopsie

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/overf...
File ash/common/shelf/overflow_button.h (right):

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/overf...
ash/common/shelf/overflow_button.h:69: // Color used to paint the background;
On 2017/02/24 02:28:12, msw wrote:
> nit: period

D'oh

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/shelf...
File ash/common/shelf/shelf_background_animator.h (right):

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/shelf...
ash/common/shelf/shelf_background_animator.h:48: static const int kMaxAlpha =
255;
On 2017/02/24 02:28:12, msw wrote:
> optional nit: use or inline SK_AlphaOPAQUE or 0xFF?

Awesome, thx!

https://codereview.chromium.org/2679133003/diff/100001/ash/common/shelf/shelf...
ash/common/shelf/shelf_background_animator.h:110: void UpdateCurrentColor(double
t);
On 2017/02/24 02:28:12, msw wrote:
> optional nit: Color -> Value for consistency with other function names.

Done.

https://codereview.chromium.org/2679133003/diff/100001/ash/common/wallpaper/w...
File ash/common/wallpaper/wallpaper_controller.cc (right):

https://codereview.chromium.org/2679133003/diff/100001/ash/common/wallpaper/w...
ash/common/wallpaper/wallpaper_controller.cc:44: ash::switches::kAshShelfColor))
{
On 2017/02/24 18:30:36, bshe wrote:
> nit: ash:: namespace is not necessary for this.

Done.

https://codereview.chromium.org/2679133003/diff/100001/ash/common/wallpaper/w...
ash/common/wallpaper/wallpaper_controller.cc:53: ash::switches::kAshShelfColor);
On 2017/02/24 18:30:36, bshe wrote:
> ditto

Done.

https://codereview.chromium.org/2679133003/diff/100001/components/wallpaper/w...
File components/wallpaper/wallpaper_color_calculator.h (right):

https://codereview.chromium.org/2679133003/diff/100001/components/wallpaper/w...
components/wallpaper/wallpaper_color_calculator.h:5: #ifndef
COMPONENTS_WALLPAPER_COLOR_CALCULATOR_H_
On 2017/02/24 18:30:36, bshe wrote:
> nit: should this be COMPONENTS_WALLPAPER_WALLPAPER_COLOR_CALCULATOR_H_?

Done.

https://codereview.chromium.org/2679133003/diff/140001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2679133003/diff/140001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:2146: +  <summary>The time taken to
extract colors from wallpapers.</summary>
On 2017/02/24 17:42:08, Mark P wrote:
> Please state when this is recorded.
>
https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histo...

Done.

[bruthig, 2017-02-25 17:51:43]

The CQ bit was checked by bruthig@chromium.org

[bruthig, 2017-02-25 17:51:43]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org,
sky@chromium.org, mpearson@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2679133003/#ps160001
(title: "Addressed nits.")

[commit-bot: I haz the power, 2017-02-25 17:51:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-25 18:54:25]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1488045103419990,
"parent_rev": "f4eb4359fec12b0cd91dca5b0c8a9857bd55ef7c", "commit_rev":
"33189d2b610025a3f8595e3cf6cc1347ad6a9d41"}

[commit-bot: I haz the power, 2017-02-25 18:55:05]

Description was changed from

==========
[ash-md] Wired in the Shelf color to be derived from the Wallpaper.

This CL adds a flag, which is disabled by default, that will color the shelf
based on a color extracted from the current wallpaper.

BUG=595010
TEST=ash_unittest, components_unittests
==========

to

==========
[ash-md] Wired in the Shelf color to be derived from the Wallpaper.

This CL adds a flag, which is disabled by default, that will color the shelf
based on a color extracted from the current wallpaper.

BUG=595010
TEST=ash_unittest, components_unittests

Review-Url: https://codereview.chromium.org/2679133003
Cr-Commit-Position: refs/heads/master@{#453101}
Committed:
https://chromium.googlesource.com/chromium/src/+/33189d2b610025a3f8595e3cf6cc...
==========

[commit-bot: I haz the power, 2017-02-25 18:55:06]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/33189d2b610025a3f8595e3cf6cc...

[sketch420stoner, 2017-03-22 15:27:08]

sketch420stoner@gmail.com changed reviewers:
+ sketch420stoner@gmail.com

[sketch420stoner, 2017-03-22 15:27:09]

lgtm

[sketch420stoner, 2017-03-22 16:37:44]

lgtm

[sketch420stoner, 2017-03-22 16:37:46]

lgtm

lgtm