Prevent a user from hiding the shelf via a gesture when there are no visible windows part 2

Prevent a user from hiding the shelf via a gesture when there are no visible windows part 2

- This CL combines the common parts of ShelfLayoutManager::LayoutShelf() and ShelfLayoutManager::SetState() into
ShelfLayoutManager::UpdateBoundsAndOpacity().
- This CL animates the shelf when a gesture drag is completed / canceled

BUG=268209
TEST=ShelfLayoutManagerTest.ShelfAnimatesWhenGestureComplete

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

[pkotwicz, 2013-08-09 02:56:03]

sadrul@, PTAL

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

https://codereview.chromium.org/22638013/diff/1/ash/shelf/shelf_layout_manage...
ash/shelf/shelf_layout_manager.cc:282: // other direction.
I do not call SetLauncherViewBounds() in UpateBoundsAndOpacity() because this
call is also used to set the initial LauncherView height (in the case of a
horizontally aligned shelf) when the launcher is created.

If you have suggestions as to how to discuss that in the comment above, I would
be more than grateful

[pkotwicz, 2013-08-09 03:08:37]

The main reason I wanted to unify LayoutShelf() and SetState() is that we use
the two methods are supposed to very similar.
For instance, https://chromiumcodereview.appspot.com/13994014 was written
because the two methods were not similar enough.

[sadrul, 2013-08-09 05:37:11]

I am going to let harrym@ do the first review.

Please add a test for the fix for the shelf animating at the end of a drag.

[Harry McCleave, 2013-08-09 18:27:30]

lgtm /w suggested test added.

[pkotwicz, 2013-08-10 18:51:55]

Added a test.
Also moved updating the dimmer widget's bounds to occur as a result of
ShelfWidget::DelegateView::OnBoundsChanged(). Otherwise, the dimmer widget's
bounds would not be updated during the animation following the completion of the
gesture drag.

[pkotwicz, 2013-08-12 18:18:16]

sadrul@, PTAL

[pkotwicz, 2013-08-13 05:34:29]

Ping!

[sadrul, 2013-08-13 17:21:34]

Please add an owner for reviewer for the next patchset.

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
ash/shelf/shelf_layout_manager.cc:1013: return SHELF_AUTO_HIDE_HIDDEN;
This parts are from another CL, right?

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
File ash/shelf/shelf_layout_manager_unittest.cc (right):

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
ash/shelf/shelf_layout_manager_unittest.cc:75: class ShelfAnimationWaiter :
views::WidgetObserver {
: public

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
ash/shelf/shelf_layout_manager_unittest.cc:77: ShelfAnimationWaiter(const
gfx::Rect& target_bounds)
explicit

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
ash/shelf/shelf_layout_manager_unittest.cc:1542:
EXPECT_TRUE(waiter.WasValidAnimation());
Can we also have a test that the animation happens correctly when:
 * The animation happens when dragging in from auto-hide-hidden to visible.
 * The animation happens correctly when dragging up/down while there are no
visible windows.

[pkotwicz, 2013-08-14 17:03:21]

James, PTAL

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
ash/shelf/shelf_layout_manager.cc:1013: return SHELF_AUTO_HIDE_HIDDEN;
That CL has now landed, rebasing is easier :)

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
File ash/shelf/shelf_layout_manager_unittest.cc (right):

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
ash/shelf/shelf_layout_manager_unittest.cc:77: ShelfAnimationWaiter(const
gfx::Rect& target_bounds)
On 2013/08/13 17:21:34, sadrul wrote:
> explicit

Done.

https://codereview.chromium.org/22638013/diff/17001/ash/shelf/shelf_layout_ma...
ash/shelf/shelf_layout_manager_unittest.cc:1542:
EXPECT_TRUE(waiter.WasValidAnimation());
On 2013/08/13 17:21:34, sadrul wrote:
> Can we also have a test that the animation happens correctly when:
>  * The animation happens when dragging in from auto-hide-hidden to visible.
>  * The animation happens correctly when dragging up/down while there are no
> visible windows.

Done.

[James Cook, 2013-08-14 20:41:45]

lgtm

[commit-bot: I haz the power, 2013-08-14 20:51:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/22638013/31001

[commit-bot: I haz the power, 2013-08-14 23:48:21]

Change committed as 217700