Consolidate bubble border code.

ui/views/bubble/bubble_border.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 <algorithm>  // for std::max
 
 #include "base/logging.h"
 #include "grit/ui_resources.h"
@@ skipping 11 matching lines @@
         top(NULL),
         top_right(NULL),
         right(NULL),
         bottom_right(NULL),
         bottom(NULL),
         bottom_left(NULL),
         left_arrow(NULL),
         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;
   gfx::ImageSkia* top_left;
   gfx::ImageSkia* top;
   gfx::ImageSkia* top_right;
   gfx::ImageSkia* right;
   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(),
                         images_->bottom_right->height()));
   arrow_offset_ = std::max(offset_x, offset_y);
 }
 
 gfx::Rect BubbleBorder::GetBounds(const gfx::Rect& position_relative_to,
                                   const gfx::Size& contents_size) const {
   // Desired size is size of contents enlarged by the size of the border images.
   gfx::Size border_size(contents_size);
   gfx::Insets insets;
   GetInsets(&insets);
   border_size.Enlarge(insets.width(), insets.height());
 
   // Ensure the bubble has a minimum size that draws arrows correctly.
   if (is_arrow_on_horizontal(arrow_location_))
     border_size.set_width(std::max(border_size.width(), 2 * arrow_offset_));
   else if (has_arrow(arrow_location_))
     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:
     case FLOAT:
       x += w / 2 - border_size.width() / 2;
       break;
   }
 
   // Calculate bubble y coordinate.
   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:
       y += h;
       break;
 
     case FLOAT:
       y += h / 2 - border_size.height() / 2;
       break;
   }
@@ skipping 37 matching lines @@
       // Nothing to do.
       break;
   }
   insets->Set(top, left, bottom, right);
 }
 
 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) {
+  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();
-    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) {
-    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() {}
 
 void BubbleBorder::Paint(const views::View& view, gfx::Canvas* canvas) const {
   // Convenience shorthand variables.
   const int tl_width = images_->top_left->width();
   const int tl_height = images_->top_left->height();
   const int t_height = images_->top->height();
   const int tr_width = images_->top_right->width();
   const int tr_height = images_->top_right->height();
   const int l_width = images_->left->width();
   const int r_width = images_->right->width();
   const int br_width = images_->bottom_right->width();
   const int br_height = images_->bottom_right->height();
   const int b_height = images_->bottom->height();
   const int bl_width = images_->bottom_left->width();
   const int bl_height = images_->bottom_left->height();
 
   gfx::Insets insets;
   GetInsets(&insets);
   const int top = insets.top() - t_height;
   const int bottom = view.height() - insets.bottom() + b_height;
   const int left = insets.left() - l_width;
   const int right = view.width() - insets.right() + r_width;
   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_)) {
     arrow_offset = view.height() - arrow_offset - 1;
   }
 
   // Left edge.
   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,
                       images_->left_arrow,
                       left,
                       start_y,
                       before_arrow,
                       after_arrow,
                       images_->left->width() - images_->left_arrow->width());
   } else {
     canvas->TileImageInt(*images_->left, left, top + tl_height, l_width,
                          height - tl_height - bl_height);
   }
 
   // Top left corner.
   canvas->DrawImageInt(*images_->top_left, left, top);
 
   // 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,
                       images_->top_arrow,
                       start_x,
                       top,
                       before_arrow,
                       after_arrow,
                       images_->top->height() - images_->top_arrow->height());
   } else {
     canvas->TileImageInt(*images_->top, left + tl_width, top,
                          width - tl_width - tr_width, t_height);
   }
 
   // Top right corner.
   canvas->DrawImageInt(*images_->top_right, right - tr_width, top);
 
   // Right edge.
   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,
                       images_->right_arrow,
                       right - r_width,
                       start_y,
                       before_arrow,
                       after_arrow,
                       0);
   } else {
     canvas->TileImageInt(*images_->right, right - r_width, top + tr_height,
                          r_width, height - tr_height - br_height);
   }
 
   // Bottom right corner.
   canvas->DrawImageInt(*images_->bottom_right,
                        right - br_width,
                        bottom - br_height);
 
   // Bottom edge.
   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,
                       images_->bottom_arrow,
                       start_x,
                       bottom - b_height,
                       before_arrow,
                       after_arrow,
                       0);
   } else {
@@ skipping 44 matching lines @@
   }
 }
 
 void BubbleBorder::DrawArrowInterior(gfx::Canvas* canvas,
                                      float tip_x,
                                      float tip_y) const {
   const bool is_horizontal = is_arrow_on_horizontal(arrow_location_);
   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);
   path.moveTo(SkDoubleToScalar(tip_x), SkDoubleToScalar(tip_y));
   path.lineTo(SkDoubleToScalar(tip_x + offset_to_next_vertex),
               SkDoubleToScalar(tip_y + offset_to_next_vertex));
   if (is_horizontal) {
     path.lineTo(SkDoubleToScalar(tip_x - offset_to_next_vertex),
                 SkDoubleToScalar(tip_y + offset_to_next_vertex));
   } else {
     path.lineTo(SkDoubleToScalar(tip_x + offset_to_next_vertex),
                 SkDoubleToScalar(tip_y - offset_to_next_vertex));
   }
   path.close();
 
   SkPaint paint;
   paint.setStyle(SkPaint::kFill_Style);
   paint.setColor(background_color_);
 
   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 {
   // Clip out the client bounds to prevent overlapping transparent widgets.
   if (!border_->client_bounds().IsEmpty()) {
     SkRect client_rect(gfx::RectToSkRect(border_->client_bounds()));
     canvas->sk_canvas()->clipRect(client_rect, SkRegion::kDifference_Op);
   }
 
   // The border of this view creates an anti-aliased round-rect region for the
   // contents, which we need to fill with the background color.
   // NOTE: This doesn't handle an arrow location of "NONE", which has square top
   // corners.
   SkPaint paint;
   paint.setAntiAlias(true);
   paint.setStyle(SkPaint::kFill_Style);
   paint.setColor(border_->background_color());
   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);
 }
 
 }  // namespace views