Fixed BubbleBorder sizing problems & added unit tests.

ui/views/bubble/bubble_border_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_border.h"
 
+#include "base/memory/scoped_ptr.h"
 #include "ui/views/test/views_test_base.h"
 
 namespace views {
 
 typedef ViewsTestBase BubbleBorderTest;
 
 TEST_F(BubbleBorderTest, GetMirroredArrow) {
   // Horizontal mirroring.
   EXPECT_EQ(BubbleBorder::TOP_RIGHT,
             BubbleBorder::horizontal_mirror(BubbleBorder::TOP_LEFT));
@@ skipping 178 matching lines @@
   EXPECT_TRUE(BubbleBorder::is_arrow_at_center(BubbleBorder::TOP_CENTER));
   EXPECT_TRUE(BubbleBorder::is_arrow_at_center(BubbleBorder::BOTTOM_CENTER));
 
   EXPECT_TRUE(BubbleBorder::is_arrow_at_center(BubbleBorder::LEFT_CENTER));
   EXPECT_TRUE(BubbleBorder::is_arrow_at_center(BubbleBorder::RIGHT_CENTER));
 
   EXPECT_FALSE(BubbleBorder::is_arrow_at_center(BubbleBorder::NONE));
   EXPECT_FALSE(BubbleBorder::is_arrow_at_center(BubbleBorder::FLOAT));
 }
 
-TEST_F(BubbleBorderTest, TestMinimalSize) {
-  gfx::Rect anchor = gfx::Rect(100, 100, 20, 20);
-  gfx::Size contents = gfx::Size(10, 10);
-  BubbleBorder b1(BubbleBorder::RIGHT_TOP, BubbleBorder::NO_SHADOW, 0);
-
-  // The height should be much bigger then the requested size + border and
-  // padding since it needs to be able to include the tip bitmap.
-  gfx::Rect visible_tip_1 = b1.GetBounds(anchor, contents);
-  EXPECT_GE(visible_tip_1.height(), 30);
-  EXPECT_LE(visible_tip_1.width(), 30);
-
-  // With the tip being invisible the height should now be much smaller.
-  b1.set_paint_arrow(BubbleBorder::PAINT_TRANSPARENT);
-  gfx::Rect invisible_tip_1 = b1.GetBounds(anchor, contents);
-  EXPECT_LE(invisible_tip_1.height(), 30);
-  EXPECT_LE(invisible_tip_1.width(), 30);
-
-  // When the orientation of the tip changes, the above mentioned tests need to
-  // be reverse for width and height.
-  BubbleBorder b2(BubbleBorder::TOP_RIGHT, BubbleBorder::NO_SHADOW, 0);
-
-  // The width should be much bigger then the requested size + border and
-  // padding since it needs to be able to include the tip bitmap.
-  gfx::Rect visible_tip_2 = b2.GetBounds(anchor, contents);
-  EXPECT_GE(visible_tip_2.width(), 30);
-  EXPECT_LE(visible_tip_2.height(), 30);
-
-  // With the tip being invisible the width should now be much smaller.
-  b2.set_paint_arrow(BubbleBorder::PAINT_TRANSPARENT);
-  gfx::Rect invisible_tip_2 = b2.GetBounds(anchor, contents);
-  EXPECT_LE(invisible_tip_2.width(), 30);
-  EXPECT_LE(invisible_tip_2.height(), 30);
-}
-
+class BubbleBorderGetBoundsSizeTest : public views::ViewsTestBase {

[msw, 2014/08/11 22:26:33]

Merge the two test classes, they're similar enough.
(optionally replace the BubbleBorderTest typedef)

[bruthig, 2014/08/19 19:02:13]

Done.

+ public:
+  const BubbleBorder::Arrow kHorizontalArrow = BubbleBorder::TOP_CENTER;

[msw, 2014/08/11 22:26:32]

nit: inline these constants, they're only used once each.

[bruthig, 2014/08/19 19:02:13]

Done.

+  const BubbleBorder::Arrow kVerticalArrow = BubbleBorder::LEFT_CENTER;
+
+  const gfx::Size kSmallSize = gfx::Size(1, 1);
+  const gfx::Size kMediumSize = gfx::Size(50, 50);
+
+  BubbleBorderGetBoundsSizeTest() {}
+  virtual ~BubbleBorderGetBoundsSizeTest() {}
+
+  virtual gfx::Size GetSize(gfx::Size contents_size) {

[msw, 2014/08/11 22:26:33]

Make these member functions protected and non-virtual.

[bruthig, 2014/08/19 19:02:13]

Done.

+    DCHECK(bubble_border_);

[msw, 2014/08/11 22:26:32]

Remove DCHECKs before accessing the member.

[bruthig, 2014/08/19 19:02:13]

Done.

+    return bubble_border_->GetBounds(gfx::Rect(100, 100, 0, 0),
+                                     contents_size).size();
+  }
+
+  virtual void CreateBubbleBorder(BubbleBorder::Arrow arrow) {
+    bubble_border_.reset(new BubbleBorder(arrow,
+                                          BubbleBorder::NO_SHADOW,
+                                          0 /* color */));

[msw, 2014/08/11 22:26:33]

nit: just use SK_ColorWHITE or similar and remove the comment.

[bruthig, 2014/08/19 19:02:13]

Done.

+  }
+
+  virtual bool HasArrow() {
+    DCHECK(bubble_border_);
+    return BubbleBorder::has_arrow(bubble_border_->arrow());
+  }
+
+  virtual void SetPaintArrow(BubbleBorder::ArrowPaintType value) {
+    DCHECK(bubble_border_);
+    bubble_border_->set_paint_arrow(value);
+  }
+
+ private:
+  scoped_ptr<BubbleBorder> bubble_border_;
+
+  DISALLOW_COPY_AND_ASSIGN(BubbleBorderGetBoundsSizeTest);
+};
+
+TEST_F(BubbleBorderGetBoundsSizeTest, WithHorizontalArrow) {
+  CreateBubbleBorder(kHorizontalArrow);
+
+  ASSERT_TRUE(HasArrow());
+
+  SetPaintArrow(BubbleBorder::PAINT_NORMAL);
+  EXPECT_EQ(gfx::Size(45, 29), GetSize(kSmallSize));

[msw, 2014/08/11 22:26:32]

These values won't be robust to bubble asset changes, etc. Can you break them up
into components that would be easier to update, then ensure that the resulting
sizes match some combination, like:
  const int kBorder = x;
  ...
  EXPECT_EQ(2 * kBorder + kSmallSize.width(), GetSize(kSmallSize).width())

[bruthig, 2014/08/19 19:02:13]

Done.

+  EXPECT_EQ(gfx::Size(60, 67), GetSize(kMediumSize));
+
+  SetPaintArrow(BubbleBorder::PAINT_TRANSPARENT);
+  EXPECT_EQ(gfx::Size(45, 29), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 67), GetSize(kMediumSize));
+
+  SetPaintArrow(BubbleBorder::PAINT_NONE);
+  EXPECT_EQ(gfx::Size(22, 22), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 60), GetSize(kMediumSize));
+}
+
+TEST_F(BubbleBorderGetBoundsSizeTest, WithVerticalArrow) {
+  CreateBubbleBorder(kVerticalArrow);
+
+  ASSERT_TRUE(HasArrow());
+
+  SetPaintArrow(BubbleBorder::PAINT_NORMAL);
+  EXPECT_EQ(gfx::Size(29, 45), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(67, 60), GetSize(kMediumSize));
+
+  SetPaintArrow(BubbleBorder::PAINT_TRANSPARENT);
+  EXPECT_EQ(gfx::Size(29, 45), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(67, 60), GetSize(kMediumSize));
+
+  SetPaintArrow(BubbleBorder::PAINT_NONE);
+  EXPECT_EQ(gfx::Size(22, 22), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 60), GetSize(kMediumSize));
+}
+
+TEST_F(BubbleBorderGetBoundsSizeTest, WithoutArrow) {
+  CreateBubbleBorder(BubbleBorder::NONE);
+
+  ASSERT_FALSE(HasArrow());
+
+  SetPaintArrow(BubbleBorder::PAINT_NORMAL);
+  EXPECT_EQ(gfx::Size(22, 22), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 60), GetSize(kMediumSize));
+
+  SetPaintArrow(BubbleBorder::PAINT_TRANSPARENT);
+  EXPECT_EQ(gfx::Size(22, 22), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 60), GetSize(kMediumSize));
+
+  SetPaintArrow(BubbleBorder::PAINT_NONE);
+  EXPECT_EQ(gfx::Size(22, 22), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 60), GetSize(kMediumSize));
+}
+
+TEST_F(BubbleBorderGetBoundsSizeTest, WithFloatArrow) {
+  CreateBubbleBorder(BubbleBorder::FLOAT);
+
+  ASSERT_FALSE(HasArrow());
+
+  SetPaintArrow(BubbleBorder::PAINT_NORMAL);
+  EXPECT_EQ(gfx::Size(22, 22), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 60), GetSize(kMediumSize));
+
+  SetPaintArrow(BubbleBorder::PAINT_TRANSPARENT);
+  EXPECT_EQ(gfx::Size(22, 22), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 60), GetSize(kMediumSize));
+
+  SetPaintArrow(BubbleBorder::PAINT_NONE);
+  EXPECT_EQ(gfx::Size(22, 22), GetSize(kSmallSize));
+  EXPECT_EQ(gfx::Size(60, 60), GetSize(kMediumSize));
+}
+
+class BubbleBorderGetBoundsOriginTest : public views::ViewsTestBase {
+ public:
+  const gfx::Rect kEmptyAnchor = gfx::Rect(100, 100, 0, 0);
+  const gfx::Rect kNonEmptyAnchor = gfx::Rect(100, 100, 20, 20);
+
+  gfx::Size kContentsSize = gfx::Size(50, 50);
+
+  BubbleBorderGetBoundsOriginTest() {}
+  virtual ~BubbleBorderGetBoundsOriginTest() {}
+
+  virtual void CreateBubbleBorder(BubbleBorder::Arrow arrow) {
+    bubble_border_.reset(new BubbleBorder(arrow,
+                                          BubbleBorder::NO_SHADOW,
+                                          0 /* color */));
+  }
+
+  virtual void SetAlignment(BubbleBorder::BubbleAlignment alignment) {
+    DCHECK(bubble_border_);
+    bubble_border_->set_alignment(alignment);
+  }
+
+  virtual gfx::Point GetOrigin(gfx::Rect anchor, gfx::Size contents_size) {

[msw, 2014/08/11 22:26:33]

nit: remove |contents_size| and send |kContentsSize| to GetBounds.

[bruthig, 2014/08/19 19:02:13]

Done.

+    DCHECK(bubble_border_);
+    return bubble_border_->GetBounds(anchor, contents_size).origin();
+  }
+
+ private:
+  scoped_ptr<BubbleBorder> bubble_border_;
+
+  DISALLOW_COPY_AND_ASSIGN(BubbleBorderGetBoundsOriginTest);
+};
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsHorizontalLeft) {
+  CreateBubbleBorder(BubbleBorder::TOP_LEFT);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(78, 96), GetOrigin(kEmptyAnchor, kContentsSize));

