Fixing invalid window opacity/visibility combination with the shelf

Fixing invalid window opacity/visibility combination with the shelf

BUG=351553
TEST=existing unit tests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270293

[Mr4D (OOO till 08-26), 2014-05-13 20:00:23]

Please have a look!

[sky, 2014-05-13 21:43:04]

https://codereview.chromium.org/280383009/diff/1/ash/shelf/shelf_layout_manag...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/280383009/diff/1/ash/shelf/shelf_layout_manag...
ash/shelf/shelf_layout_manager.cc:730: if (visible &&
!target_bounds.status_opacity)
Do you really care about the current visibility?

https://codereview.chromium.org/280383009/diff/1/ui/aura/window.cc
File ui/aura/window.cc (right):

https://codereview.chromium.org/280383009/diff/1/ui/aura/window.cc#newcode341
ui/aura/window.cc:341: DCHECK(!(visible_ && layer()->GetTargetOpacity() ==
0.0f));
I think you should land this separately. That way if you have to revert you're
only reverting this part of the change.

[Mr4D (OOO till 08-26), 2014-05-13 22:08:16]

Addressed. Please have another look!

https://codereview.chromium.org/280383009/diff/1/ash/shelf/shelf_layout_manag...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/280383009/diff/1/ash/shelf/shelf_layout_manag...
ash/shelf/shelf_layout_manager.cc:730: if (visible &&
!target_bounds.status_opacity)
Was only trying to avoid duplicate calls. But since window::SetVisible(..) is
bailing early on no change this should be fine. Done.

https://codereview.chromium.org/280383009/diff/1/ui/aura/window.cc
File ui/aura/window.cc (right):

https://codereview.chromium.org/280383009/diff/1/ui/aura/window.cc#newcode341
ui/aura/window.cc:341: DCHECK(!(visible_ && layer()->GetTargetOpacity() ==
0.0f));
Indeed! Thanks, good idea! Done.

[sky, 2014-05-13 22:16:18]

LGTM

[Mr4D (OOO till 08-26), 2014-05-13 22:17:03]

The CQ bit was checked by skuhne@chromium.org

[commit-bot: I haz the power, 2014-05-13 22:18:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skuhne@chromium.org/280383009/20001

[commit-bot: I haz the power, 2014-05-14 01:57:36]

Change committed as 270293