aura: brightness and volume bubble.

chrome/browser/chromeos/setting_level_bubble.cc

Index: chrome/browser/chromeos/setting_level_bubble.cc
diff --git a/chrome/browser/chromeos/setting_level_bubble.cc b/chrome/browser/chromeos/setting_level_bubble.cc
index c546f88455987beeed36cab168edef82d1719c95..1908ed1af877c8ed50a4df45b2f31ab8798cc8fd 100644
--- a/chrome/browser/chromeos/setting_level_bubble.cc
+++ b/chrome/browser/chromeos/setting_level_bubble.cc
@@ -14,8 +14,9 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/views/bubble/bubble.h"
 #include "ui/gfx/screen.h"
+#include "views/bubble/bubble_delegate.h"
+#include "views/layout/fill_layout.h"
 #include "views/widget/root_view.h"
 
 using base::TimeDelta;
@@ -84,91 +85,147 @@ static views::Widget* GetToplevelWidget() {
     return WebUILoginDisplay::GetLoginWindow();
 }
 
-SettingLevelBubble::SettingLevelBubble(SkBitmap* increase_icon,
-                                       SkBitmap* decrease_icon,
-                                       SkBitmap* disabled_icon)
-    : current_percent_(-1.0),
-      target_percent_(-1.0),
-      increase_icon_(increase_icon),
-      decrease_icon_(decrease_icon),
-      disabled_icon_(disabled_icon),
-      bubble_(NULL),
-      view_(NULL),
-      is_animating_(false) {
+// SettingLevelBubbleDelegateView ----------------------------------------------
+class SettingLevelBubbleDelegateView : public views::BubbleDelegateView {
+ public:
+  // BubbleDelegate overrides:
+  virtual gfx::Point GetAnchorPoint() OVERRIDE;
+
+  // Create the bubble delegate view.
+  explicit SettingLevelBubbleDelegateView(views::Widget* parent);
+  virtual ~SettingLevelBubbleDelegateView();
+
+  SettingLevelBubbleView* view() { return view_; }
+
+ protected:
+  // BubbleDelegate overrides:
+  virtual void Init() OVERRIDE;
+
+ private:
+  views::Widget* parent_;
+
+  SettingLevelBubbleView* view_;
+
+  DISALLOW_COPY_AND_ASSIGN(SettingLevelBubbleDelegateView);
+};
+
+gfx::Point SettingLevelBubbleDelegateView::GetAnchorPoint() {
+  gfx::Size view_size = GetPreferredSize();
+  // Calculate the position in screen coordinates that the bubble should
+  // "point" at (since we use BubbleBorder::FLOAT, this position actually
+  // specifies the center of the bubble).
+  gfx::Rect monitor_area =
+      gfx::Screen::GetMonitorAreaNearestWindow(
+          parent_->GetNativeView());
+  return (gfx::Point(
+      monitor_area.x() + kBubbleXRatio * monitor_area.width(),
+      monitor_area.bottom() - view_size.height() / 2 - kBubbleBottomGap));
 }
 
-SettingLevelBubble::~SettingLevelBubble() {}
+SettingLevelBubbleDelegateView::SettingLevelBubbleDelegateView(
+    views::Widget* parent)
+    : BubbleDelegateView(gfx::Point(),
+                         views::BubbleBorder::FLOAT,
+                         SK_ColorWHITE),
+      parent_(parent),
+      view_(NULL) {
+  set_close_on_esc(false);
+}
 
-void SettingLevelBubble::ShowBubble(double percent, bool enabled) {
-  const double old_target_percent = target_percent_;
-  UpdateTargetPercent(percent);
+SettingLevelBubbleDelegateView::~SettingLevelBubbleDelegateView() {
+  view_ = NULL;
+}
 
-  SkBitmap* icon = increase_icon_;
-  if (!enabled || target_percent_ == 0)
-    icon = disabled_icon_;
-  else if (old_target_percent >= 0 && target_percent_ < old_target_percent)
-    icon = decrease_icon_;
+void SettingLevelBubbleDelegateView::Init() {
+  SetLayoutManager(new views::FillLayout());
+  view_ = new SettingLevelBubbleView();

[Daniel Erat, 2011/10/26 05:08:01]

should probably call the SettingLevelBubbleView's Init() method here -- see
below

[alicet1, 2011/10/26 16:12:48]

yes, didn't do that for a reason -- the view's init needs current_icon, percent,
and enabled args. The delegate, after the rework, doesn't hold any of these
members, so I opt for calling Init() at where ShowBubble() calls it. let me see
if you think that's still ok.

+  AddChildView(view_);
+}
 
-  if (!bubble_) {
-    views::Widget* parent_widget = GetToplevelWidget();
-    if (parent_widget == NULL) {
-      LOG(WARNING) << "Unable to locate parent widget to display a bubble";
-      return;
-    }
-    DCHECK(view_ == NULL);
-    view_ = new SettingLevelBubbleView;
-    view_->Init(icon, current_percent_, enabled);
-
-    // Calculate the position in screen coordinates that the bubble should
-    // "point" at (since we use BubbleBorder::FLOAT, this position actually
-    // specifies the center of the bubble).
-    const gfx::Rect monitor_area =
-        gfx::Screen::GetMonitorAreaNearestWindow(
-            parent_widget->GetNativeView());
-    const gfx::Size view_size = view_->GetPreferredSize();
-    const gfx::Rect position_relative_to(
-        monitor_area.x() + kBubbleXRatio * monitor_area.width(),
-        monitor_area.bottom() - view_size.height() / 2 - kBubbleBottomGap,
-        0, 0);
-
-    bubble_ = Bubble::ShowFocusless(parent_widget,
-                                    position_relative_to,
-                                    views::BubbleBorder::FLOAT,
-                                    view_,  // contents
-                                    this,   // delegate
-                                    true);  // show while screen is locked
-    // TODO(derat): We probably shouldn't be using Bubble.  It'd be nice to call
-    // bubble_->set_fade_away_on_close(true) here, but then, if ShowBubble()
-    // gets called while the bubble is fading away, we end up just adjusting the
-    // value on the disappearing bubble; ideally we'd have a way to cancel the
-    // fade and show the bubble at full opacity for another
-    // kBubbleShowTimeoutMs.
-  } else {
-    DCHECK(view_);
-    hide_timer_.Stop();
-    view_->SetIcon(icon);
-    view_->SetEnabled(enabled);
+// SettingLevelBubble ----------------------------------------------
+void SettingLevelBubble::OnWidgetClosing(views::Widget* widget) {
+  if (view_ && view_->GetWidget() == widget) {
+    view_->GetWidget()->RemoveObserver(this);
+    view_= NULL;

[Daniel Erat, 2011/10/26 05:08:01]

nit: "view_ ="

[alicet1, 2011/10/26 16:12:48]

Done.

   }
+  // Update states.
+  current_percent_ = target_percent_;
+  target_time_ = TimeTicks();
+  last_animation_update_time_ = TimeTicks();
+  last_target_update_time_ = TimeTicks();
+  hide_timer_.Stop();
+  StopAnimation();
+}
 
+void SettingLevelBubble::ShowBubble(double percent, bool enabled) {
+  if (!view_)
+    view_ = CreateView();
+  UpdateWithoutShowingBubble(percent, enabled);
+  view_->GetWidget()->Show();
+  // Show the widget and start the timer. When the timer
+  // runs out, we start the fade sequence.
+  hide_timer_.Stop();
   hide_timer_.Start(FROM_HERE,
                     base::TimeDelta::FromMilliseconds(kBubbleShowTimeoutMs),
                     this, &SettingLevelBubble::OnHideTimeout);
 }
 
 void SettingLevelBubble::HideBubble() {
-  if (bubble_)
-    bubble_->Close();
+  if (view_) {
+    view_->GetWidget()->Close();
+    view_ = NULL;
+  }
+}
+
+SettingLevelBubble::SettingLevelBubble(SkBitmap* increase_icon,
+                                       SkBitmap* decrease_icon,
+                                       SkBitmap* disabled_icon)
+    :  current_percent_(-1.0),
+       target_percent_(-1.0),
+       increase_icon_(increase_icon),
+       decrease_icon_(decrease_icon),
+       disabled_icon_(disabled_icon),
+       is_animating_(false) {
+  view_ = CreateView();
+}
+
+SettingLevelBubble::~SettingLevelBubble() {
+  view_ = NULL;
+}
+
+SettingLevelBubbleView* SettingLevelBubble::CreateView() {
+  views::Widget* parent = GetToplevelWidget();
+  SettingLevelBubbleDelegateView* delegate =
+      new SettingLevelBubbleDelegateView(parent);
+  views::Widget* widget =
+      views::BubbleDelegateView::CreateBubble(delegate, parent);
+  widget->AddObserver(this);
+  // Hold on to the content view.
+  return delegate->view();
 }
 
 void SettingLevelBubble::UpdateWithoutShowingBubble(double percent,
                                                     bool enabled) {
+  const double old_target_percent = target_percent_;
   UpdateTargetPercent(percent);
-  if (view_)
-    view_->SetEnabled(enabled);
+  SkBitmap* current_icon = increase_icon_;
+  if (!enabled || target_percent_ == 0)
+    current_icon = disabled_icon_;
+  else if (old_target_percent >= 0 && target_percent_ < old_target_percent)
+    current_icon = decrease_icon_;
+  if (view_) {

[Daniel Erat, 2011/10/26 05:08:01]

nit: no curly braces for single-line if statements in chromium

[alicet1, 2011/10/26 16:12:48]

Done.

+    view_->Init(current_icon, percent, enabled);

[Daniel Erat, 2011/10/26 05:08:01]

i think that it's the case in all of the chrome code that i've seen that Init()
methods are intended to only be called once -- it's just a way to move more
heavyweight work (or things that can fail) outside of the constructor.  i'd
recommend sticking to that pattern here, and instead calling the view's
individual SetIcon/Level/Enabled methods.

[alicet1, 2011/10/26 16:12:48]

removed.

+  }
 }
 
 void SettingLevelBubble::OnHideTimeout() {
-  HideBubble();
+  // Start fading away.
+  if (view_) {
+    SettingLevelBubbleDelegateView* delegate =
+        static_cast<SettingLevelBubbleDelegateView*>
+        (view_->GetWidget()->widget_delegate());
+    delegate->StartFade(false);
+  }
 }
 
 void SettingLevelBubble::OnAnimationTimeout() {
@@ -194,26 +251,6 @@ void SettingLevelBubble::OnAnimationTimeout() {
     view_->SetLevel(current_percent_);
 }
 
-void SettingLevelBubble::BubbleClosing(Bubble* bubble, bool) {
-  DCHECK(bubble == bubble_);
-  hide_timer_.Stop();
-  StopAnimation();
-  bubble_ = NULL;
-  view_ = NULL;
-  current_percent_ = target_percent_;
-  target_time_ = TimeTicks();
-  last_animation_update_time_ = TimeTicks();
-  last_target_update_time_ = TimeTicks();
-}
-
-bool SettingLevelBubble::CloseOnEscape() {
-  return true;
-}
-
-bool SettingLevelBubble::FadeInOnShow() {
-  return false;
-}
-
 void SettingLevelBubble::UpdateTargetPercent(double percent) {
   target_percent_ = LimitPercent(percent);
   const TimeTicks now = TimeTicks::Now();