Consolidate bubble border code.

ui/views/bubble/bubble_border.cc

Index: ui/views/bubble/bubble_border.cc
diff --git a/ui/views/bubble/bubble_border.cc b/ui/views/bubble/bubble_border.cc
index 881f082369efbbf2a41259c80a8545da9c2b12e7..79da979cc0bda712c7491906ed5acdc0552b3518 100644
--- a/ui/views/bubble/bubble_border.cc
+++ b/ui/views/bubble/bubble_border.cc
@@ -29,7 +29,13 @@ struct BubbleBorder::BorderImages {
         top_arrow(NULL),
         right_arrow(NULL),
         bottom_arrow(NULL),
-        border_thickness(0) {
+        left_border_padding(0),
+        top_border_padding(0),
+        right_border_padding(0),
+        bottom_border_padding(0),
+        arrow_interior_height(0),
+        border_thickness(0),
+        corner_radius(0) {
   }
 
   gfx::ImageSkia* left;
@@ -40,39 +46,63 @@ struct BubbleBorder::BorderImages {
   gfx::ImageSkia* bottom_right;
   gfx::ImageSkia* bottom;
   gfx::ImageSkia* bottom_left;
+
   gfx::ImageSkia* left_arrow;
   gfx::ImageSkia* top_arrow;
   gfx::ImageSkia* right_arrow;
   gfx::ImageSkia* bottom_arrow;
+
+  // Offsets of where arrow tip pixel is in corresponding image assets.

[msw, 2012/09/18 19:33:42]

nit: consider "Location of the arrow tip pixels in corr..."

[xiyuan, 2012/09/19 18:20:09]

Done.

+  gfx::Point left_arrow_tip;

[msw, 2012/09/18 19:33:42]

We can derive the tip position from the image and |arrow_interior_height|.
Defining these points should not be necessary.

[xiyuan, 2012/09/19 18:20:09]

Done.

+  gfx::Point top_arrow_tip;
+  gfx::Point right_arrow_tip;
+  gfx::Point bottom_arrow_tip;
+
+  // Padding pixels between border pixels and image bounds.
+  int left_border_padding;

[msw, 2012/09/18 19:33:42]

We can derive the image padding from the image and |arrow_interior_height|.
Defining these sizes should not be necessary.

[xiyuan, 2012/09/19 18:20:09]

Done.

+  int top_border_padding;
+  int right_border_padding;
+  int bottom_border_padding;
+
+  int arrow_interior_height;  // The height inside the arrow image, in pixels.
   int border_thickness;
+  int corner_radius;
 };
 
 // static
-struct BubbleBorder::BorderImages* BubbleBorder::normal_images_ = NULL;
-struct BubbleBorder::BorderImages* BubbleBorder::shadow_images_ = NULL;
-
-// The height inside the arrow image, in pixels.
-static const int kArrowInteriorHeight = 7;
+struct BubbleBorder::BorderImages*
+BubbleBorder::border_images_[SHADOW_COUNT] = { NULL };
 
 BubbleBorder::BubbleBorder(ArrowLocation arrow_location, Shadow shadow)
     : override_arrow_offset_(0),
+      relative_arrow_offset_(0),
       arrow_location_(arrow_location),
       alignment_(ALIGN_ARROW_TO_MID_ANCHOR),
-      background_color_(SK_ColorWHITE) {
+      background_color_(SK_ColorWHITE),
+      center_arrow_(false) {
+  DCHECK(shadow < SHADOW_COUNT);
   images_ = GetBorderImages(shadow);
 
   // Calculate horizontal and vertical insets for arrow by ensuring that

[msw, 2012/09/18 19:33:42]

Should this be moved into CalculateArrowOffset?

[xiyuan, 2012/09/19 18:20:09]

|arrow_offset_| is needed to get minimum bubble size in Getbounds. We might want
to keep it here to set |arrow_offset_|.

[msw, 2012/09/19 20:12:31]

ok.

   // the widest arrow and corner images will have enough room to avoid overlap
   int offset_x =
-      (std::max(images_->top_arrow->width(),
-                images_->bottom_arrow->width()) / 2) +
+      std::max(std::max(images_->top_arrow_tip.x(),

[msw, 2012/09/18 19:33:42]

It seems like you could assume that the arrow images are symmetrical with the
arrow in the center and avoid the new std::max you're adding here.

[xiyuan, 2012/09/19 18:20:09]

Done.

+                        images_->top_arrow->width() -
+                            images_->top_arrow_tip.x()),
+               std::max(images_->bottom_arrow_tip.x(),
+                        images_->bottom_arrow->width() -
+                            images_->bottom_arrow_tip.x())) +
       std::max(std::max(images_->top_left->width(),
                         images_->top_right->width()),
                std::max(images_->bottom_left->width(),
                         images_->bottom_right->width()));
   int offset_y =
-      (std::max(images_->left_arrow->height(),
-                images_->right_arrow->height()) / 2) +
+      std::max(std::max(images_->left_arrow_tip.y(),
+                        images_->left_arrow->height() -
+                            images_->left_arrow_tip.y()),
+               std::max(images_->right_arrow_tip.y(),
+                        images_->right_arrow->height() -
+                            images_->right_arrow_tip.y())) +
       std::max(std::max(images_->top_left->height(),
                         images_->top_right->height()),
                std::max(images_->bottom_left->height(),
@@ -95,39 +125,39 @@ gfx::Rect BubbleBorder::GetBounds(const gfx::Rect& position_relative_to,
     border_size.set_height(std::max(border_size.height(), 2 * arrow_offset_));
 
   // Screen position depends on the arrow location.
-  // The arrow should overlap the target by some amount since there is space
-  // for shadow between arrow tip and image bounds.
-  const int kArrowOverlap = 3;
+  // The bubble should overlap the target by some amount since there is space
+  // for shadow between arrow tip/bubble border and image bounds.
   int x = position_relative_to.x();
   int y = position_relative_to.y();
   int w = position_relative_to.width();
   int h = position_relative_to.height();
-  int arrow_offset = override_arrow_offset_ ? override_arrow_offset_ :
-                                              arrow_offset_;
+
+  const int arrow_offset = CalculateArrowOffset(border_size);
 
   // Calculate bubble x coordinate.
   switch (arrow_location_) {
     case TOP_LEFT:
     case BOTTOM_LEFT:
       x += alignment_ == ALIGN_ARROW_TO_MID_ANCHOR ? w / 2 - arrow_offset :
-           -kArrowOverlap;
+           -images_->left_border_padding;
       break;
 
     case TOP_RIGHT:
     case BOTTOM_RIGHT:
       x += alignment_ == ALIGN_ARROW_TO_MID_ANCHOR ?
           w / 2 + arrow_offset - border_size.width() + 1 :
-          w - border_size.width() + kArrowOverlap;
+          w - border_size.width() + images_->right_border_padding;
       break;
 
     case LEFT_TOP:
     case LEFT_BOTTOM:
-      x += w - kArrowOverlap;
+      x += w - images_->left_arrow_tip.x();
       break;
 
     case RIGHT_TOP:
     case RIGHT_BOTTOM:
-      x += kArrowOverlap - border_size.width();
+      x += images_->right_arrow->width() - images_->right_arrow_tip.x() -
+           border_size.width();
       break;
 
     case NONE:
@@ -140,25 +170,26 @@ gfx::Rect BubbleBorder::GetBounds(const gfx::Rect& position_relative_to,
   switch (arrow_location_) {
     case TOP_LEFT:
     case TOP_RIGHT:
-      y += h - kArrowOverlap;
+      y += h - images_->top_arrow_tip.y();
       break;
 
     case BOTTOM_LEFT:
     case BOTTOM_RIGHT:
-      y += kArrowOverlap - border_size.height();
+      y += images_->bottom_arrow->height() - images_->bottom_arrow_tip.y() -
+          border_size.height();
       break;
 
     case LEFT_TOP:
     case RIGHT_TOP:
       y += alignment_ == ALIGN_ARROW_TO_MID_ANCHOR ? h / 2 - arrow_offset :
-           -kArrowOverlap;
+           -images_->top_border_padding;
       break;
 
     case LEFT_BOTTOM:
     case RIGHT_BOTTOM:
       y += alignment_ == ALIGN_ARROW_TO_MID_ANCHOR ?
           h / 2 + arrow_offset - border_size.height() + 1 :
-          h - border_size.height() + kArrowOverlap;
+          h - border_size.height() + images_->bottom_border_padding;
       break;
 
     case NONE:
@@ -216,55 +247,156 @@ int BubbleBorder::GetBorderThickness() const {
   return images_->border_thickness;
 }
 
+int BubbleBorder::GetBorderCornerRadius() const {
+  return images_->corner_radius;
+}
+
 int BubbleBorder::SetArrowOffset(int offset, const gfx::Size& contents_size) {
   gfx::Size border_size(contents_size);
   gfx::Insets insets;
   GetInsets(&insets);
   border_size.Enlarge(insets.left() + insets.right(),
                       insets.top() + insets.bottom());
-  offset = std::max(arrow_offset_,
-      std::min(offset, (is_arrow_on_horizontal(arrow_location_) ?
-          border_size.width() : border_size.height()) - arrow_offset_));
+  offset = SanitizeArrowOffset(offset, border_size);

[msw, 2012/09/18 19:33:42]

Nix this, its redundant with the call in CalculateArrowOffset.

[xiyuan, 2012/09/19 18:20:09]

Done. And made SetArrowOffset an inline accessor.

[msw, 2012/09/19 20:12:31]

Nice.

   override_arrow_offset_ = offset;
   return override_arrow_offset_;
 }
 
 // static
 BubbleBorder::BorderImages* BubbleBorder::GetBorderImages(Shadow shadow) {
-  if (shadow == SHADOW && shadow_images_ == NULL) {
-    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-    shadow_images_ = new BorderImages();
-    shadow_images_->left = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_L);
-    shadow_images_->top_left = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_TL);
-    shadow_images_->top = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_T);
-    shadow_images_->top_right = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_TR);
-    shadow_images_->right = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_R);
-    shadow_images_->bottom_right = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_BR);
-    shadow_images_->bottom = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_B);
-    shadow_images_->bottom_left = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_BL);
-    shadow_images_->left_arrow = new gfx::ImageSkia();
-    shadow_images_->top_arrow = new gfx::ImageSkia();
-    shadow_images_->right_arrow = new gfx::ImageSkia();
-    shadow_images_->bottom_arrow = new gfx::ImageSkia();
-    shadow_images_->border_thickness = 10;
-  } else if (shadow == NO_SHADOW && normal_images_ == NULL) {
+  DCHECK(shadow < SHADOW_COUNT);
+
+  struct BorderImages*& images = border_images_[shadow];
+  if (images == NULL) {

[msw, 2012/09/18 19:33:42]

Can you invert this check to make it an early return?
Smaller indenting below might make more one-line statements.

[xiyuan, 2012/09/19 18:20:09]

Done.

+    images = new BorderImages();
+
     ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-    normal_images_ = new BorderImages();
-    normal_images_->left = rb.GetImageSkiaNamed(IDR_BUBBLE_L);
-    normal_images_->top_left = rb.GetImageSkiaNamed(IDR_BUBBLE_TL);
-    normal_images_->top = rb.GetImageSkiaNamed(IDR_BUBBLE_T);
-    normal_images_->top_right = rb.GetImageSkiaNamed(IDR_BUBBLE_TR);
-    normal_images_->right = rb.GetImageSkiaNamed(IDR_BUBBLE_R);
-    normal_images_->bottom_right = rb.GetImageSkiaNamed(IDR_BUBBLE_BR);
-    normal_images_->bottom = rb.GetImageSkiaNamed(IDR_BUBBLE_B);
-    normal_images_->bottom_left = rb.GetImageSkiaNamed(IDR_BUBBLE_BL);
-    normal_images_->left_arrow = rb.GetImageSkiaNamed(IDR_BUBBLE_L_ARROW);
-    normal_images_->top_arrow = rb.GetImageSkiaNamed(IDR_BUBBLE_T_ARROW);
-    normal_images_->right_arrow = rb.GetImageSkiaNamed(IDR_BUBBLE_R_ARROW);
-    normal_images_->bottom_arrow = rb.GetImageSkiaNamed(IDR_BUBBLE_B_ARROW);
-    normal_images_->border_thickness = 0;
+    switch (shadow) {

[msw, 2012/09/18 19:33:42]

This is getting unwieldy, perhaps BorderImages needs a ctor?
Also, consider defining a static array of IDR arrays or similar.

[xiyuan, 2012/09/19 18:20:09]

Done. Added IDR arrays and pass it to BorderImage's ctor to load image
resources.

Change resource loading function from GetImageSkiaNamed to
GetImageName().ToImageSkia because the later is the preferred way.

Also changed ImageSkia member from pointer to object. Assigning ImageSkia is
cheap similar to SkBitmap, it is a add ref op on its internal storage.

[msw, 2012/09/19 20:12:31]

All excellent (though GetImageSkiaNamed was probably ok).

+      case SHADOW:
+        images->left = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_L);
+        images->top_left = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_TL);
+        images->top = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_T);
+        images->top_right = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_TR);
+        images->right = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_R);
+        images->bottom_right = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_BR);
+        images->bottom = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_B);
+        images->bottom_left = rb.GetImageSkiaNamed(IDR_BUBBLE_SHADOW_BL);
+        images->left_arrow = new gfx::ImageSkia();

