Add spinner UI to show bluetooth is discovering devices.

Add spinner UI to show bluetooth is discovering devices. For M26, I used the default image from Throbber. We may polish the UI with better asset in M27.

Add an interactive_ flag in Throbber to allow it become non-interactive so that the tooltip will show up when user hover the mouse on the Throbber who is the child view of ThrobberView class.


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

[stevenjb, 2013-02-05 01:25:48]

You'll need someone to review ui/views changes.

I won't insist on this now, but tray_views.h has gotten out of hand. Each of
those classes should really be in their own file, probably under
ash/system/tray/views/. I'll leave it up to you whether or not to extract this
one now, or create a cleanup task to break this file up.

Otherwise, lgtm

[jennyz, 2013-02-05 01:39:08]

On 2013/02/05 01:25:48, stevenjb (chromium) wrote:
> You'll need someone to review ui/views changes.
> 
> I won't insist on this now, but tray_views.h has gotten out of hand. Each of
> those classes should really be in their own file, probably under
> ash/system/tray/views/. I'll leave it up to you whether or not to extract this
> one now, or create a cleanup task to break this file up.
> 
> Otherwise, lgtm

Yes, I agree with you we should clean up the tray_views.h/cc to make each class
their own files. I filed the bug for M27: crbug.com/174228.

Thanks for suggestion!

[sadrul, 2013-02-05 02:31:41]

LGTM

+sky@ to see if he has opinions on HitTestRect() vs HasHitTestMask().

https://codereview.chromium.org/12180027/diff/1/ash/system/tray/tray_views.h
File ash/system/tray/tray_views.h (right):

https://codereview.chromium.org/12180027/diff/1/ash/system/tray/tray_views.h#...
ash/system/tray/tray_views.h:282: virtual gfx::Size GetPreferredSize() OVERRIDE;
Overrides usually go after the non-overrides.

https://codereview.chromium.org/12180027/diff/1/ui/views/controls/throbber.h
File ui/views/controls/throbber.h (right):

https://codereview.chromium.org/12180027/diff/1/ui/views/controls/throbber.h#...
ui/views/controls/throbber.h:44: virtual bool HitTestRect(const gfx::Rect& rect)
const OVERRIDE;
You can do this by overriding HasHitTestMask too. I seem to remember that is the
preferred way. But I don't remember if that is really the case.

[sky, 2013-02-05 15:58:52]

https://codereview.chromium.org/12180027/diff/1/ui/views/controls/throbber.cc
File ui/views/controls/throbber.cc (right):

https://codereview.chromium.org/12180027/diff/1/ui/views/controls/throbber.cc...
ui/views/controls/throbber.cc:93: return interactive_ ? View::HitTestRect(rect)
: false;
Why do you need this? Rather than adding set_interactive can't you use
set_enabled?

[jennyz, 2013-02-05 17:37:20]

https://codereview.chromium.org/12180027/diff/1/ash/system/tray/tray_views.h
File ash/system/tray/tray_views.h (right):

https://codereview.chromium.org/12180027/diff/1/ash/system/tray/tray_views.h#...
ash/system/tray/tray_views.h:282: virtual gfx::Size GetPreferredSize() OVERRIDE;
On 2013/02/05 02:31:42, sadrul wrote:
> Overrides usually go after the non-overrides.

Done.

https://codereview.chromium.org/12180027/diff/1/ui/views/controls/throbber.cc
File ui/views/controls/throbber.cc (right):

https://codereview.chromium.org/12180027/diff/1/ui/views/controls/throbber.cc...
ui/views/controls/throbber.cc:93: return interactive_ ? View::HitTestRect(rect)
: false;
On 2013/02/05 15:58:52, sky wrote:
> Why do you need this? Rather than adding set_interactive can't you use
> set_enabled?
Yes, you are right, I can just use set_enabled, removed set_interactive. Thanks!

[jennyz, 2013-02-05 19:06:08]

sky@, I removed the change in Throbber and added a subclass of SmoothedThrobber
which overrides GetTooltipText to show tooltip on the throbber, as we discussed,
thanks a lot for your suggestion, this is much more cleaner, PTAL.

[sky, 2013-02-05 20:52:33]

Thanks! Now that you removed all the views changes you no longer need me to
review:)

[commit-bot: I haz the power, 2013-02-05 21:16:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennyz@chromium.org/12180027/16001