first run bubble using the views/bubble api.

chrome/browser/ui/views/first_run_bubble.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/first_run_bubble.h"
 
 #include "base/bind.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/ui/views/bubble/bubble.h"  // for kBackgroundColor
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/search_engines/util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "content/browser/user_metrics.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "grit/theme_resources_standard.h"
 #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"

[msw, 2011/10/20 19:17:50]

remove to merge with the latest bubble changes.

[alicet1, 2011/10/21 19:13:16]

Done.

 #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/fill_layout.h"
 #include "views/layout/layout_constants.h"
+#if defined(OS_WIN) && !defined(USE_AURA)
 #include "views/widget/native_widget_win.h"
+#endif
 #include "views/widget/widget.h"
 
 namespace {
 
 // How much extra padding to put around our content over what the Bubble
 // provides.
 const int kBubblePadding = 4;
 
 // How much extra padding to put around our content over what the Bubble
 // 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;
 
-// 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
-// to the frame HWND.
-HWND GetLogicalBubbleOwner(HWND bubble_hwnd) {
-  HWND border_widget_hwnd = GetWindow(bubble_hwnd, GW_OWNER);
-  return GetWindow(border_widget_hwnd, GW_OWNER);
-}
-
 }  // namespace
 
 // Base class for implementations of the client view which appears inside the
 // first run bubble. It is a dialog-ish view, but is not a true dialog.
 class FirstRunBubbleViewBase : public views::View,

[msw, 2011/10/20 19:17:50]

This should be merged with FirstRunBubble (it's exactly what BubbleDelegateView
subclasses are for). Unfortunately, that *might* be complicated by all the
various FirstRunBubbleViews.

[alicet1, 2011/10/21 19:13:16]

I think I'll leave this like this, the three views are slightly different, and
laying them out seems clearer.

                                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 void BubbleShown()  = 0;

[msw, 2011/10/20 19:17:50]

remove extra space

[alicet1, 2011/10/21 19:13:16]

Done.

 };
 
 // FirstRunBubbleView ---------------------------------------------------------
 
 class FirstRunBubbleView : public FirstRunBubbleViewBase {
  public:
   FirstRunBubbleView(FirstRunBubble* bubble_window, Profile* profile);
+  // Override from FirstRunBubbleViewBase:

[msw, 2011/10/20 19:17:50]

Add a blank line above this comment.

[alicet1, 2011/10/21 19:13:16]

Done.

+  void BubbleShown();
 
  private:
-  virtual ~FirstRunBubbleView() {}
-
-  // FirstRunBubbleViewBase:
-  virtual void BubbleShown();
+  virtual ~FirstRunBubbleView();
 
   // Overridden from View:
   virtual void ButtonPressed(views::Button* sender, const views::Event& event);
+  virtual gfx::Size GetPreferredSize();
   virtual void Layout();
-  virtual gfx::Size GetPreferredSize();
 
   // FocusChangeListener:
   virtual void FocusWillChange(View* focused_before, View* focused_now);
 
   FirstRunBubble* bubble_window_;

[msw, 2011/10/20 19:17:50]

bubble_window_ and profile_ are use on all three FirstRunBubbleViewBase
subclasses. How about moving them there and adding protected accessors if
they're needed by the bubble subclasses?

[alicet1, 2011/10/21 19:13:16]

Done.

   views::Label* label1_;
   views::Label* label2_;
   views::Label* label3_;
+  views::NativeTextButton* keep_button_;
   views::NativeTextButton* change_button_;
-  views::NativeTextButton* keep_button_;
   Profile* profile_;
 
   DISALLOW_COPY_AND_ASSIGN(FirstRunBubbleView);
 };
 
 FirstRunBubbleView::FirstRunBubbleView(FirstRunBubble* bubble_window,
                                        Profile* profile)
     : bubble_window_(bubble_window),
       label1_(NULL),
       label2_(NULL),
@@ skipping 24 matching lines @@
       IDS_FR_BUBBLE_QUESTION,
       GetDefaultSearchEngineName(profile));
   label3_ = new views::Label(question_str);
   label3_->SetMultiLine(true);
   label3_->SetFont(font);
   label3_->SetBackgroundColor(Bubble::kBackgroundColor);
   label3_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   label3_->SizeToFit(ps.width() - kBubblePadding * 2);
   AddChildView(label3_);
 
-  std::wstring keep_str = UTF16ToWide(l10n_util::GetStringFUTF16(
+  string16 keep_str = l10n_util::GetStringFUTF16(
       IDS_FR_BUBBLE_OK,
-      GetDefaultSearchEngineName(profile)));
+      GetDefaultSearchEngineName(profile));
   keep_button_ = new views::NativeTextButton(this, keep_str);
   keep_button_->SetIsDefault(true);
   AddChildView(keep_button_);
 