[msw, 2012/09/18 19:33:42]

These seem to be leaked (prior to this CL, not your fault).
Please add a TODO (for yourself, the original author, or me).

[xiyuan, 2012/09/19 18:20:09]

Resolved as we use ImageSkia object instead of pointer and no longer allocates
these dummy ImageSkias.

[msw, 2012/09/19 20:12:31]

Great!

+        images->top_arrow = new gfx::ImageSkia();
+        images->right_arrow = new gfx::ImageSkia();
+        images->bottom_arrow = new gfx::ImageSkia();
+        images->arrow_interior_height = 7;

[msw, 2012/09/18 19:33:42]

This group has no arrow, why is arrow_interior_height 7?

[xiyuan, 2012/09/19 18:20:09]

Removed since it's not used and we should be okay with default 0.

+        images->border_thickness = 10;
+        images->corner_radius = 4;
+        break;
+      case NO_SHADOW:
+        images->left = rb.GetImageSkiaNamed(IDR_BUBBLE_L);
+        images->top_left = rb.GetImageSkiaNamed(IDR_BUBBLE_TL);
+        images->top = rb.GetImageSkiaNamed(IDR_BUBBLE_T);
+        images->top_right = rb.GetImageSkiaNamed(IDR_BUBBLE_TR);
+        images->right = rb.GetImageSkiaNamed(IDR_BUBBLE_R);
+        images->bottom_right = rb.GetImageSkiaNamed(IDR_BUBBLE_BR);
+        images->bottom = rb.GetImageSkiaNamed(IDR_BUBBLE_B);
+        images->bottom_left = rb.GetImageSkiaNamed(IDR_BUBBLE_BL);
+        images->left_arrow = rb.GetImageSkiaNamed(IDR_BUBBLE_L_ARROW);
+        images->top_arrow = rb.GetImageSkiaNamed(IDR_BUBBLE_T_ARROW);
+        images->right_arrow = rb.GetImageSkiaNamed(IDR_BUBBLE_R_ARROW);
+        images->bottom_arrow = rb.GetImageSkiaNamed(IDR_BUBBLE_B_ARROW);
+        images->left_arrow_tip = gfx::Point(3, 11);
+        images->top_arrow_tip = gfx::Point(9, 3);
+        images->right_arrow_tip = gfx::Point(9, 11);
+        images->bottom_arrow_tip = gfx::Point(9, 9);
+        images->left_border_padding = 3;
+        images->top_border_padding = 1;
+        images->right_border_padding = 3;
+        images->bottom_border_padding = 5;
+        images->arrow_interior_height = 7;
+        images->border_thickness = 0;
+        images->corner_radius = 4;
+        break;
+#if defined(USE_AURA)
+      case BIG_SHADOW:
+        images->left =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_BIG_LEFT);
+        images->top_left =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_BIG_TOP_LEFT);
+        images->top =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_BIG_TOP);
+        images->top_right =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_BIG_TOP_RIGHT);
+        images->right =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_BIG_RIGHT);
+        images->bottom_right =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_BIG_BOTTOM_RIGHT);
+        images->bottom =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_BIG_BOTTOM);
+        images->bottom_left =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_BIG_BOTTOM_LEFT);
+        images->left_arrow =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SPIKE_BIG_LEFT);
+        images->top_arrow =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SPIKE_BIG_TOP);
+        images->right_arrow =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SPIKE_BIG_RIGHT);
+        images->bottom_arrow =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SPIKE_BIG_BOTTOM);
+        images->left_arrow_tip = gfx::Point(20, 16);
+        images->top_arrow_tip = gfx::Point(21, 20);
+        images->right_arrow_tip = gfx::Point(10, 16);
+        images->bottom_arrow_tip = gfx::Point(21, 10);
+        images->left_border_padding = 30;
+        images->top_border_padding = 30;
+        images->right_border_padding = 30;
+        images->bottom_border_padding = 30;
+        images->arrow_interior_height = 9;
+        images->border_thickness = 0;
+        images->corner_radius = 3;
+        break;
+      case SMALL_SHADOW:
+        images->left =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SMALL_LEFT);
+        images->top_left =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SMALL_TOP_LEFT);
+        images->top =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SMALL_TOP);
+        images->top_right =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SMALL_TOP_RIGHT);
+        images->right =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SMALL_RIGHT);
+        images->bottom_right =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SMALL_BOTTOM_RIGHT);
+        images->bottom =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SMALL_BOTTOM);
+        images->bottom_left =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SMALL_BOTTOM_LEFT);
+        images->left_arrow =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SPIKE_SMALL_LEFT);
+        images->top_arrow =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SPIKE_SMALL_TOP);
+        images->right_arrow =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SPIKE_SMALL_RIGHT);
+        images->bottom_arrow =
+            rb.GetImageSkiaNamed(IDR_WINDOW_BUBBLE_SHADOW_SPIKE_SMALL_BOTTOM);
+        images->left_arrow_tip = gfx::Point(5, 9);
+        images->top_arrow_tip = gfx::Point(12, 5);
+        images->right_arrow_tip = gfx::Point(10, 9);
+        images->bottom_arrow_tip = gfx::Point(12, 10);
+        images->left_border_padding = 15;
+        images->top_border_padding = 15;
+        images->right_border_padding = 15;
+        images->bottom_border_padding = 15;
+        images->arrow_interior_height = 9;
+        images->border_thickness = 0;
+        images->corner_radius = 3;
+        break;
+#endif
+      case SHADOW_COUNT:

