mash: Fix shelf overflow bubble not closing on click outside its bounds

mash: Fix shelf overflow bubble not closing on click outside its bounds

* Add screen location to views::PointerWatcher notifications.
* Convert OverflowBubble from a global ui::EventHandler to a PointerWatcher,
which allows it to work on mus.

Add/remove the PointerWatcher in the constructor/destructor which ensures
it is properly cleaned up when the widget is destroyed (unlike the previous
pre-target handler).

BUG=595851, 608508
TEST=existing ash_unittests for overflow bubble. For mash, shrink the window
to show overflow arrow, click to spawn it, click outside bubble to close it.

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

[James Cook, 2016-04-30 21:08:15]

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

[James Cook, 2016-04-30 21:08:16]

msw, please take a look

[msw, 2016-05-02 17:18:13]

https://codereview.chromium.org/1934123004/diff/1/ash/shelf/overflow_bubble.cc
File ash/shelf/overflow_bubble.cc (right):

https://codereview.chromium.org/1934123004/diff/1/ash/shelf/overflow_bubble.c...
ash/shelf/overflow_bubble.cc:70:
aura::client::GetScreenPositionClient(root)->ConvertPointToScreen(
Will this work? What is the coordinate system used for event.root_location()? If
it's in the space of some other root window, then using the bubble's root won't
work for getting the actual screen coordinate, right?

[James Cook, 2016-05-02 17:48:35]

https://codereview.chromium.org/1934123004/diff/1/ash/shelf/overflow_bubble.cc
File ash/shelf/overflow_bubble.cc (right):

https://codereview.chromium.org/1934123004/diff/1/ash/shelf/overflow_bubble.c...
ash/shelf/overflow_bubble.cc:70:
aura::client::GetScreenPositionClient(root)->ConvertPointToScreen(
On 2016/05/02 17:18:13, msw wrote:
> Will this work? What is the coordinate system used for event.root_location()?
If
> it's in the space of some other root window, then using the bubble's root
won't
> work for getting the actual screen coordinate, right?

Yes, I think this is wrong for multi-display. Good catch.

I need to look at this some more. I'll ping again when I have a solution.

I would hold off on your patch to close tooltips on touch until I sort out the
coordinate systems with PointerWatcher.

[James Cook, 2016-05-03 16:35:44]

Description was changed from

==========
mash: Fix shelf overflow bubble not closing on click outside its bounds

Convert it from a global ui::EventHandler to a views:PointerWatcher,
which allows it to work on mus.

BUG=595851
TEST=Run mash, shrink window to show overflow arrow, click to spawn it,
click outside bubble to close it. Also works with classic ash.
==========

to

==========
mash: Fix shelf overflow bubble not closing on click outside its bounds

* Add screen location to views::PointerWatcher notifications.
* Convert OverflowBubble from a global ui::EventHandler to a PointerWatcher,
which allows it to work on mus.

Add/remove the PointerWatcher in the constructor/destructor which ensures
it is properly cleaned up when the widget is destroyed (unlike the previous
pre-target handler).

BUG=595851
TEST=existing ash_unittests for overflow bubble. For mash, shrink the window
to show overflow arrow, click to spawn it, click outside bubble to close it.
==========

[James Cook, 2016-05-03 16:37:05]

Description was changed from

==========
mash: Fix shelf overflow bubble not closing on click outside its bounds

* Add screen location to views::PointerWatcher notifications.
* Convert OverflowBubble from a global ui::EventHandler to a PointerWatcher,
which allows it to work on mus.

Add/remove the PointerWatcher in the constructor/destructor which ensures
it is properly cleaned up when the widget is destroyed (unlike the previous
pre-target handler).

BUG=595851
TEST=existing ash_unittests for overflow bubble. For mash, shrink the window
to show overflow arrow, click to spawn it, click outside bubble to close it.
==========

to

==========
mash: Fix shelf overflow bubble not closing on click outside its bounds

* Add screen location to views::PointerWatcher notifications.
* Convert OverflowBubble from a global ui::EventHandler to a PointerWatcher,
which allows it to work on mus.

Add/remove the PointerWatcher in the constructor/destructor which ensures
it is properly cleaned up when the widget is destroyed (unlike the previous
pre-target handler).

BUG=595851,608508
TEST=existing ash_unittests for overflow bubble. For mash, shrink the window
to show overflow arrow, click to spawn it, click outside bubble to close it.
==========

[James Cook, 2016-05-03 16:37:42]

msw, please take another look. I added screen coordinates to the PointerWatcher
notifications, which also made the system tray bubble code slightly simpler.

[msw, 2016-05-03 17:26:47]

lgtm with a nit

https://codereview.chromium.org/1934123004/diff/20001/ash/shelf/overflow_bubb...
File ash/shelf/overflow_bubble.cc (right):

https://codereview.chromium.org/1934123004/diff/20001/ash/shelf/overflow_bubb...
ash/shelf/overflow_bubble.cc:66: const ui::LocatedEvent& event,
nit: |event| is no longer needed here.

[James Cook, 2016-05-03 17:35:53]

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

[James Cook, 2016-05-03 17:35:53]

sky, can I get OWNERS?

https://codereview.chromium.org/1934123004/diff/20001/ash/shelf/overflow_bubb...
File ash/shelf/overflow_bubble.cc (right):

https://codereview.chromium.org/1934123004/diff/20001/ash/shelf/overflow_bubb...
ash/shelf/overflow_bubble.cc:66: const ui::LocatedEvent& event,
On 2016/05/03 17:26:47, msw wrote:
> nit: |event| is no longer needed here.

Done.

[sky, 2016-05-03 18:01:08]

LGTM

[James Cook, 2016-05-03 18:49:48]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-05-03 18:49:49]

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/1934123004/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-03 18:50:08]

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

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

