first run bubble using the views/bubble api.

chrome/browser/ui/views/first_run_bubble.cc

Index: chrome/browser/ui/views/first_run_bubble.cc
diff --git a/chrome/browser/ui/views/first_run_bubble.cc b/chrome/browser/ui/views/first_run_bubble.cc
index 349257a66e8f143a00a279ded5b6de2ad5e62db1..38ae6ada773f18c6df26eae02892dad0adf24b2d 100644
--- a/chrome/browser/ui/views/first_run_bubble.cc
+++ b/chrome/browser/ui/views/first_run_bubble.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/ui/views/bubble/bubble.h"  // for kBackgroundColor

[msw, 2011/10/18 18:39:23]

really? this seems wrong, let's not mix bubbles. Assume the next step is
deleting those files; would we move kBackgroundColor here or into the new views
bubble delegate header as the default GetColor value, or as a seperate constant?

[alicet1, 2011/10/19 05:02:15]

I think you're handling this one already, so I'll leave it here. The constant
should move from old bubbles to views/bubbles.

 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/search_engines/util.h"
 #include "chrome/browser/ui/browser.h"
@@ -19,15 +20,17 @@
 #include "ui/base/l10n/l10n_font_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "views/bubble/bubble_view.h"
 #include "views/controls/button/image_button.h"
 #include "views/controls/button/text_button.h"
 #include "views/controls/label.h"
 #include "views/events/event.h"
 #include "views/focus/focus_manager.h"
 #include "views/layout/layout_constants.h"
+#if defined(OS_WIN)
 #include "views/widget/native_widget_win.h"

[msw, 2011/10/18 18:39:23]

Can we avoid this platform specific include altogether?

[alicet1, 2011/10/19 05:02:15]

I'll remove it when I manage to remove the two OS_WIN blocks below, not entirely
sure I can remove the EnableParent part.

+#endif
 #include "views/widget/widget.h"
-

[msw, 2011/10/18 18:39:23]

leave this line there.

[alicet1, 2011/10/19 05:02:15]

Done.

 namespace {
 
 // How much extra padding to put around our content over what the Bubble
@@ -38,13 +41,10 @@ const int kBubblePadding = 4;
 // provides in alternative OEM bubble.
 const int kOEMBubblePadding = 4;
 
-// Padding between parts of strings on the same line (for instance,
-// "New!" and "Search from the address bar!"
-const int kStringSeparationPadding = 2;
-
 // Margin around close button.
 const int kMarginRightOfCloseButton = 7;
 
+#if defined(OS_WIN)
 // The bubble's HWND is actually owned by the border widget, and it's the border
 // widget that's owned by the frame window the bubble is anchored to. This
 // function makes the two leaps necessary to go from the bubble contents HWND
@@ -53,6 +53,7 @@ HWND GetLogicalBubbleOwner(HWND bubble_hwnd) {
   HWND border_widget_hwnd = GetWindow(bubble_hwnd, GW_OWNER);
   return GetWindow(border_widget_hwnd, GW_OWNER);
 }
+#endif
 
 }  // namespace
 
@@ -62,9 +63,7 @@ class FirstRunBubbleViewBase : public views::View,
                                public views::ButtonListener,
                                public views::FocusChangeListener {
  public:
-  // Called by FirstRunBubble::Show to request focus for the proper button
-  // in the FirstRunBubbleView when it is shown.
-  virtual void BubbleShown() = 0;
+  virtual views::View* GetInitiallyFocusedView()  = 0;
 };
 
 // FirstRunBubbleView ---------------------------------------------------------