[msw, 2012/09/18 19:33:42]

Make this (or add a similar) default: case.

[xiyuan, 2012/09/19 18:20:09]

Done.

I found not using "default" for enum based switch has a benefit to catch missing
values at compile time. But I am okay either way.

[msw, 2012/09/19 20:12:31]

Good point, feel free to switch it back; thanks for the tip!

+        NOTREACHED();
+        break;
+    }
   }
-  return shadow == SHADOW ? shadow_images_ : normal_images_;
+
+  return images;
 }
 
 BubbleBorder::~BubbleBorder() {}
@@ -293,12 +425,10 @@ void BubbleBorder::Paint(const views::View& view, gfx::Canvas* canvas) const {
   const int height = bottom - top;
   const int width = right - left;
 
-  // |arrow_offset| is offset of arrow from the begining of the edge.
-  int arrow_offset = arrow_offset_;
-  if (override_arrow_offset_)
-    arrow_offset = override_arrow_offset_;
-  else if (is_arrow_on_horizontal(arrow_location_) &&
-           !is_arrow_on_left(arrow_location_)) {
+  // |arrow_offset| is offset of arrow from the beginning of the edge.
+  int arrow_offset = CalculateArrowOffset(view.size());
+  if (is_arrow_on_horizontal(arrow_location_) &&
+     !is_arrow_on_left(arrow_location_)) {
     arrow_offset = view.width() - arrow_offset - 1;
   } else if (!is_arrow_on_horizontal(arrow_location_) &&
              !is_arrow_on_top(arrow_location_)) {
@@ -309,13 +439,13 @@ void BubbleBorder::Paint(const views::View& view, gfx::Canvas* canvas) const {
   if (arrow_location_ == LEFT_TOP || arrow_location_ == LEFT_BOTTOM) {
     int start_y = top + tl_height;
     int before_arrow =
-        arrow_offset - start_y - images_->left_arrow->height() / 2;
+        arrow_offset - start_y - images_->left_arrow_tip.y();
     int after_arrow = height - tl_height - bl_height -
         images_->left_arrow->height() - before_arrow;
     // Shift tip coordinates half pixel so that skia draws the tip correctly.
     DrawArrowInterior(canvas,
-        images_->left_arrow->width() - kArrowInteriorHeight - 0.5f,
-        start_y + before_arrow + images_->left_arrow->height() / 2 - 0.5f);
+        images_->left_arrow->width() - images_->arrow_interior_height - 0.5f,
+        start_y + before_arrow + images_->left_arrow_tip.y() - 0.5f);
     DrawEdgeWithArrow(canvas,
                       false,
                       images_->left,
@@ -336,12 +466,12 @@ void BubbleBorder::Paint(const views::View& view, gfx::Canvas* canvas) const {
   // Top edge.
   if (arrow_location_ == TOP_LEFT || arrow_location_ == TOP_RIGHT) {
     int start_x = left + tl_width;
-    int before_arrow = arrow_offset - start_x - images_->top_arrow->width() / 2;
+    int before_arrow = arrow_offset - start_x - images_->top_arrow_tip.x();
     int after_arrow = width - tl_width - tr_width -
         images_->top_arrow->width() - before_arrow;
     DrawArrowInterior(canvas,
-        start_x + before_arrow + images_->top_arrow->width() / 2,
-        images_->top_arrow->height() - kArrowInteriorHeight);
+        start_x + before_arrow + images_->top_arrow_tip.x(),
+        images_->top_arrow->height() - images_->arrow_interior_height);
     DrawEdgeWithArrow(canvas,
                       true,
                       images_->top,
@@ -363,13 +493,13 @@ void BubbleBorder::Paint(const views::View& view, gfx::Canvas* canvas) const {
   if (arrow_location_ == RIGHT_TOP || arrow_location_ == RIGHT_BOTTOM) {
     int start_y = top + tr_height;
     int before_arrow =
-        arrow_offset - start_y - images_->right_arrow->height() / 2;
+        arrow_offset - start_y - images_->right_arrow_tip.y();
     int after_arrow = height - tl_height - bl_height -
         images_->right_arrow->height() - before_arrow;
     // Shift tip coordinates half pixel so that skia draws the tip correctly.
     DrawArrowInterior(canvas,
-        right - r_width + kArrowInteriorHeight - 0.5f,
-        start_y + before_arrow + images_->right_arrow->height() / 2 - 0.5f);
+        right - r_width + images_->arrow_interior_height - 0.5f,
+        start_y + before_arrow + images_->right_arrow_tip.y() - 0.5f);
     DrawEdgeWithArrow(canvas,
                       false,
                       images_->right,
@@ -393,12 +523,12 @@ void BubbleBorder::Paint(const views::View& view, gfx::Canvas* canvas) const {
   if (arrow_location_ == BOTTOM_LEFT || arrow_location_ == BOTTOM_RIGHT) {
     int start_x = left + bl_width;
     int before_arrow =
-        arrow_offset - start_x - images_->bottom_arrow->width() / 2;
+        arrow_offset - start_x - images_->bottom_arrow_tip.x();
     int after_arrow = width - bl_width - br_width -
         images_->bottom_arrow->width() - before_arrow;
     DrawArrowInterior(canvas,
-        start_x + before_arrow + images_->bottom_arrow->width() / 2,
-        bottom - b_height + kArrowInteriorHeight);
+        start_x + before_arrow + images_->bottom_arrow_tip.x(),
+        bottom - b_height + images_->arrow_interior_height);
     DrawEdgeWithArrow(canvas,
                       true,
                       images_->bottom,
@@ -463,7 +593,7 @@ void BubbleBorder::DrawArrowInterior(gfx::Canvas* canvas,
   const bool positive_offset = is_horizontal ?
       is_arrow_on_top(arrow_location_) : is_arrow_on_left(arrow_location_);
   const int offset_to_next_vertex = positive_offset ?
-      kArrowInteriorHeight : -kArrowInteriorHeight;
+      images_->arrow_interior_height : -images_->arrow_interior_height;
 
   SkPath path;
   path.incReserve(4);
@@ -486,6 +616,28 @@ void BubbleBorder::DrawArrowInterior(gfx::Canvas* canvas,
   canvas->DrawPath(path, paint);
 }
 
+int BubbleBorder::SanitizeArrowOffset(int arrow_offset,

[msw, 2012/09/18 19:33:42]

After removing the call in SetArrowOffset, inline this in CalculateArrowOffset.

[xiyuan, 2012/09/19 18:20:09]

Done.

+                                      const gfx::Size& border_size) const {
+  return std::max(arrow_offset_,
+      std::min(arrow_offset, (is_arrow_on_horizontal(arrow_location_) ?
+          border_size.width() : border_size.height()) - arrow_offset_));
+}
+
+int BubbleBorder::CalculateArrowOffset(const gfx::Size& border_size) const {
+  int arrow_offset = arrow_offset_;
+  if (override_arrow_offset_)

[msw, 2012/09/18 19:33:42]

Add braces here to match the else case.

[xiyuan, 2012/09/19 18:20:09]

Done.

+    arrow_offset = override_arrow_offset_;
+  else if (center_arrow_) {
+    if (is_arrow_on_horizontal(arrow_location_))
+      arrow_offset = border_size.width() / 2;
+    else if (has_arrow(arrow_location_))
+      arrow_offset = border_size.height() / 2;
+  }
+
+  arrow_offset += relative_arrow_offset_;
+  return SanitizeArrowOffset(arrow_offset, border_size);
+}
+
 /////////////////////////
 
 void BubbleBackground::Paint(gfx::Canvas* canvas, views::View* view) const {
@@ -506,7 +658,7 @@ void BubbleBackground::Paint(gfx::Canvas* canvas, views::View* view) const {
   SkPath path;
   gfx::Rect bounds(view->GetContentsBounds());
   bounds.Inset(-border_->GetBorderThickness(), -border_->GetBorderThickness());
-  SkScalar radius = SkIntToScalar(BubbleBorder::GetCornerRadius());
+  SkScalar radius = SkIntToScalar(border_->GetBorderCornerRadius());
   path.addRoundRect(gfx::RectToSkRect(bounds), radius, radius);
   canvas->DrawPath(path, paint);
 }