Update WM shadow transitions.

Update WM shadow transitions.

When changing from one shadow type to another, cross fade the two.

BUG=608852
Committed: https://crrev.com/77932377e59a528cfc1fa97b0ef91bafad8d2f86
Cr-Commit-Position: refs/heads/master@{#437956}

[Evan Stade, 2016-12-09 21:11:15]

estade@chromium.org changed reviewers:
+ jamescook@chromium.org

[Evan Stade, 2016-12-09 21:11:15]

This change is ready for review and doesn't depend on
https://codereview.chromium.org/2555373004/ but if you think it's easier to
handle reviews one at a time, feel free to postpone it.

[James Cook, 2016-12-09 23:56:43]

LGTM with nits. Nice patch.

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc
File ui/wm/core/shadow.cc (right):

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc#newcode98
ui/wm/core/shadow.cc:98: UpdateImagesForStyle();
Maybe this should be renamed to RecreateLayerForStyle() or similar to help
indicate it rebuilds shadow_layer_ ?

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc#newcode99
ui/wm/core/shadow.cc:99: shadow_layer_->SetOpacity(0.0);
nit: SetOpacity takes a float, I prefer 0.f or 0.0f. Ditto below.

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc#newcod...
ui/wm/core/shadow.cc:121: UpdateLayerBounds();
nit: might be nice to comment why this is necessary

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc#newcod...
ui/wm/core/shadow.cc:150: gfx::Rect new_layer_bounds = content_bounds_;
Thanks for renaming this. |new_layer_bounds| is clearer.

[Evan Stade, 2016-12-10 00:07:02]

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc
File ui/wm/core/shadow.cc (right):

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc#newcode98
ui/wm/core/shadow.cc:98: UpdateImagesForStyle();
On 2016/12/09 23:56:43, James Cook wrote:
> Maybe this should be renamed to RecreateLayerForStyle() or similar to help
> indicate it rebuilds shadow_layer_ ?

Done.

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc#newcode99
ui/wm/core/shadow.cc:99: shadow_layer_->SetOpacity(0.0);
On 2016/12/09 23:56:43, James Cook wrote:
> nit: SetOpacity takes a float, I prefer 0.f or 0.0f. Ditto below.

Done.

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc#newcod...
ui/wm/core/shadow.cc:121: UpdateLayerBounds();
On 2016/12/09 23:56:43, James Cook wrote:
> nit: might be nice to comment why this is necessary

Done.

https://codereview.chromium.org/2562143002/diff/1/ui/wm/core/shadow.cc#newcod...
ui/wm/core/shadow.cc:150: gfx::Rect new_layer_bounds = content_bounds_;
On 2016/12/09 23:56:43, James Cook wrote:
> Thanks for renaming this. |new_layer_bounds| is clearer.

pleasure is mine

[Evan Stade, 2016-12-10 00:08:23]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-12-10 00:08:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2562143002/#ps20001
(title: "review feedback")

[commit-bot: I haz the power, 2016-12-10 00:08:56]

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, 2016-12-10 02:06:48]

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

[commit-bot: I haz the power, 2016-12-10 02:06:49]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Evan Stade, 2016-12-12 16:19:15]

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

[Evan Stade, 2016-12-12 16:19:15]

+sky for owners

(should ui/wm have its own OWNERS file? perhaps it should mirror ash/OWNERS?)

[sky, 2016-12-12 16:28:19]

As James reviewed LGTM. As to whether ui/wm's owners should mirror that of ash.
The same set of people didn't necessarily work on the two. I'm certainly open to
more people being owners in ui/wm.

[Evan Stade, 2016-12-12 16:58:49]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2016-12-12 16:59:25]

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, 2016-12-12 23:31:48]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481561929291060,
"parent_rev": "8ddb06de03eded6cd6f2b5355babfd3c7922d259", "commit_rev":
"a2923491342e26f7059e4619daad571cef219be0"}

[commit-bot: I haz the power, 2016-12-12 23:34:52]

Description was changed from

==========
Update WM shadow transitions.

When changing from one shadow type to another, cross fade the two.

BUG=608852
==========

to

==========
Update WM shadow transitions.

When changing from one shadow type to another, cross fade the two.

BUG=608852

Committed: https://crrev.com/77932377e59a528cfc1fa97b0ef91bafad8d2f86
Cr-Commit-Position: refs/heads/master@{#437956}
==========

[commit-bot: I haz the power, 2016-12-12 23:34:53]

Patchset 2 (id:??) landed as
https://crrev.com/77932377e59a528cfc1fa97b0ef91bafad8d2f86
Cr-Commit-Position: refs/heads/master@{#437956}