Description was changed from

==========
mash: Fix shelf overflow bubble not closing on click outside its bounds

* Add screen location to views::PointerWatcher notifications.
* Convert OverflowBubble from a global ui::EventHandler to a PointerWatcher,
which allows it to work on mus.

Add/remove the PointerWatcher in the constructor/destructor which ensures
it is properly cleaned up when the widget is destroyed (unlike the previous
pre-target handler).

BUG=595851,608508
TEST=existing ash_unittests for overflow bubble. For mash, shrink the window
to show overflow arrow, click to spawn it, click outside bubble to close it.
==========

to

==========
mash: Fix shelf overflow bubble not closing on click outside its bounds

* Add screen location to views::PointerWatcher notifications.
* Convert OverflowBubble from a global ui::EventHandler to a PointerWatcher,
which allows it to work on mus.

Add/remove the PointerWatcher in the constructor/destructor which ensures
it is properly cleaned up when the widget is destroyed (unlike the previous
pre-target handler).

BUG=595851,608508
TEST=existing ash_unittests for overflow bubble. For mash, shrink the window
to show overflow arrow, click to spawn it, click outside bubble to close it.
==========

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

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-03 20:01:37]

Description was changed from

==========
mash: Fix shelf overflow bubble not closing on click outside its bounds

* Add screen location to views::PointerWatcher notifications.
* Convert OverflowBubble from a global ui::EventHandler to a PointerWatcher,
which allows it to work on mus.

Add/remove the PointerWatcher in the constructor/destructor which ensures
it is properly cleaned up when the widget is destroyed (unlike the previous
pre-target handler).

BUG=595851,608508
TEST=existing ash_unittests for overflow bubble. For mash, shrink the window
to show overflow arrow, click to spawn it, click outside bubble to close it.
==========

to

==========
mash: Fix shelf overflow bubble not closing on click outside its bounds

* Add screen location to views::PointerWatcher notifications.
* Convert OverflowBubble from a global ui::EventHandler to a PointerWatcher,
which allows it to work on mus.

Add/remove the PointerWatcher in the constructor/destructor which ensures
it is properly cleaned up when the widget is destroyed (unlike the previous
pre-target handler).

BUG=595851,608508
TEST=existing ash_unittests for overflow bubble. For mash, shrink the window
to show overflow arrow, click to spawn it, click outside bubble to close it.

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

[commit-bot: I haz the power, 2016-05-03 20:01:38]

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