-  std::wstring change_str =
-      UTF16ToWide(l10n_util::GetStringUTF16(IDS_FR_BUBBLE_CHANGE));
+  string16 change_str = l10n_util::GetStringUTF16(IDS_FR_BUBBLE_CHANGE);
   change_button_ = new views::NativeTextButton(this, change_str);
   AddChildView(change_button_);
 }
 
-void FirstRunBubbleView::BubbleShown() {
-  keep_button_->RequestFocus();
+FirstRunBubbleView::~FirstRunBubbleView() {
+  GetFocusManager()->RemoveFocusChangeListener(this);

[msw, 2011/10/20 19:17:50]

This belongs in ~FirstRunBubbleViewBase.

[alicet1, 2011/10/21 19:13:16]

Done.

 }
 
 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);

[msw, 2011/10/20 19:17:50]

You can just call StartFade on a BubbleDelegateView subclass with the latest
changes. This should be made a BubbleDelegateView subclass via
FirstRunBubbleViewBase, but if you don't do that, I guess you can
static_cast<BubbleDelegateView>(GetWidget->widget_delegate())->StartFade.

[alicet1, 2011/10/21 19:13:16]

Done.

   if (change_button_ == sender) {
     UserMetrics::RecordAction(
                     UserMetricsAction("FirstRunBubbleView_ChangeButton"));
 
     Browser* browser = BrowserList::GetLastActiveWithProfile(profile_);
     if (browser) {
       browser->OpenSearchEngineOptionsDialog();
     }
   }
+  GetWidget()->Close();
+}
+
+void FirstRunBubbleView::BubbleShown() {
+  keep_button_->RequestFocus();

[msw, 2011/10/20 19:17:50]

Couldn't this be accomplished by overriding GetInitiallyFocusedView?
With that, this function may not be needed after all.

[alicet1, 2011/10/21 19:13:16]

if this is a delegate, then it would have GetInitiallyFocusedView. Since we kept
the original implementation of separate views, so this method stays.

 }
 
 void FirstRunBubbleView::Layout() {
   gfx::Size canvas = GetPreferredSize();
 
   // The multiline business that follows is dirty hacks to get around
   // bug 1325257.
   label1_->SetMultiLine(false);
   gfx::Size pref_size = label1_->GetPreferredSize();
   label1_->SetMultiLine(true);
@@ skipping 53 matching lines @@
     after->SetIsDefault(true);
   }
 }
 
 // FirstRunOEMBubbleView ------------------------------------------------------
 
 class FirstRunOEMBubbleView : public FirstRunBubbleViewBase {
  public:
   FirstRunOEMBubbleView(FirstRunBubble* bubble_window, Profile* profile);
 
+  // Override from FirstRunBubbleViewBase:
+  void BubbleShown();
  private:

[msw, 2011/10/20 19:17:50]

add a blank line above private.

[alicet1, 2011/10/21 19:13:16]

Done.

-  virtual ~FirstRunOEMBubbleView() { }
-
-  // FirstRunBubbleViewBase:
-  virtual void BubbleShown();
+  virtual ~FirstRunOEMBubbleView();
 
   // Overridden from View:
   virtual void ButtonPressed(views::Button* sender, const views::Event& event);
+  virtual gfx::Size GetPreferredSize();
   virtual void Layout();
-  virtual gfx::Size GetPreferredSize();
 
   // FocusChangeListener:
   virtual void FocusWillChange(View* focused_before, View* focused_now);
 
   FirstRunBubble* bubble_window_;
   views::Label* label1_;
   views::Label* label2_;
   views::Label* label3_;
   views::ImageButton* close_button_;
   Profile* profile_;
@@ skipping 44 matching lines @@
   close_button_->SetImage(views::CustomButton::BS_HOT,
                           rb.GetBitmapNamed(IDR_CLOSE_BAR_H));
   close_button_->SetImage(views::CustomButton::BS_PUSHED,
                           rb.GetBitmapNamed(IDR_CLOSE_BAR_P));
 
   AddChildView(close_button_);
 }
 
 void FirstRunOEMBubbleView::BubbleShown() {
   RequestFocus();
-  // No button in oem_bubble to request focus.
+}
+
+FirstRunOEMBubbleView::~FirstRunOEMBubbleView() {
+  GetFocusManager()->RemoveFocusChangeListener(this);

[msw, 2011/10/20 19:17:50]

Ditto, this belongs in ~FirstRunBubbleViewBase.

[alicet1, 2011/10/21 19:13:16]

Done.

 }
 
 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);