[msw, 2014/08/11 22:26:33]

Ditto; can you make this more robust with something like:
  const int kAnchorOffset = x;
  ...
  EXPECT_EQ(kAnchorOffset + kEmptyAnchor.x(), GetOrigin(kEmptyAnchor).x());

[bruthig, 2014/08/19 19:02:13]

Done.

+  EXPECT_EQ(gfx::Point(88, 116), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(96, 96), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(96, 116), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsHorizontalCenter) {
+  CreateBubbleBorder(BubbleBorder::TOP_CENTER);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(70, 96), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(80, 116), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(70, 96), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(80, 116), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsHorizontalRight) {
+  CreateBubbleBorder(BubbleBorder::TOP_RIGHT);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(62, 96), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(72, 116), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(44, 96), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(64, 116), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsHorizontalBottom) {
+  CreateBubbleBorder(BubbleBorder::BOTTOM_LEFT);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(78, 37), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(88, 37), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(96, 37), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(96, 37), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsVerticalTop) {
+  CreateBubbleBorder(BubbleBorder::LEFT_TOP);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(96, 78), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(116, 88), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(96, 96), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(116, 96), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsVerticalCenter) {
+  CreateBubbleBorder(BubbleBorder::LEFT_CENTER);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(96, 70), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(116, 80), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(96, 70), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(116, 80), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsVerticalBottom) {
+  CreateBubbleBorder(BubbleBorder::LEFT_BOTTOM);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(96, 62), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(116, 72), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(96, 44), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(116, 64), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsVerticalRight) {
+  CreateBubbleBorder(BubbleBorder::RIGHT_CENTER);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(37, 70), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(37, 80), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(37, 70), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(37, 80), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsNone) {
+  CreateBubbleBorder(BubbleBorder::NONE);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(70, 100), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(80, 120), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(70, 100), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(80, 120), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
+
+TEST_F(BubbleBorderGetBoundsOriginTest, ArrowIsFloat) {
+  CreateBubbleBorder(BubbleBorder::FLOAT);
+
+  SetAlignment(BubbleBorder::ALIGN_ARROW_TO_MID_ANCHOR);
+  EXPECT_EQ(gfx::Point(70, 70), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(80, 80), GetOrigin(kNonEmptyAnchor, kContentsSize));
+
+  SetAlignment(BubbleBorder::ALIGN_EDGE_TO_ANCHOR_EDGE);
+  EXPECT_EQ(gfx::Point(70, 70), GetOrigin(kEmptyAnchor, kContentsSize));
+  EXPECT_EQ(gfx::Point(80, 80), GetOrigin(kNonEmptyAnchor, kContentsSize));
+}
 
 }  // namespace views