aura: Update how the tooltip manager works.

aura: Update how the tooltip manager works.

Currently, each widget's tooltip-manager listens to every single X event in the system, all the time. This can get expensive as the number of windows increase. Instead of doing this, the change here makes a TooltipManagerViews only listen to the mouse-events on the corresponding Widget. It starts its timer when the mouse enters the widget, and stops the timer when the mouse leaves the widget. So at a single time, only one TooltipManagerViews will be activated, and looking to update the its tooltip.

BUG=97249
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107809

[varunjain, 2011-10-28 16:09:23]

Looks much better.. thanks Sadrul!

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
File views/widget/native_widget_views.cc (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
views/widget/native_widget_views.cc:129: #if defined(TOUCH_UI) ||
defined(USE_AURA)
I am wondering if we really need a views tooltip manager. (Perhaps in a separate
CL) We can rename TMV to TMAura and remove it from everywhere else.

[sadrul, 2011-10-28 16:26:15]

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
File views/widget/native_widget_views.cc (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
views/widget/native_widget_views.cc:129: #if defined(TOUCH_UI) ||
defined(USE_AURA)
On 2011/10/28 16:09:23, varunjain wrote:
> I am wondering if we really need a views tooltip manager. (Perhaps in a
separate
> CL) We can rename TMV to TMAura and remove it from everywhere else.

Indeed. Right now, apart from aura, only TOUCH_UI uses the TMV. Once touch-ui
switches to aura, we can rename this.

[sky, 2011-10-28 17:47:27]

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_aura.cc
File views/widget/native_widget_aura.cc (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_aura...
views/widget/native_widget_aura.cc:502:
static_cast<TooltipManagerViews*>(tooltip_manager_.get());
Change the field to be of type TooltipManagerViews so you don't need to cast.

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
File views/widget/native_widget_views.cc (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
views/widget/native_widget_views.cc:131:
static_cast<TooltipManagerViews*>(GetTooltipManager());
Same comment about changing type of field to avoid cast.

http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
File views/widget/tooltip_manager_views.cc (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
views/widget/tooltip_manager_views.cc:92: if (event.type() ==
ui::ET_MOUSE_EXITED) {
nit: use a switch statement.

http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
File views/widget/tooltip_manager_views.h (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
views/widget/tooltip_manager_views.h:35: void UpdateForMouseEvent(const
MouseEvent& event);
Describe this, including where the MouseEvent comes from.

[sadrul, 2011-10-28 21:42:52]

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_aura.cc
File views/widget/native_widget_aura.cc (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_aura...
views/widget/native_widget_aura.cc:502:
static_cast<TooltipManagerViews*>(tooltip_manager_.get());
On 2011/10/28 17:47:27, sky wrote:
> Change the field to be of type TooltipManagerViews so you don't need to cast.

Done.

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
File views/widget/native_widget_views.cc (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
views/widget/native_widget_views.cc:131:
static_cast<TooltipManagerViews*>(GetTooltipManager());
On 2011/10/28 17:47:27, sky wrote:
> Same comment about changing type of field to avoid cast.

This is complicated here since NWViews doesn't actually create a TMViews. NWGtk
creates it instead, and NWV just uses it. Since this is code going away pretty
soon, this is probably OK? :)

http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
File views/widget/tooltip_manager_views.cc (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
views/widget/tooltip_manager_views.cc:92: if (event.type() ==
ui::ET_MOUSE_EXITED) {
On 2011/10/28 17:47:27, sky wrote:
> nit: use a switch statement.

Done.

http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
File views/widget/tooltip_manager_views.h (right):

http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
views/widget/tooltip_manager_views.h:35: void UpdateForMouseEvent(const
MouseEvent& event);
On 2011/10/28 17:47:27, sky wrote:
> Describe this, including where the MouseEvent comes from.

Done.

[sky, 2011-10-28 21:49:35]

Oy, LGTM

On Fri, Oct 28, 2011 at 2:42 PM,  <sadrul@chromium.org> wrote:
>
>
http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_aura.cc
> File views/widget/native_widget_aura.cc (right):
>
>
http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_aura...
> views/widget/native_widget_aura.cc:502:
> static_cast<TooltipManagerViews*>(tooltip_manager_.get());
> On 2011/10/28 17:47:27, sky wrote:
>>
>> Change the field to be of type TooltipManagerViews so you don't need
>
> to cast.
>
> Done.
>
>
http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
> File views/widget/native_widget_views.cc (right):
>
>
http://codereview.chromium.org/8417025/diff/8/views/widget/native_widget_view...
> views/widget/native_widget_views.cc:131:
> static_cast<TooltipManagerViews*>(GetTooltipManager());
> On 2011/10/28 17:47:27, sky wrote:
>>
>> Same comment about changing type of field to avoid cast.
>
> This is complicated here since NWViews doesn't actually create a
> TMViews. NWGtk creates it instead, and NWV just uses it. Since this is
> code going away pretty soon, this is probably OK? :)
>
>
http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
> File views/widget/tooltip_manager_views.cc (right):
>
>
http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
> views/widget/tooltip_manager_views.cc:92: if (event.type() ==
> ui::ET_MOUSE_EXITED) {
> On 2011/10/28 17:47:27, sky wrote:
>>
>> nit: use a switch statement.
>
> Done.
>
>
http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
> File views/widget/tooltip_manager_views.h (right):
>
>
http://codereview.chromium.org/8417025/diff/8/views/widget/tooltip_manager_vi...
> views/widget/tooltip_manager_views.h:35: void UpdateForMouseEvent(const
> MouseEvent& event);
> On 2011/10/28 17:47:27, sky wrote:
>>
>> Describe this, including where the MouseEvent comes from.
>
> Done.
>
> http://codereview.chromium.org/8417025/
>