[msw, 2011/10/20 19:17:50]

Ditto on BubbleDelegateView::StartFade comment, also remove the "/*fade_in=*/"

[alicet1, 2011/10/21 19:13:16]

Done.

+  GetWidget()->Close();
 }
 
 void FirstRunOEMBubbleView::Layout() {
   gfx::Size canvas = GetPreferredSize();
 
   // First, draw the close button on the far right.
   gfx::Size sz = close_button_->GetPreferredSize();
   close_button_->SetBounds(
       canvas.width() - sz.width() - kMarginRightOfCloseButton,
       kOEMBubblePadding, sz.width(), sz.height());
@@ skipping 22 matching lines @@
 gfx::Size FirstRunOEMBubbleView::GetPreferredSize() {
   // Calculate width based on font and text.
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   const gfx::Font& font = rb.GetFont(
       ResourceBundle::MediumFont).DeriveFont(3, gfx::Font::BOLD);
   gfx::Size size = gfx::Size(
       ui::GetLocalizedContentsWidthForFont(
           IDS_FIRSTRUNOEMBUBBLE_DIALOG_WIDTH_CHARS, font),
       ui::GetLocalizedContentsHeightForFont(
           IDS_FIRSTRUNOEMBUBBLE_DIALOG_HEIGHT_LINES, font));
-
+#if defined(OS_WIN) && !defined(USE_AURA)
   // 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
   // XP or Vista so that the length of a string is calculated properly. For
   // now, we force Vista to show a correctly-sized box by taking account of
   // the difference in font size calculation. The coefficient should not be
   // stored in a variable because it's a hack and should go away.

[msw, 2011/10/20 19:17:50]

cry :( even if this comment is [still] true, switching by aero glass isn't
equivalent to switching on xp/vista, although it might be what's actually
intended by this hack. It'd be great if you could remove this and ensure that's
okay.

[alicet1, 2011/10/21 19:13:16]

I can probably believe this is still true.

   if (views::NativeWidgetWin::IsAeroGlassEnabled()) {
     size.set_width(static_cast<int>(size.width() * 0.85));
     size.set_height(static_cast<int>(size.height() * 0.85));
   }
+#endif
   return size;
 }
 
 void FirstRunOEMBubbleView::FocusWillChange(View* focused_before,
                                             View* focused_now) {
   // No buttons in oem_bubble to register focus changes.
 }
 
 // FirstRunMinimalBubbleView --------------------------------------------------
 // TODO(mirandac): combine FRBubbles more elegantly.  http://crbug.com/41353
 
 class FirstRunMinimalBubbleView : public FirstRunBubbleViewBase {
  public:
   FirstRunMinimalBubbleView(FirstRunBubble* bubble_window, Profile* profile);
 
+  // Override from FirstRunBubbleViewBase:

[msw, 2011/10/20 19:17:50]

Can you go through and make sure all the override comments in these files are
consistent? "override/overriden from"/[x]/"overrides"...

[alicet1, 2011/10/21 19:13:16]

Done.

+  void BubbleShown();
+
  private:
-  virtual ~FirstRunMinimalBubbleView() { }
-
-  // FirstRunBubbleViewBase:
-  virtual void BubbleShown();
+  virtual ~FirstRunMinimalBubbleView();
 
   // Overridden from View:
   virtual void ButtonPressed(views::Button* sender,
                              const views::Event& event) { }
+  virtual gfx::Size GetPreferredSize();
   virtual void Layout();
-  virtual gfx::Size GetPreferredSize();
 
   // FocusChangeListener:
   virtual void FocusWillChange(View* focused_before, View* focused_now);
 
   FirstRunBubble* bubble_window_;
   Profile* profile_;
   views::Label* label1_;
   views::Label* label2_;
 
   DISALLOW_COPY_AND_ASSIGN(FirstRunMinimalBubbleView);
@@ skipping 22 matching lines @@
   label2_ = new views::Label(
       l10n_util::GetStringUTF16(IDS_FR_BUBBLE_SUBTEXT));
   label2_->SetMultiLine(true);
   label2_->SetFont(font);
   label2_->SetBackgroundColor(Bubble::kBackgroundColor);
   label2_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   label2_->SizeToFit(ps.width() - kBubblePadding * 2);
   AddChildView(label2_);
 }
 
+FirstRunMinimalBubbleView::~FirstRunMinimalBubbleView() {
+  GetFocusManager()->RemoveFocusChangeListener(this);

[msw, 2011/10/20 19:17:50]

Ditto, this belongs in ~FirstRunBubbleViewBase.

[alicet1, 2011/10/21 19:13:16]

Done.

+}
+
 void FirstRunMinimalBubbleView::BubbleShown() {
   RequestFocus();
 }
 
 void FirstRunMinimalBubbleView::Layout() {
   gfx::Size canvas = GetPreferredSize();
 
   // See comments in FirstRunOEMBubbleView::Layout explaining this hack.
   label1_->SetMultiLine(false);
   gfx::Size pref_size = label1_->GetPreferredSize();
@@ skipping 20 matching lines @@
 
 void FirstRunMinimalBubbleView::FocusWillChange(View* focused_before,
                                                 View* focused_now) {
   // No buttons in minimal bubble to register focus changes.
 }
 
 
 // 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();
+  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);

[msw, 2011/10/20 19:17:50]

Ditto on BubbleDelegateView::StartFade comment, also remove the "/*fade_in=*/"

[alicet1, 2011/10/21 19:13:16]

Done.

+  return widget;
+}
+
+void FirstRunBubble::Init() {
+  SetLayoutManager(new views::FillLayout());
   FirstRunBubbleViewBase* view = NULL;
-
-  switch (bubble_type) {
+  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);
+  AddChildView(view);
+  parent_->GetFocusManager()->AddFocusChangeListener(view);

[msw, 2011/10/20 19:17:50]

I think you can just call GetFocusManager on this view.

[alicet1, 2011/10/21 19:13:16]

Init() is called before we have a widget for this view, that will mean a
GetWidget() call inside GetFocusManager() will fail and not return a proper
focus manager. parent_ seems safer.

   view->BubbleShown();
-  return bubble;
 }
 
-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),