@@ -72,17 +71,14 @@ class FirstRunBubbleViewBase : public views::View,
 class FirstRunBubbleView : public FirstRunBubbleViewBase {
  public:
   FirstRunBubbleView(FirstRunBubble* bubble_window, Profile* profile);
-
+  virtual gfx::Size GetPreferredSize();
+  virtual views::View* GetInitiallyFocusedView() { return keep_button_; }
  private:
-  virtual ~FirstRunBubbleView() {}
-
-  // FirstRunBubbleViewBase:
-  virtual void BubbleShown();
+  virtual ~FirstRunBubbleView();
 
   // Overridden from View:
   virtual void ButtonPressed(views::Button* sender, const views::Event& event);
   virtual void Layout();
-  virtual gfx::Size GetPreferredSize();
 
   // FocusChangeListener:
   virtual void FocusWillChange(View* focused_before, View* focused_now);
@@ -91,8 +87,8 @@ class FirstRunBubbleView : public FirstRunBubbleViewBase {
   views::Label* label1_;
   views::Label* label2_;
   views::Label* label3_;
-  views::NativeTextButton* change_button_;
   views::NativeTextButton* keep_button_;
+  views::NativeTextButton* change_button_;
   Profile* profile_;
 
   DISALLOW_COPY_AND_ASSIGN(FirstRunBubbleView);
@@ -150,15 +146,15 @@ FirstRunBubbleView::FirstRunBubbleView(FirstRunBubble* bubble_window,
   AddChildView(change_button_);
 }
 
-void FirstRunBubbleView::BubbleShown() {
-  keep_button_->RequestFocus();
+FirstRunBubbleView::~FirstRunBubbleView() {
+  GetFocusManager()->RemoveFocusChangeListener(this);
 }
 
 void FirstRunBubbleView::ButtonPressed(views::Button* sender,
                                        const views::Event& event) {
   UserMetrics::RecordAction(UserMetricsAction("FirstRunBubbleView_Clicked"));
-  bubble_window_->set_fade_away_on_close(true);
-  bubble_window_->Close();
+  // Fades out.
+  GetWidget()->client_view()->AsBubbleView()->StartFade(false);
   if (change_button_ == sender) {
     UserMetrics::RecordAction(
                     UserMetricsAction("FirstRunBubbleView_ChangeButton"));
@@ -168,6 +164,7 @@ void FirstRunBubbleView::ButtonPressed(views::Button* sender,
       browser->OpenSearchEngineOptionsDialog();
     }
   }
+  GetWidget()->Close();
 }
 
 void FirstRunBubbleView::Layout() {
@@ -240,17 +237,16 @@ void FirstRunBubbleView::FocusWillChange(View* focused_before,
 class FirstRunOEMBubbleView : public FirstRunBubbleViewBase {
  public:
   FirstRunOEMBubbleView(FirstRunBubble* bubble_window, Profile* profile);
+  virtual gfx::Size GetPreferredSize();
+  // FirstRunBubbleViewBase:
+  virtual views::View* GetInitiallyFocusedView() { return this; }
 
  private:
-  virtual ~FirstRunOEMBubbleView() { }
-
-  // FirstRunBubbleViewBase:
-  virtual void BubbleShown();
+  virtual ~FirstRunOEMBubbleView();
 
   // Overridden from View:
   virtual void ButtonPressed(views::Button* sender, const views::Event& event);
   virtual void Layout();
-  virtual gfx::Size GetPreferredSize();
 
   // FocusChangeListener:
   virtual void FocusWillChange(View* focused_before, View* focused_now);
@@ -313,17 +309,16 @@ FirstRunOEMBubbleView::FirstRunOEMBubbleView(FirstRunBubble* bubble_window,
   AddChildView(close_button_);
 }
 
-void FirstRunOEMBubbleView::BubbleShown() {
-  RequestFocus();
-  // No button in oem_bubble to request focus.
+FirstRunOEMBubbleView::~FirstRunOEMBubbleView() {
+  GetFocusManager()->RemoveFocusChangeListener(this);
 }
 
 void FirstRunOEMBubbleView::ButtonPressed(views::Button* sender,
                                           const views::Event& event) {
   UserMetrics::RecordAction(
       UserMetricsAction("FirstRunOEMBubbleView_Clicked"));
-  bubble_window_->set_fade_away_on_close(true);
-  bubble_window_->Close();
+  GetWidget()->client_view()->AsBubbleView()->StartFade(/*fade_in=*/false);
+  GetWidget()->Close();
 }
 
 void FirstRunOEMBubbleView::Layout() {
@@ -366,7 +361,7 @@ gfx::Size FirstRunOEMBubbleView::GetPreferredSize() {
           IDS_FIRSTRUNOEMBUBBLE_DIALOG_WIDTH_CHARS, font),
       ui::GetLocalizedContentsHeightForFont(
           IDS_FIRSTRUNOEMBUBBLE_DIALOG_HEIGHT_LINES, font));
-
+#if defined(OS_WIN)
   // WARNING: HACK. Vista and XP calculate font size differently; this means
   // that a dialog box correctly proportioned for XP will appear too large in
   // Vista. The correct thing to do is to change font size calculations in
@@ -378,6 +373,7 @@ gfx::Size FirstRunOEMBubbleView::GetPreferredSize() {
     size.set_width(static_cast<int>(size.width() * 0.85));
     size.set_height(static_cast<int>(size.height() * 0.85));
   }
+#endif
   return size;
 }
 
@@ -392,18 +388,17 @@ void FirstRunOEMBubbleView::FocusWillChange(View* focused_before,
 class FirstRunMinimalBubbleView : public FirstRunBubbleViewBase {
  public:
   FirstRunMinimalBubbleView(FirstRunBubble* bubble_window, Profile* profile);
+  virtual gfx::Size GetPreferredSize();
+  // FirstRunBubbleViewBase:
+  virtual views::View* GetInitiallyFocusedView() { return this; }
 
  private:
-  virtual ~FirstRunMinimalBubbleView() { }
-
-  // FirstRunBubbleViewBase:
-  virtual void BubbleShown();
+  virtual ~FirstRunMinimalBubbleView();
 
   // Overridden from View:
   virtual void ButtonPressed(views::Button* sender,
                              const views::Event& event) { }
   virtual void Layout();
-  virtual gfx::Size GetPreferredSize();
 
   // FocusChangeListener:
   virtual void FocusWillChange(View* focused_before, View* focused_now);
@@ -446,8 +441,8 @@ FirstRunMinimalBubbleView::FirstRunMinimalBubbleView(
   AddChildView(label2_);
 }
 
-void FirstRunMinimalBubbleView::BubbleShown() {
-  RequestFocus();
+FirstRunMinimalBubbleView::~FirstRunMinimalBubbleView() {
+  GetFocusManager()->RemoveFocusChangeListener(this);
 }
 
 void FirstRunMinimalBubbleView::Layout() {
@@ -486,47 +481,78 @@ void FirstRunMinimalBubbleView::FocusWillChange(View* focused_before,
 // FirstRunBubble -------------------------------------------------------------
 
 // static
-FirstRunBubble* FirstRunBubble::Show(
+views::Widget* FirstRunBubble::Show(
     Profile* profile,
     views::Widget* parent,
     const gfx::Rect& position_relative_to,
     views::BubbleBorder::ArrowLocation arrow_location,
     FirstRun::BubbleType bubble_type) {
-  FirstRunBubble* bubble = new FirstRunBubble();
-  FirstRunBubbleViewBase* view = NULL;
+  FirstRunBubble* delegate =
+      new FirstRunBubble(profile,
+                         parent,
+                         position_relative_to,
+                         arrow_location,
+                         bubble_type);
+  views::Widget* widget =
+      views::BubbleDelegateView::CreateBubble(delegate, parent);
+  // StartFade and show.
+  widget->client_view()->AsBubbleView()->StartFade(/*fade_in=*/true);
+  return widget;
+}
 
-  switch (bubble_type) {
+void FirstRunBubble::Init() {
+  FirstRunBubbleViewBase* view = NULL;
+  switch (bubble_type_) {
     case FirstRun::OEM_BUBBLE:
-      view = new FirstRunOEMBubbleView(bubble, profile);
+      view = new FirstRunOEMBubbleView(this, profile_);
       break;
     case FirstRun::LARGE_BUBBLE:
-      view = new FirstRunBubbleView(bubble, profile);
+      view = new FirstRunBubbleView(this, profile_);
       break;
     case FirstRun::MINIMAL_BUBBLE:
-      view = new FirstRunMinimalBubbleView(bubble, profile);
+      view = new FirstRunMinimalBubbleView(this, profile_);
       break;
     default:
       NOTREACHED();
   }
-  bubble->set_view(view);
-  bubble->InitBubble(
-      parent, position_relative_to, arrow_location, view, bubble);
-  bubble->GetWidget()->GetFocusManager()->AddFocusChangeListener(view);
-  view->BubbleShown();

[msw, 2011/10/18 18:39:23]

Do you need to still call BubbleShown?

[alicet1, 2011/10/19 05:02:15]

re-added.

-  return bubble;
+  AddChildView(view);

[msw, 2011/10/18 18:39:23]

You might need to use a FillLayout to make the child view bounds match this
view's bounds.

[alicet1, 2011/10/19 05:02:15]

Done.

+  preferred_size_ = view->GetPreferredSize();
+  initially_focused_view_ = view->GetInitiallyFocusedView();
+  parent_->GetFocusManager()->AddFocusChangeListener(view);
 }
 
-FirstRunBubble::FirstRunBubble()
-    : has_been_activated_(false),
-      ALLOW_THIS_IN_INITIALIZER_LIST(enable_window_method_factory_(this)),
-      view_(NULL) {
+FirstRunBubble::FirstRunBubble(
+    Profile* profile,
+    views::Widget* parent,
+    const gfx::Rect& position_relative_to,
+    views::BubbleBorder::ArrowLocation arrow_location,
+    FirstRun::BubbleType bubble_type)
+    : profile_(profile),
+      parent_(parent),
+      position_relative_to_(position_relative_to),
+      arrow_location_(arrow_location),
+      bubble_type_(bubble_type),
+      initially_focused_view_(NULL),
+      has_been_activated_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(enable_window_method_factory_(this)) {
 }
 
 FirstRunBubble::~FirstRunBubble() {
   enable_window_method_factory_.InvalidateWeakPtrs();
-  GetWidget()->GetFocusManager()->RemoveFocusChangeListener(view_);
 }
 
+gfx::Point FirstRunBubble::GetAnchorPoint() const {
+  return position_relative_to_.origin();
+}
+
+gfx::Size FirstRunBubble::GetPreferredSize() {
+  return preferred_size_;

[msw, 2011/10/18 18:39:23]

preferred_size_ and this override are unnecessary as far as I can tell; remove
them. The preferred size calculations should work just fine as long as you add
FirstRunBubbleViewBase view to this view in Init().

[alicet1, 2011/10/19 05:02:15]

Done.

+}
+views::View* FirstRunBubble::GetInitiallyFocusedView() {

[msw, 2011/10/18 18:39:23]

insert a blank line between these function definitions.

[alicet1, 2011/10/19 05:02:15]

removed

+  return initially_focused_view_;
+}
+
+#if !defined(USE_AURA)

[msw, 2011/10/18 18:39:23]

You've got an OS_WIN block above for GetLogicalBubbleOwner, but !USE_AURA here,
this should be OS_WIN if it can't be eliminated altogether.

[alicet1, 2011/10/19 05:02:15]

Done.

 void FirstRunBubble::EnableParent() {

[msw, 2011/10/18 18:39:23]

This just seems wrong altogether... Can you safely eliminate this and
GetLogicalBubbleOwner, perhaps using Widget/FocusManager API instead?

[alicet1, 2011/10/19 05:02:15]

Got rid of the GetLogicalBubbleOwner, not sure if I can get rid of this method
all together.

   ::EnableWindow(GetParent(), true);
   // The EnableWindow() call above causes the parent to become active, which
@@ -542,6 +568,7 @@ void FirstRunBubble::EnableParent() {
   SetWindowPos(GetParent(), 0, 0, 0, 0,
                SWP_NOSIZE | SWP_NOMOVE | SWP_NOREDRAW | SWP_SHOWWINDOW);
 }
+#endif
 
 #if defined(OS_WIN) && !defined(USE_AURA)
 void FirstRunBubble::OnActivate(UINT action, BOOL minimized, HWND window) {
@@ -571,8 +598,10 @@ void FirstRunBubble::OnActivate(UINT action, BOOL minimized, HWND window) {
 }
 #endif
 
+#if !defined(USE_AURA)

[msw, 2011/10/18 18:39:23]

This seems like it won't work in any case...
I think we need to add some BubbleClosing API to the new bubble (I have the same
roadblock in CriticalNotificationBubbleView)

[alicet1, 2011/10/19 05:02:15]

updated.

 void FirstRunBubble::BubbleClosing(Bubble* bubble, bool closed_by_escape) {
   // Make sure our parent window is re-enabled.
   if (!IsWindowEnabled(GetParent()))
     ::EnableWindow(GetParent(), true);
 }
+#endif