mash: Preliminary support for shelf auto-hide

mash: Preliminary support for shelf auto-hide

Shelf now collapses to light-bar when auto-hide is enabled and shows itself on
mouse hover.

* Observe ShelfLayoutManager for state changes and send the preferred size to
  the window manager via a window property.
* Use mouse hover/enter/exit over the shelf widget and status area widget to
  trigger shelf show.

This depends on a patch to shutdown the layout manager before the status area
widget is destroyed to avoid processing ET_MOUSE_EXITED events mid-shutdown.

TODO: Animations of widget position and opacity
TODO: Update the user window work area when auto-hide enabled
TODO: Track visible and maximized windows for special-case shelf opacity and
      visibility behaviors

BUG=613212
TEST=existing ash_unittests

Committed: https://crrev.com/84d491374cd8d3eef55deb09117a11b0a70b813c
Cr-Commit-Position: refs/heads/master@{#396048}

[James Cook, 2016-05-24 16:27:05]

Description was changed from

==========
WIP: mash: Preliminary support for shelf auto-hide

BUG=
==========

to

==========
mash: Preliminary support for shelf auto-hide

Shelf now collapses to light-bar when auto-hide is enabled and shows itself on
mouse hover.

* Observe ShelfLayoutManager for state changes and send the preferred size to
  the window manager via a window property.
* Use mouse hover/enter/exit over the shelf widget and status area widget to
  trigger shelf show.

This depends on a patch to shutdown the layout manager before the status area
widget is destroyed to avoid processing ET_MOUSE_EXITED events mid-shutdown.

TODO: Animations of widget position and opacity
TODO: Update the user window work area when auto-hide enabled
TODO: Track visible and maximized windows for special-case shelf opacity and
      visibility behaviors

BUG=613212
TEST=existing ash_unittests
==========

[James Cook, 2016-05-24 16:35:01]

Description was changed from

==========
mash: Preliminary support for shelf auto-hide

Shelf now collapses to light-bar when auto-hide is enabled and shows itself on
mouse hover.

* Observe ShelfLayoutManager for state changes and send the preferred size to
  the window manager via a window property.
* Use mouse hover/enter/exit over the shelf widget and status area widget to
  trigger shelf show.

This depends on a patch to shutdown the layout manager before the status area
widget is destroyed to avoid processing ET_MOUSE_EXITED events mid-shutdown.

TODO: Animations of widget position and opacity
TODO: Update the user window work area when auto-hide enabled
TODO: Track visible and maximized windows for special-case shelf opacity and
      visibility behaviors

BUG=613212
TEST=existing ash_unittests
==========

to

==========
mash: Preliminary support for shelf auto-hide

Shelf now collapses to light-bar when auto-hide is enabled and shows itself on
mouse hover.

* Observe ShelfLayoutManager for state changes and send the preferred size to
  the window manager via a window property.
* Use mouse hover/enter/exit over the shelf widget and status area widget to
  trigger shelf show.

This depends on a patch to shutdown the layout manager before the status area
widget is destroyed to avoid processing ET_MOUSE_EXITED events mid-shutdown.

TODO: Animations of widget position and opacity
TODO: Update the user window work area when auto-hide enabled
TODO: Track visible and maximized windows for special-case shelf opacity and
      visibility behaviors

BUG=613212
TEST=existing ash_unittests
==========

[James Cook, 2016-05-24 16:35:27]

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

[James Cook, 2016-05-24 16:35:27]

msw, please take a look.

[msw, 2016-05-24 20:46:55]

https://codereview.chromium.org/2007003002/diff/40001/ash/mus/shelf_delegate_...
File ash/mus/shelf_delegate_mus.h (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/mus/shelf_delegate_...
ash/mus/shelf_delegate_mus.h:47: // ShelfLayoutManagerObserver:
I wonder if this should be done through ShelfDelegate instead?
(that might reduce our reliance upon the ash ShelfLayoutManager)

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_button.cc
File ash/shelf/shelf_button.cc (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_button....
ash/shelf/shelf_button.cc:136: // Draws a bar underneath the button to represent
the state of the application.
nit: bar will soon be dot for MD, maybe omit this comment for now?

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:385: // exit) may be generate during status
area widget teardown.
nit: generated

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:386: if (visibility_state() != SHELF_AUTO_HIDE
|| in_shutdown_)
q: Where is |in_shutdown_| declared and set?

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:963: if (Shell::GetInstance()->in_mus())
q: Do we actually need this early return? What happens if you fall through?

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.h (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.h:132: // from an EventHandler. In mash these
come from events that hit the shelf
Could we always forward shelf/status events and remove the EventHandler?
(ShelfLayoutManager could track in_mouse_drag_ itself, right?)

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager_observer.h (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager_observer.h:31: virtual void
DidChangeVisibilityState(ShelfLayoutManager* layout_manager,
optional nit: pass the ash::Shelf instead? ditto below.

[James Cook, 2016-05-25 00:25:43]

msw, please take another look. (Apologies for the rebase, but it's only onto the
previous CL in the dependency chain.)

https://codereview.chromium.org/2007003002/diff/40001/ash/mus/shelf_delegate_...
File ash/mus/shelf_delegate_mus.h (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/mus/shelf_delegate_...
ash/mus/shelf_delegate_mus.h:47: // ShelfLayoutManagerObserver:
On 2016/05/24 20:46:55, msw wrote:
> I wonder if this should be done through ShelfDelegate instead?
> (that might reduce our reliance upon the ash ShelfLayoutManager)

Done.

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_button.cc
File ash/shelf/shelf_button.cc (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_button....
ash/shelf/shelf_button.cc:136: // Draws a bar underneath the button to represent
the state of the application.
On 2016/05/24 20:46:55, msw wrote:
> nit: bar will soon be dot for MD, maybe omit this comment for now?

Done.

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:385: // exit) may be generate during status
area widget teardown.
On 2016/05/24 20:46:55, msw wrote:
> nit: generated

Whoops! Done.

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:386: if (visibility_state() != SHELF_AUTO_HIDE
|| in_shutdown_)
On 2016/05/24 20:46:55, msw wrote:
> q: Where is |in_shutdown_| declared and set?

Declared in the header, set in PrepareForShutdown(). It was added in the earlier
CL in the dependency chain.

However, as part of review feedback there I took it out and re-added it in this
CL, so the diff should be a little clearer after this latest rebase.

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:963: if (Shell::GetInstance()->in_mus())
On 2016/05/24 20:46:55, msw wrote:
> q: Do we actually need this early return? What happens if you fall through?

Good catch, we don't really need this.

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.h (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.h:132: // from an EventHandler. In mash these
come from events that hit the shelf
On 2016/05/24 20:46:55, msw wrote:
> Could we always forward shelf/status events and remove the EventHandler?
> (ShelfLayoutManager could track in_mouse_drag_ itself, right?)

I'd like to do that, but I don't think I can do it yet. I had it that way
initially, but I think in_mouse_drag_ is set for drags that start/end off the
shelf (from app list?). I'm afraid I'm going to break some behavior when I
change it. I can investigate further in a later CL, and I've got a TODO at
AutoHideEventFilter for that.

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager_observer.h (right):

https://codereview.chromium.org/2007003002/diff/40001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager_observer.h:31: virtual void
DidChangeVisibilityState(ShelfLayoutManager* layout_manager,
On 2016/05/24 20:46:55, msw wrote:
> optional nit: pass the ash::Shelf instead? ditto below.

I moved this method to the delegate and removed the ShelfLayoutManager parameter
below.

[msw, 2016-05-25 01:10:34]

lgtm with minor nits

https://codereview.chromium.org/2007003002/diff/100001/ash/shelf/shelf_delega...
File ash/shelf/shelf_delegate.h (right):

https://codereview.chromium.org/2007003002/diff/100001/ash/shelf/shelf_delega...
ash/shelf/shelf_delegate.h:34: // Called after |shelf|'s auto-hide state
changes.
nit: s/after/when/ to be consistent. 'On*changed' somewhat indicates timing.

https://codereview.chromium.org/2007003002/diff/100001/ash/shelf/shelf_layout...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2007003002/diff/100001/ash/shelf/shelf_layout...
ash/shelf/shelf_layout_manager.cc:390: event->type() == ui::ET_MOUSE_EXITED)
nit: curlies?

[James Cook, 2016-05-25 16:24:26]

https://codereview.chromium.org/2007003002/diff/100001/ash/shelf/shelf_delega...
File ash/shelf/shelf_delegate.h (right):

https://codereview.chromium.org/2007003002/diff/100001/ash/shelf/shelf_delega...
ash/shelf/shelf_delegate.h:34: // Called after |shelf|'s auto-hide state
changes.
On 2016/05/25 01:10:33, msw wrote:
> nit: s/after/when/ to be consistent. 'On*changed' somewhat indicates timing.

Done, although frankly I would prefer all the comments to be specific about
before/during/after timing, because that would help readers learn our
OnFooChanging vs. OnFooChanged convention. But that's for another CL.

https://codereview.chromium.org/2007003002/diff/100001/ash/shelf/shelf_layout...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2007003002/diff/100001/ash/shelf/shelf_layout...
ash/shelf/shelf_layout_manager.cc:390: event->type() == ui::ET_MOUSE_EXITED)
On 2016/05/25 01:10:34, msw wrote:
> nit: curlies?

Done.

[James Cook, 2016-05-25 16:25:05]

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

[James Cook, 2016-05-25 16:25:06]

sky, can I get OWNERS?

[sky, 2016-05-25 19:29:36]

LGTM

[James Cook, 2016-05-25 21:56:45]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-05-25 21:56:46]

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

[commit-bot: I haz the power, 2016-05-25 21:57:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2007003002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2007003002/160001

[commit-bot: I haz the power, 2016-05-26 00:00:50]

Description was changed from

==========
mash: Preliminary support for shelf auto-hide

Shelf now collapses to light-bar when auto-hide is enabled and shows itself on
mouse hover.

* Observe ShelfLayoutManager for state changes and send the preferred size to
  the window manager via a window property.
* Use mouse hover/enter/exit over the shelf widget and status area widget to
  trigger shelf show.

This depends on a patch to shutdown the layout manager before the status area
widget is destroyed to avoid processing ET_MOUSE_EXITED events mid-shutdown.

TODO: Animations of widget position and opacity
TODO: Update the user window work area when auto-hide enabled
TODO: Track visible and maximized windows for special-case shelf opacity and
      visibility behaviors

BUG=613212
TEST=existing ash_unittests
==========

to

==========
mash: Preliminary support for shelf auto-hide

Shelf now collapses to light-bar when auto-hide is enabled and shows itself on
mouse hover.

* Observe ShelfLayoutManager for state changes and send the preferred size to
  the window manager via a window property.
* Use mouse hover/enter/exit over the shelf widget and status area widget to
  trigger shelf show.

This depends on a patch to shutdown the layout manager before the status area
widget is destroyed to avoid processing ET_MOUSE_EXITED events mid-shutdown.

TODO: Animations of widget position and opacity
TODO: Update the user window work area when auto-hide enabled
TODO: Track visible and maximized windows for special-case shelf opacity and
      visibility behaviors

BUG=613212
TEST=existing ash_unittests
==========

[commit-bot: I haz the power, 2016-05-26 00:00:52]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-05-26 00:02:20]

Description was changed from

==========
mash: Preliminary support for shelf auto-hide

Shelf now collapses to light-bar when auto-hide is enabled and shows itself on
mouse hover.

* Observe ShelfLayoutManager for state changes and send the preferred size to
  the window manager via a window property.
* Use mouse hover/enter/exit over the shelf widget and status area widget to
  trigger shelf show.

This depends on a patch to shutdown the layout manager before the status area
widget is destroyed to avoid processing ET_MOUSE_EXITED events mid-shutdown.

TODO: Animations of widget position and opacity
TODO: Update the user window work area when auto-hide enabled
TODO: Track visible and maximized windows for special-case shelf opacity and
      visibility behaviors

BUG=613212
TEST=existing ash_unittests
==========

to

==========
mash: Preliminary support for shelf auto-hide

Shelf now collapses to light-bar when auto-hide is enabled and shows itself on
mouse hover.

* Observe ShelfLayoutManager for state changes and send the preferred size to
  the window manager via a window property.
* Use mouse hover/enter/exit over the shelf widget and status area widget to
  trigger shelf show.

This depends on a patch to shutdown the layout manager before the status area
widget is destroyed to avoid processing ET_MOUSE_EXITED events mid-shutdown.

TODO: Animations of widget position and opacity
TODO: Update the user window work area when auto-hide enabled
TODO: Track visible and maximized windows for special-case shelf opacity and
      visibility behaviors

BUG=613212
TEST=existing ash_unittests

Committed: https://crrev.com/84d491374cd8d3eef55deb09117a11b0a70b813c
Cr-Commit-Position: refs/heads/master@{#396048}
==========

[commit-bot: I haz the power, 2016-05-26 00:02:21]

Patchset 9 (id:??) landed as
https://crrev.com/84d491374cd8d3eef55deb09117a11b0a70b813c
Cr-Commit-Position: refs/heads/master@{#396048}