[msw, 2011/10/20 19:17:50]

You can use the new BubbleDelegateView ctor here to set the anchor and arrow,
and then you don't need to override GetAnchorPoint

[alicet1, 2011/10/21 19:13:16]

Done.

+      parent_(parent),
+      position_relative_to_(position_relative_to),
+      arrow_location_(arrow_location),
+      bubble_type_(bubble_type),
+      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();
+}
+
+#if defined(OS_WIN) && !defined(USE_AURA)
 void FirstRunBubble::EnableParent() {
   ::EnableWindow(GetParent(), true);
   // The EnableWindow() call above causes the parent to become active, which
   // resets the flag set by Bubble's call to DisableInactiveRendering(), so we
   // have to call it again before activating the bubble to prevent the parent
   // window from rendering inactive.
   // TODO(beng): this only works in custom-frame mode, not glass-frame mode.
-  HWND bubble_owner = GetLogicalBubbleOwner(GetNativeView());
-  views::Widget* parent = views::Widget::GetWidgetForNativeView(bubble_owner);
-  if (parent)
-    parent->DisableInactiveRendering();
+  if (parent_)
+    parent_->DisableInactiveRendering();
   // Reactivate the FirstRunBubble so it responds to OnActivate messages.
   SetWindowPos(GetParent(), 0, 0, 0, 0,
                SWP_NOSIZE | SWP_NOMOVE | SWP_NOREDRAW | SWP_SHOWWINDOW);
 }
 
-#if defined(OS_WIN) && !defined(USE_AURA)
 void FirstRunBubble::OnActivate(UINT action, BOOL minimized, HWND window) {
   // Keep the bubble around for kLingerTime milliseconds, to prevent accidental
   // closure.
   const int kLingerTime = 3000;
 
   // We might get re-enabled right before we are closed (sequence is: we get
   // deactivated, we call close, before we are actually closed we get
   // reactivated). Don't do the disabling of the parent in such cases.
   if (action == WA_ACTIVE && !has_been_activated_) {
     has_been_activated_ = true;
 
     ::EnableWindow(GetParent(), false);
 
     MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
         base::Bind(&FirstRunBubble::EnableParent,
                    enable_window_method_factory_.GetWeakPtr()),
         kLingerTime);
     return;
   }
 
   // Keep window from automatically closing until kLingerTime has passed.
   if (::IsWindowEnabled(GetParent()))
     Bubble::OnActivate(action, minimized, window);
 }
 #endif
 
-void FirstRunBubble::BubbleClosing(Bubble* bubble, bool closed_by_escape) {
+void FirstRunBubble::WindowClosing() {
   // Make sure our parent window is re-enabled.
-  if (!IsWindowEnabled(GetParent()))
-    ::EnableWindow(GetParent(), true);
+  parent_->GetRootView()->SetEnabled(true);

[msw, 2011/10/20 19:17:50]

I don't think this is right... perhaps try the FocusManager?

[alicet1, 2011/10/21 19:13:16]

I'm not sure why we need to explicitly enable/set focus for the parent, assuming
this bubble is already closing? 

 }