Enable mash shelf tooltips.

Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG=595853
TEST=mash shelf shows tooltips; no cros regressions (except not closing on touch/gesture outside shelf).
R=sky@chromium.org

Committed: https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8
Cr-Commit-Position: refs/heads/master@{#382976}

[msw, 2016-03-23 06:31:52]

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

[commit-bot: I haz the power, 2016-03-23 06:32:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1816753002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1816753002/40001

[commit-bot: I haz the power, 2016-03-23 06:34:24]

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

[commit-bot: I haz the power, 2016-03-23 06:34:25]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[msw, 2016-03-23 06:52:25]

Description was changed from

==========
Enable mash shelf tooltips.

BUG=595853
==========

to

==========
Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG=595853
==========

[msw, 2016-03-23 06:59:18]

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

[commit-bot: I haz the power, 2016-03-23 06:59:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1816753002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1816753002/60001

[commit-bot: I haz the power, 2016-03-23 07:34:39]

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

[commit-bot: I haz the power, 2016-03-23 07:34:40]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[msw, 2016-03-23 16:38:10]

Description was changed from

==========
Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG=595853
==========

to

==========
Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG=595853
TEST=mash shelf shows tooltips; no cros regressions (except not closing on
touch/gesture outside shelf).
R=sky@chromium.org
==========

[msw, 2016-03-23 16:38:10]

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

[msw, 2016-03-23 16:39:18]

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

[commit-bot: I haz the power, 2016-03-23 16:39:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1816753002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1816753002/80001

[msw, 2016-03-23 16:39:38]

Hey Scott, please take a look; thanks!

[commit-bot: I haz the power, 2016-03-23 17:32:13]

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

[commit-bot: I haz the power, 2016-03-23 17:32:14]

Dry run: This issue passed the CQ dry run.

[sky, 2016-03-23 17:52:39]

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

[sky, 2016-03-23 17:53:03]

+jamescook in case he is interested in seeing these changes.

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/app_list_butt...
File ash/shelf/app_list_button.cc (left):

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/app_list_butt...
ash/shelf/app_list_button.cc:80: host_->MouseMovedOverButton(this);
How come it's safe to remove this and enter/exit? I see you removed the calls
here, but the functions still exist on ShelfView.

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_tooltip...
File ash/shelf/shelf_tooltip_manager.cc (right):

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_tooltip...
ash/shelf/shelf_tooltip_manager.cc:168: if (!event->IsKeyEvent())
Did you mean !IsMouseEvent here? If not, how come you close only on key?

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_tooltip...
ash/shelf/shelf_tooltip_manager.cc:192: void
ShelfTooltipManager::WillDeleteShelf() {
Should this be called from the destructor too? I'm nervous that you're assuming
the Shelf is always destroyed first.

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_view.cc
File ash/shelf/shelf_view.cc (right):

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_view.cc...
ash/shelf/shelf_view.cc:526: tooltip_bounds.Union(child->GetMirroredBounds());
On reviewing this code, it looks wrong. Consider if there are three items, 1, 2
and 3 (in that order). If 2 shouldn't show tooltips the union of 1 and 3
includes 2.

[msw, 2016-03-23 18:58:01]

Please take another look; thanks!

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/app_list_butt...
File ash/shelf/app_list_button.cc (left):

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/app_list_butt...
ash/shelf/app_list_button.cc:80: host_->MouseMovedOverButton(this);
On 2016/03/23 17:53:03, sky wrote:
> How come it's safe to remove this and enter/exit? I see you removed the calls
> here, but the functions still exist on ShelfView.

These do not exist still on ShelfView; am I missing something?
(MouseMovedOverButton, MouseEnteredButton, MouseExitedButton)
These are all replaced by ShelfTooltipManager::OnEvent.

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_tooltip...
File ash/shelf/shelf_tooltip_manager.cc (right):

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_tooltip...
ash/shelf/shelf_tooltip_manager.cc:168: if (!event->IsKeyEvent())
On 2016/03/23 17:53:03, sky wrote:
> Did you mean !IsMouseEvent here? If not, how come you close only on key?

This closes for non-mouse and non-kbd events (touch/gesture/etc.).
I think closing on key events would be fine, but there's a test against that.

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_tooltip...
ash/shelf/shelf_tooltip_manager.cc:192: void
ShelfTooltipManager::WillDeleteShelf() {
On 2016/03/23 17:53:03, sky wrote:
> Should this be called from the destructor too? I'm nervous that you're
assuming
> the Shelf is always destroyed first.

Done with some added checks that avoid test crashes. WillDeleteShelf is fired on
destruction of ShelfLayoutManager (the aura::LayoutManager for the shelf's
container). I want to move away from relying on ShelfLayoutManager here, but
it's needed (for now) to close tooltips on visibility/autohide changes (at least
for tests, likely not needed in practice).

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_view.cc
File ash/shelf/shelf_view.cc (right):

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/shelf_view.cc...
ash/shelf/shelf_view.cc:526: tooltip_bounds.Union(child->GetMirroredBounds());
On 2016/03/23 17:53:03, sky wrote:
> On reviewing this code, it looks wrong. Consider if there are three items, 1,
2
> and 3 (in that order). If 2 shouldn't show tooltips the union of 1 and 3
> includes 2.

That might be intentional; 2 itself wouldn't show a tooltip, but this allows 1
and 3 to continue showing theirs (and switching 1->3 immediately) when mousing
between them... it makes sense for a quick 1<->3 movements, but maybe not if you
linger over 2. I haven't altered this logic, but I'm happy to exclude bounds
around non-tooltip items (or pursue some better behavior).

IMO, all items (except open panels?) should always show tooltips, and buttons
should occupy the space we currently put between them; then this would be moot.

[sky, 2016-03-23 21:35:11]

LGTM

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/app_list_butt...
File ash/shelf/app_list_button.cc (left):

https://codereview.chromium.org/1816753002/diff/80001/ash/shelf/app_list_butt...
ash/shelf/app_list_button.cc:80: host_->MouseMovedOverButton(this);
On 2016/03/23 18:58:01, msw wrote:
> On 2016/03/23 17:53:03, sky wrote:
> > How come it's safe to remove this and enter/exit? I see you removed the
calls
> > here, but the functions still exist on ShelfView.
> 
> These do not exist still on ShelfView; am I missing something?
> (MouseMovedOverButton, MouseEnteredButton, MouseExitedButton)
> These are all replaced by ShelfTooltipManager::OnEvent.

My mistake. You clearly removed them.

[msw, 2016-03-23 23:05:33]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2016-03-23 23:05:59]

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

[commit-bot: I haz the power, 2016-03-23 23:48:58]

Description was changed from

==========
Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG=595853
TEST=mash shelf shows tooltips; no cros regressions (except not closing on
touch/gesture outside shelf).
R=sky@chromium.org
==========

to

==========
Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG=595853
TEST=mash shelf shows tooltips; no cros regressions (except not closing on
touch/gesture outside shelf).
R=sky@chromium.org
==========

[commit-bot: I haz the power, 2016-03-23 23:48:59]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-03-23 23:50:43]

Description was changed from

==========
Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG=595853
TEST=mash shelf shows tooltips; no cros regressions (except not closing on
touch/gesture outside shelf).
R=sky@chromium.org
==========

to

==========
Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG=595853
TEST=mash shelf shows tooltips; no cros regressions (except not closing on
touch/gesture outside shelf).
R=sky@chromium.org

Committed: https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8
Cr-Commit-Position: refs/heads/master@{#382976}
==========

[commit-bot: I haz the power, 2016-03-23 23:50:44]

Patchset 6 (id:??) landed as
https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8
Cr-Commit-Position: refs/heads/master@{#382976}