Create the Tooltip widget in Aura as needed instead of just creating it in the timer.

Create the Tooltip widget in Aura as needed instead of just creating it in the timer.

Currently the Tooltip widget is created in the tooltip timer which also handles setting the tooltip text on
the Tooltip object. There is a timing window between this and the mouse event coming in, which ends up causing
the tooltips to not show up if the underlying root changes in between.

Fixes bug https://code.google.com/p/chromium/issues/detail?id=292530

BUG=292530
R=sky@chromium.org, sky

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

[sky, 2013-09-16 22:46:51]

I don't think I understand the exact sequence that is causing things to go
wrong. Can you elaborate on it?

https://codereview.chromium.org/23958012/diff/8001/ui/views/corewm/tooltip_co...
File ui/views/corewm/tooltip_controller.cc (left):

https://codereview.chromium.org/23958012/diff/8001/ui/views/corewm/tooltip_co...
ui/views/corewm/tooltip_controller.cc:127: SetTooltipBounds(location, width,
height);
Won't this mean we potentially set the bounds only to recreate later on? And
similarly if you invoke GetWidgetForWindow later on and recreate the widget
won't the bounds be wrong?

https://codereview.chromium.org/23958012/diff/8001/ui/views/corewm/tooltip_co...
File ui/views/corewm/tooltip_controller.cc (right):

https://codereview.chromium.org/23958012/diff/8001/ui/views/corewm/tooltip_co...
ui/views/corewm/tooltip_controller.cc:533:
tooltip_->GetWidgetForWindow(tooltip_window_);
I know we discussed invoking GetWidgetForWindow, but now that I see it this
isn't the right place. Or rather TooltipWindow should expose a better API for
this.

[ananta, 2013-09-16 23:45:45]

https://codereview.chromium.org/23958012/diff/8001/ui/views/corewm/tooltip_co...
File ui/views/corewm/tooltip_controller.cc (left):

https://codereview.chromium.org/23958012/diff/8001/ui/views/corewm/tooltip_co...
ui/views/corewm/tooltip_controller.cc:127: SetTooltipBounds(location, width,
height);
On 2013/09/16 22:46:51, sky wrote:
> Won't this mean we potentially set the bounds only to recreate later on? And
> similarly if you invoke GetWidgetForWindow later on and recreate the widget
> won't the bounds be wrong?

As per our discussion, we now destroy the tooltip if the tooltip_window_ is
NULLed out.

https://codereview.chromium.org/23958012/diff/8001/ui/views/corewm/tooltip_co...
File ui/views/corewm/tooltip_controller.cc (right):

https://codereview.chromium.org/23958012/diff/8001/ui/views/corewm/tooltip_co...
ui/views/corewm/tooltip_controller.cc:533:
tooltip_->GetWidgetForWindow(tooltip_window_);
On 2013/09/16 22:46:51, sky wrote:
> I know we discussed invoking GetWidgetForWindow, but now that I see it this
> isn't the right place. Or rather TooltipWindow should expose a better API for
> this.

This is no longer needed.

[sky, 2013-09-16 23:58:56]

LGTM

[sky, 2013-09-16 23:59:11]

Also, if you could add test coverage that would be great!

[ananta, 2013-09-17 00:47:43]

On 2013/09/16 23:59:11, sky wrote:
> Also, if you could add test coverage that would be great!

I will do that in a separate CL tomorrow.

[ananta, 2013-09-17 00:48:53]

Committed patchset #2 manually as r223492 (presubmit successful).