BubbleFrameView: add padding if platform dislikes close buttons

ui/views/bubble/bubble_frame_view_unittest.cc

 // Copyright (c) 2012 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 "ui/views/bubble/bubble_frame_view.h"
 
 #include <memory>
 
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/views/bubble/bubble_border.h"
+#include "ui/views/controls/button/label_button.h"
 #include "ui/views/test/test_views.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 
 namespace views {
 
 typedef ViewsTestBase BubbleFrameViewTest;
 
 namespace {
@@ skipping 29 matching lines @@
           gfx::Size(kPreferredClientWidth, kPreferredClientHeight));
       contents_view->set_minimum_size(
           gfx::Size(kMinimumClientWidth, kMinimumClientHeight));
       contents_view->set_maximum_size(
           gfx::Size(kMaximumClientWidth, kMaximumClientHeight));
       contents_view_ = contents_view;
     }
     return contents_view_;
   }
 
+  bool ShouldShowCloseButton() const override { return should_show_close_; }
+  bool should_show_close_ = false;

[msw, 2016/07/29 19:38:39]

Make this private and add a setter.

[Elly Fong-Jones, 2016/08/02 17:49:29]

Done.

+
  private:
   Widget* widget_;
   View* contents_view_ = nullptr;  // Owned by |widget_|.
 };
 
 class TestBubbleFrameView : public BubbleFrameView {
  public:
   TestBubbleFrameView(ViewsTestBase* test_base)
       : BubbleFrameView(gfx::Insets(), gfx::Insets(kMargin)),
         test_base_(test_base),
@@ skipping 17 matching lines @@
     }
 
     return widget_.get();
   }
 
   // BubbleFrameView overrides:
   gfx::Rect GetAvailableScreenBounds(const gfx::Rect& rect) const override {
     return available_bounds_;
   }
 
+  TestBubbleFrameViewWidgetDelegate* widget_delegate() {
+    return widget_delegate_.get();
+  }
+
  private:
   ViewsTestBase* test_base_;
 
   gfx::Rect available_bounds_;
 
   // Widget returned by GetWidget(). Only created if GetWidget() is called.
   mutable std::unique_ptr<TestBubbleFrameViewWidgetDelegate> widget_delegate_;
   mutable std::unique_ptr<Widget> widget_;
 
   DISALLOW_COPY_AND_ASSIGN(TestBubbleFrameView);
 };
 
 }  // namespace
 
 TEST_F(BubbleFrameViewTest, GetBoundsForClientView) {
   TestBubbleFrameView frame(this);
   EXPECT_EQ(kArrow, frame.bubble_border()->arrow());
   EXPECT_EQ(kColor, frame.bubble_border()->background_color());
 
   int margin_x = frame.content_margins().left();
   int margin_y = frame.content_margins().top();
   gfx::Insets insets = frame.bubble_border()->GetInsets();
   EXPECT_EQ(insets.left() + margin_x, frame.GetBoundsForClientView().x());
   EXPECT_EQ(insets.top() + margin_y, frame.GetBoundsForClientView().y());
 }
 
+TEST_F(BubbleFrameViewTest, GetBoundsForClientViewWithClose) {
+  TestBubbleFrameView frame(this);
+  // TestBubbleFrameView::GetWidget() is responsible for creating the widget and
+  // widget delegate at first call, so it is called here for that side-effect.
+  (void)frame.GetWidget();

[msw, 2016/07/29 19:38:39]

I think there's a more chrome-standard way of doing this, but I don't recall the
exact syntax, look for a minute?

[msw, 2016/07/29 19:40:59]

Found it: https://cs.chromium.org/chromium/src/base/macros.h?rcl=0&l=60

[Elly Fong-Jones, 2016/08/02 17:49:29]

Done.

[Elly Fong-Jones, 2016/08/02 17:49:29]

Done.

+  frame.widget_delegate()->should_show_close_ = true;
+  frame.OnThemeChanged();
+  EXPECT_EQ(kArrow, frame.bubble_border()->arrow());
+  EXPECT_EQ(kColor, frame.bubble_border()->background_color());
+
+  int margin_x = frame.content_margins().left();
+  int margin_y = frame.content_margins().top() +
+                 frame.GetCloseButtonForTest()->height();
+  gfx::Insets insets = frame.bubble_border()->GetInsets();
+  EXPECT_EQ(insets.left() + margin_x, frame.GetBoundsForClientView().x());
+  EXPECT_EQ(insets.top() + margin_y, frame.GetBoundsForClientView().y());
+}
+
 // Tests that the arrow is mirrored as needed to better fit the screen.
 TEST_F(BubbleFrameViewTest, GetUpdatedWindowBounds) {
   TestBubbleFrameView frame(this);
   gfx::Rect window_bounds;
 
   gfx::Insets insets = frame.bubble_border()->GetInsets();
   int xposition = 95 - insets.width();
 
   // Test that the info bubble displays normally when it fits.
   frame.bubble_border()->set_arrow(BubbleBorder::TOP_LEFT);
@@ skipping 316 matching lines @@
   maximum_rect.Inset(frame.bubble_border()->GetInsets());
 
   // Should ignore the contents view's maximum size and use the preferred size.
   gfx::Size expected_size(kPreferredClientWidth + kExpectedAdditionalWidth,
                           kPreferredClientHeight + kExpectedAdditionalHeight);
   EXPECT_EQ(expected_size, maximum_rect.size());
 #endif
 }
 
 }  // namespace views