Cleanup BubbleFrameView and BubbleBorder construction.

Pass custom color to BubbleBorder ctor (or SK_ColorWHITE default).
Call BubbleFrameView::SetBubbleBorder instead and remove border ctor arg.
Misc refactoring and cleanup.

TODO(followup): Update bubbles to use BubbleDelegateView where possible.
TODO(followup): Use theme colors for misc borders instead of SK_ColorWHITE.

BUG=166075
TEST=No observable changes.
R=sky@chromium.org

Review URL: https://chromiumcodereview.appspot.com/12096084

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@180090 0039d316-1c4b-4281-b951-d872f2087c98

ash/wm/maximize_bubble_controller.cc

@@ -83,7 +83,8 @@ class MaximizeBubbleBorder : public views::BubbleBorder {
 MaximizeBubbleBorder::MaximizeBubbleBorder(views::View* content_view,
                                            views::View* anchor)
     : views::BubbleBorder(views::BubbleBorder::TOP_RIGHT,
-                          views::BubbleBorder::NO_SHADOW),
+                          views::BubbleBorder::NO_SHADOW,
+                          kBubbleBackgroundColor),
       anchor_size_(anchor->size()),
       anchor_screen_origin_(0, 0),
       content_view_(content_view) {

chrome/browser/ui/views/fullscreen_exit_bubble_views.cc

@@ -121,9 +121,8 @@ FullscreenExitBubbleViews::FullscreenExitView::FullscreenExitView(
       message_label_(NULL),
       button_view_(NULL),
       browser_fullscreen_exit_accelerator_(accelerator) {
-  views::BubbleBorder* bubble_border =
-      new views::BubbleBorder(views::BubbleBorder::NONE,
-                              views::BubbleBorder::SHADOW);
+  views::BubbleBorder* bubble_border = new views::BubbleBorder(
+      views::BubbleBorder::NONE, views::BubbleBorder::SHADOW, SK_ColorWHITE);
   set_background(new views::BubbleBackground(bubble_border));
   set_border(bubble_border);
   set_focusable(false);

chrome/browser/ui/views/notifications/balloon_view_views.cc

@@ -105,10 +105,8 @@ BalloonViewImpl::BalloonViewImpl(BalloonCollection* collection)
   // We're owned by Balloon and don't want to be deleted by our parent View.
   set_owned_by_client();
 
-  views::BubbleBorder* bubble_border =
-      new views::BubbleBorder(views::BubbleBorder::FLOAT,
-                              views::BubbleBorder::NO_SHADOW);
-  set_border(bubble_border);
+  set_border(new views::BubbleBorder(views::BubbleBorder::FLOAT,
+      views::BubbleBorder::NO_SHADOW, SK_ColorWHITE));
 }
 
 BalloonViewImpl::~BalloonViewImpl() {

chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc

@@ -92,13 +92,9 @@ OmniboxPopupContentsView::OmniboxPopupContentsView(
       font_(font.DeriveFont(kEditFontAdjust)),
       ignore_mouse_drag_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(size_animation_(this)) {
-  // The following little dance is required because set_border() requires a
-  // pointer to a non-const object.
-  views::BubbleBorder* bubble_border =
-      new views::BubbleBorder(views::BubbleBorder::NONE,
-                              views::BubbleBorder::NO_SHADOW);
-  bubble_border_ = bubble_border;
-  set_border(bubble_border);
+  bubble_border_ = new views::BubbleBorder(views::BubbleBorder::NONE,
+      views::BubbleBorder::NO_SHADOW, SK_ColorWHITE);
+  set_border(const_cast<views::BubbleBorder*>(bubble_border_));
   // The contents is owned by the LocationBarView.
   set_owned_by_client();
 }

ui/views/bubble/bubble_border.cc

@@ -133,12 +133,12 @@ struct BubbleBorder::BorderImages {
 struct BubbleBorder::BorderImages*
     BubbleBorder::border_images_[SHADOW_COUNT] = { NULL };
 
-BubbleBorder::BubbleBorder(ArrowLocation arrow_location, Shadow shadow)
+BubbleBorder::BubbleBorder(ArrowLocation arrow, Shadow shadow, SkColor color)
     : override_arrow_offset_(0),
-      arrow_location_(arrow_location),
+      arrow_location_(arrow),
       paint_arrow_(true),
       alignment_(ALIGN_ARROW_TO_MID_ANCHOR),
-      background_color_(SK_ColorWHITE) {
+      background_color_(color) {
   DCHECK(shadow < SHADOW_COUNT);
   images_ = GetBorderImages(shadow);
 

ui/views/bubble/bubble_border.h

@@ -65,7 +65,7 @@ class VIEWS_EXPORT BubbleBorder : public Border {
     ALIGN_EDGE_TO_ANCHOR_EDGE
   };
 
-  BubbleBorder(ArrowLocation arrow_location, Shadow shadow);
+  BubbleBorder(ArrowLocation arrow, Shadow shadow, SkColor color);
 
   // Returns the radius of the corner of the border.
   // TODO(xiyuan): Get rid of this since it's part of BorderImages now?

ui/views/bubble/bubble_delegate.cc

@@ -184,14 +184,11 @@ View* BubbleDelegateView::GetContentsView() {
 
 NonClientFrameView* BubbleDelegateView::CreateNonClientFrameView(
     Widget* widget) {
-  BubbleBorder::ArrowLocation arrow_loc = arrow_location();
-  if (base::i18n::IsRTL())
-    arrow_loc = BubbleBorder::horizontal_mirror(arrow_loc);
-  BubbleBorder* border = new BubbleBorder(arrow_loc, shadow_);
-  border->set_background_color(color());
-  BubbleFrameView* frame_view = new BubbleFrameView(margins(), border);
-  frame_view->set_background(new BubbleBackground(border));
-  return frame_view;
+  BubbleFrameView* frame = new BubbleFrameView(margins());
+  BubbleBorder::ArrowLocation arrow = base::i18n::IsRTL() ?
+      BubbleBorder::horizontal_mirror(arrow_location()) : arrow_location();
+  frame->SetBubbleBorder(new BubbleBorder(arrow, shadow(), color()));
+  return frame;
 }
 
 void BubbleDelegateView::OnWidgetClosing(Widget* widget) {

ui/views/bubble/bubble_frame_view.cc

@@ -39,11 +39,9 @@ int GetOffScreenLength(const gfx::Rect& monitor_bounds,
 
 namespace views {
 
-BubbleFrameView::BubbleFrameView(const gfx::Insets& margins,
-                                 BubbleBorder* border)
-    : bubble_border_(border),
-      content_margins_(margins) {
-  set_border(bubble_border_);
+BubbleFrameView::BubbleFrameView(const gfx::Insets& content_margins)
+    : bubble_border_(NULL),
+      content_margins_(content_margins) {
 }
 
 BubbleFrameView::~BubbleFrameView() {}
@@ -70,6 +68,14 @@ gfx::Size BubbleFrameView::GetPreferredSize() {
   return GetUpdatedWindowBounds(gfx::Rect(), client_size, false).size();
 }
 
+void BubbleFrameView::SetBubbleBorder(BubbleBorder* border) {
+  bubble_border_ = border;
+  set_border(bubble_border_);
+
+  // Update the background, which relies on the border.
+  set_background(new views::BubbleBackground(border));
+}
+
 gfx::Rect BubbleFrameView::GetUpdatedWindowBounds(const gfx::Rect& anchor_rect,
                                                   gfx::Size client_size,
                                                   bool adjust_if_offscreen) {
@@ -91,14 +97,6 @@ gfx::Rect BubbleFrameView::GetUpdatedWindowBounds(const gfx::Rect& anchor_rect,
   return bubble_border_->GetBounds(anchor_rect, client_size);
 }
 
-void BubbleFrameView::SetBubbleBorder(BubbleBorder* border) {
-  bubble_border_ = border;
-  set_border(bubble_border_);
-
-  // Update the background, which relies on the border.
-  set_background(new views::BubbleBackground(border));
-}
-
 gfx::Rect BubbleFrameView::GetMonitorBounds(const gfx::Rect& rect) {
   // TODO(scottmg): Native is wrong. http://crbug.com/133312
   return gfx::Screen::GetNativeScreen()->GetDisplayNearestPoint(

ui/views/bubble/bubble_frame_view.h

@@ -15,12 +15,11 @@ namespace views {
 
 class BubbleBorder;
 
-// This is a NonClientFrameView used to render the BubbleBorder.
+// The non-client frame view of bubble-styled widgets.
 class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView {
  public:
-  // Sets the border to |border|, taking ownership. Important: do not call
-  // set_border() directly to change the border, use SetBubbleBorder() instead.
-  BubbleFrameView(const gfx::Insets& margins, BubbleBorder* border);
+
+  explicit BubbleFrameView(const gfx::Insets& content_margins);
   virtual ~BubbleFrameView();
 
   // NonClientFrameView overrides:
@@ -37,7 +36,9 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView {
   // View overrides:
   virtual gfx::Size GetPreferredSize() OVERRIDE;
 
+  // Use bubble_border() and SetBubbleBorder(), not border() and set_border().
   BubbleBorder* bubble_border() const { return bubble_border_; }
+  void SetBubbleBorder(BubbleBorder* border);
 
   gfx::Insets content_margins() const { return content_margins_; }
 
@@ -48,8 +49,6 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView {
                                    gfx::Size client_size,
                                    bool adjust_if_offscreen);
 
-  void SetBubbleBorder(BubbleBorder* border);
-
  protected:
   // Returns the bounds for the monitor showing the specified |rect|.
   // This function is virtual to support testing environments.

ui/views/bubble/bubble_frame_view_unittest.cc

@@ -19,8 +19,8 @@ namespace {
 const BubbleBorder::ArrowLocation kArrow = BubbleBorder::TOP_LEFT;
 const int kBubbleWidth = 200;
 const int kBubbleHeight = 200;
-const SkColor kBackgroundColor = SK_ColorRED;
-const int kDefaultMargin = 6;
+const SkColor kColor = SK_ColorRED;
+const int kMargin = 6;
 
 class SizedBubbleDelegateView : public BubbleDelegateView {
  public:
@@ -59,13 +59,9 @@ class TestBubbleFrameView : public BubbleFrameView {
 };
 
 TestBubbleFrameView::TestBubbleFrameView()
-    : BubbleFrameView(gfx::Insets(kDefaultMargin,
-                                  kDefaultMargin,
-                                  kDefaultMargin,
-                                  kDefaultMargin),
-                      new BubbleBorder(kArrow, BubbleBorder::NO_SHADOW)),
+    : BubbleFrameView(gfx::Insets(kMargin, kMargin, kMargin, kMargin)),
       monitor_bounds_(gfx::Rect(0, 0, 1000, 1000)) {
-  bubble_border()->set_background_color(kBackgroundColor);
+  SetBubbleBorder(new BubbleBorder(kArrow, BubbleBorder::NO_SHADOW, kColor));
 }
 
 TestBubbleFrameView::~TestBubbleFrameView() {}
@@ -79,7 +75,7 @@ gfx::Rect TestBubbleFrameView::GetMonitorBounds(const gfx::Rect& rect) {
 TEST_F(BubbleFrameViewTest, GetBoundsForClientView) {
   TestBubbleFrameView frame;
   EXPECT_EQ(kArrow, frame.bubble_border()->arrow_location());
-  EXPECT_EQ(kBackgroundColor, frame.bubble_border()->background_color());
+  EXPECT_EQ(kColor, frame.bubble_border()->background_color());
 
   int margin_x = frame.content_margins().left();
   int margin_y = frame.content_margins().top();

ui/views/bubble/tray_bubble_view.cc

@@ -39,12 +39,12 @@ namespace internal {
 
 // Custom border for TrayBubbleView. Contains special logic for GetBounds()
 // to stack bubbles with no arrows correctly. Also calculates the arrow offset.
-class TrayBubbleBorder : public views::BubbleBorder {
+class TrayBubbleBorder : public BubbleBorder {
  public:
-  TrayBubbleBorder(views::View* owner,
-                   views::View* anchor,
+  TrayBubbleBorder(View* owner,
+                   View* anchor,
                    TrayBubbleView::InitParams params)
-      : views::BubbleBorder(params.arrow_location, params.shadow),
+      : BubbleBorder(params.arrow_location, params.shadow, params.arrow_color),
         owner_(owner),
         anchor_(anchor),
         tray_arrow_offset_(params.arrow_offset) {
@@ -55,15 +55,12 @@ class TrayBubbleBorder : public views::BubbleBorder {
 
   virtual ~TrayBubbleBorder() {}
 
-  // Overridden from views::BubbleBorder.
-  // Override views::BubbleBorder to set the bubble on top of the anchor when
-  // it has no arrow.
+  // Overridden from BubbleBorder.
+  // Sets the bubble on top of the anchor when it has no arrow.
   virtual gfx::Rect GetBounds(const gfx::Rect& position_relative_to,
                               const gfx::Size& contents_size) const OVERRIDE {
-    if (has_arrow(arrow_location())) {
-      return views::BubbleBorder::GetBounds(position_relative_to,
-                                            contents_size);
-    }
+    if (has_arrow(arrow_location()))
+      return BubbleBorder::GetBounds(position_relative_to, contents_size);
 
     gfx::Size border_size(contents_size);
     gfx::Insets insets = GetInsets();
@@ -79,8 +76,8 @@ class TrayBubbleBorder : public views::BubbleBorder {
 
   void UpdateArrowOffset() {
     int arrow_offset = 0;
-    if (arrow_location() == views::BubbleBorder::BOTTOM_RIGHT ||
-        arrow_location() == views::BubbleBorder::BOTTOM_LEFT) {
+    if (arrow_location() == BubbleBorder::BOTTOM_RIGHT ||
+        arrow_location() == BubbleBorder::BOTTOM_LEFT) {
       // Note: tray_arrow_offset_ is relative to the anchor widget.
       if (tray_arrow_offset_ ==
           TrayBubbleView::InitParams::kArrowDefaultOffset) {
@@ -88,12 +85,10 @@ class TrayBubbleBorder : public views::BubbleBorder {
       } else {
         const int width = owner_->GetWidget()->GetContentsView()->width();
         gfx::Point pt(tray_arrow_offset_, 0);
-        views::View::ConvertPointToScreen(
-            anchor_->GetWidget()->GetRootView(), &pt);
-        views::View::ConvertPointFromScreen(
-            owner_->GetWidget()->GetRootView(), &pt);
+        View::ConvertPointToScreen(anchor_->GetWidget()->GetRootView(), &pt);
+        View::ConvertPointFromScreen(owner_->GetWidget()->GetRootView(), &pt);
         arrow_offset = pt.x();
-        if (arrow_location() == views::BubbleBorder::BOTTOM_RIGHT)
+        if (arrow_location() == BubbleBorder::BOTTOM_RIGHT)
           arrow_offset = width - arrow_offset;
         arrow_offset = std::max(arrow_offset, kArrowMinOffset);
       }
@@ -103,10 +98,8 @@ class TrayBubbleBorder : public views::BubbleBorder {
         arrow_offset = kArrowMinOffset;
       } else {
         gfx::Point pt(0, tray_arrow_offset_);
-        views::View::ConvertPointToScreen(
-            anchor_->GetWidget()->GetRootView(), &pt);
-        views::View::ConvertPointFromScreen(
-            owner_->GetWidget()->GetRootView(), &pt);
+        View::ConvertPointToScreen(anchor_->GetWidget()->GetRootView(), &pt);
+        View::ConvertPointFromScreen(owner_->GetWidget()->GetRootView(), &pt);
         arrow_offset = pt.y();
         arrow_offset = std::max(arrow_offset, kArrowMinOffset);
       }
@@ -115,8 +108,8 @@ class TrayBubbleBorder : public views::BubbleBorder {
   }
 
  private:
-  views::View* owner_;
-  views::View* anchor_;
+  View* owner_;
+  View* anchor_;
   const int tray_arrow_offset_;
 
   DISALLOW_COPY_AND_ASSIGN(TrayBubbleBorder);
@@ -176,27 +169,27 @@ base::Closure TrayBubbleContentMask::PrepareForLayerBoundsChange() {
 
 // Custom layout for the bubble-view. Does the default box-layout if there is
 // enough height. Otherwise, makes sure the bottom rows are visible.
-class BottomAlignedBoxLayout : public views::BoxLayout {
+class BottomAlignedBoxLayout : public BoxLayout {
  public:
   explicit BottomAlignedBoxLayout(TrayBubbleView* bubble_view)
-      : views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0),
+      : BoxLayout(BoxLayout::kVertical, 0, 0, 0),
         bubble_view_(bubble_view) {
   }
 
   virtual ~BottomAlignedBoxLayout() {}
 
  private:
-  virtual void Layout(views::View* host) OVERRIDE {
+  virtual void Layout(View* host) OVERRIDE {
     if (host->height() >= host->GetPreferredSize().height() ||
         !bubble_view_->is_gesture_dragging()) {
-      views::BoxLayout::Layout(host);
+      BoxLayout::Layout(host);
       return;
     }
 
     int consumed_height = 0;
     for (int i = host->child_count() - 1;
         i >= 0 && consumed_height < host->height(); --i) {
-      views::View* child = host->child_at(i);
+      View* child = host->child_at(i);
       if (!child->visible())
         continue;
       gfx::Size size = child->GetPreferredSize();
@@ -232,16 +225,16 @@ TrayBubbleView::InitParams::InitParams(AnchorType anchor_type,
       can_activate(false),
       close_on_deactivate(true),
       arrow_color(SK_ColorBLACK),
-      arrow_location(views::BubbleBorder::NONE),
+      arrow_location(BubbleBorder::NONE),
       arrow_offset(kArrowDefaultOffset),
       hide_arrow(false),
-      shadow(views::BubbleBorder::BIG_SHADOW),
-      arrow_alignment(views::BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE) {
+      shadow(BubbleBorder::BIG_SHADOW),
+      arrow_alignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE) {
 }
 
 // static
 TrayBubbleView* TrayBubbleView::Create(gfx::NativeView parent_window,
-                                       views::View* anchor,
+                                       View* anchor,
                                        Delegate* delegate,
                                        InitParams* init_params) {
   // Set arrow_location here so that it can be passed correctly to the
@@ -249,26 +242,26 @@ TrayBubbleView* TrayBubbleView::Create(gfx::NativeView parent_window,
   if (init_params->anchor_type == ANCHOR_TYPE_TRAY) {
     if (init_params->anchor_alignment == ANCHOR_ALIGNMENT_BOTTOM) {
       init_params->arrow_location = base::i18n::IsRTL() ?
-          views::BubbleBorder::BOTTOM_LEFT : views::BubbleBorder::BOTTOM_RIGHT;
+          BubbleBorder::BOTTOM_LEFT : BubbleBorder::BOTTOM_RIGHT;
     } else if (init_params->anchor_alignment == ANCHOR_ALIGNMENT_TOP) {
-      init_params->arrow_location = views::BubbleBorder::TOP_LEFT;
+      init_params->arrow_location = BubbleBorder::TOP_LEFT;
     } else if (init_params->anchor_alignment == ANCHOR_ALIGNMENT_LEFT) {
-      init_params->arrow_location = views::BubbleBorder::LEFT_BOTTOM;
+      init_params->arrow_location = BubbleBorder::LEFT_BOTTOM;
     } else {
-      init_params->arrow_location = views::BubbleBorder::RIGHT_BOTTOM;
+      init_params->arrow_location = BubbleBorder::RIGHT_BOTTOM;
     }
   } else {
-    init_params->arrow_location = views::BubbleBorder::NONE;
+    init_params->arrow_location = BubbleBorder::NONE;
   }
 
   return new TrayBubbleView(parent_window, anchor, delegate, *init_params);
 }
 
 TrayBubbleView::TrayBubbleView(gfx::NativeView parent_window,
-                               views::View* anchor,
+                               View* anchor,
                                Delegate* delegate,
                                const InitParams& init_params)
-    : views::BubbleDelegateView(anchor, init_params.arrow_location),
+    : BubbleDelegateView(anchor, init_params.arrow_location),
       params_(init_params),
       delegate_(delegate),
       preferred_width_(init_params.min_width),
@@ -337,7 +330,7 @@ gfx::Insets TrayBubbleView::GetBorderInsets() const {
 }
 
 void TrayBubbleView::Init() {
-  views::BoxLayout* layout = new BottomAlignedBoxLayout(this);
+  BoxLayout* layout = new BottomAlignedBoxLayout(this);
   layout->set_spread_blank_space(true);
   SetLayoutManager(layout);
 }
@@ -354,12 +347,10 @@ bool TrayBubbleView::CanActivate() const {
   return params_.can_activate;
 }
 
-// Overridden to create BubbleFrameView and set a custom border.
-views::NonClientFrameView* TrayBubbleView::CreateNonClientFrameView(
-    views::Widget* widget) {
-  views::BubbleFrameView* bubble_frame_view =
-      new views::BubbleFrameView(margins(), bubble_border_);
-  return bubble_frame_view;
+NonClientFrameView* TrayBubbleView::CreateNonClientFrameView(Widget* widget) {
+  BubbleFrameView* frame = new BubbleFrameView(margins());
+  frame->SetBubbleBorder(bubble_border_);
+  return frame;
 }
 
 bool TrayBubbleView::WidgetHasHitTestMask() const {
@@ -372,7 +363,7 @@ void TrayBubbleView::GetWidgetHitTestMask(gfx::Path* mask) const {
 }
 
 gfx::Size TrayBubbleView::GetPreferredSize() {
-  gfx::Size size = views::BubbleDelegateView::GetPreferredSize();
+  gfx::Size size = BubbleDelegateView::GetPreferredSize();
   int height = size.height();
   if (params_.max_height != 0 && height > params_.max_height)
     height = params_.max_height;
@@ -407,8 +398,8 @@ void TrayBubbleView::ChildPreferredSizeChanged(View* child) {
 }
 
 void TrayBubbleView::ViewHierarchyChanged(bool is_add,
-                                          views::View* parent,
-                                          views::View* child) {
+                                          View* parent,
+                                          View* child) {
   if (get_use_acceleration_when_possible() && is_add && child == this) {
     parent->SetPaintToLayer(true);
     parent->SetFillsBoundsOpaquely(true);

ui/views/controls/menu/menu_scroll_view_container.cc

@@ -289,9 +289,8 @@ void MenuScrollViewContainer::CreateDefaultBorder() {
 }
 
 void MenuScrollViewContainer::CreateBubbleBorder() {
-  bubble_border_ = new BubbleBorder(
-      arrow_location_,
-      BubbleBorder::NO_SHADOW);
+  bubble_border_ =
+      new BubbleBorder(arrow_location_, BubbleBorder::NO_SHADOW, SK_ColorWHITE);
   set_border(bubble_border_);
   set_background(new BubbleBackground(bubble_border_));
 }

ui/views/window/dialog_frame_view.cc

@@ -37,9 +37,9 @@ DialogFrameView::DialogFrameView(const string16& title)
     : title_(NULL),
       close_(NULL) {
   BubbleBorder* border =
-      new BubbleBorder(BubbleBorder::FLOAT, BubbleBorder::SMALL_SHADOW);
-  border->set_background_color(GetNativeTheme()->GetSystemColor(
-      ui::NativeTheme::kColorId_DialogBackground));
+      new BubbleBorder(BubbleBorder::FLOAT, BubbleBorder::SMALL_SHADOW,
+                       GetNativeTheme()->GetSystemColor(
+                           ui::NativeTheme::kColorId_DialogBackground));
   set_border(border);
   // Update the background, which relies on the border.
   set_background(new BubbleBackground(border));