Consolidate bubble border code.

ui/views/bubble/bubble_frame_view.cc

Index: ui/views/bubble/bubble_frame_view.cc
diff --git a/ui/views/bubble/bubble_frame_view.cc b/ui/views/bubble/bubble_frame_view.cc
index 9ae1f8d5862799a76d3239555054824cbd076af1..2f9933a1e438f2acd113952a3a1e500aebb808cd 100644
--- a/ui/views/bubble/bubble_frame_view.cc
+++ b/ui/views/bubble/bubble_frame_view.cc
@@ -74,17 +74,22 @@ gfx::Size BubbleFrameView::GetPreferredSize() {
 
 gfx::Rect BubbleFrameView::GetUpdatedWindowBounds(const gfx::Rect& anchor_rect,
                                                   gfx::Size client_size,
-                                                  bool try_mirroring_arrow) {
+                                                  bool adjust_if_offscreen) {
   // Give the contents a margin.
   client_size.Enlarge(content_margins_.width(), content_margins_.height());
 
-  if (try_mirroring_arrow) {
-    // Try to mirror the anchoring if the bubble does not fit on the screen.
-    MirrorArrowIfOffScreen(true, anchor_rect, client_size);
-    MirrorArrowIfOffScreen(false, anchor_rect, client_size);
+  const BubbleBorder::ArrowLocation arrow = bubble_border_->arrow_location();
+  if (adjust_if_offscreen && BubbleBorder::has_arrow(arrow)) {
+    if (!bubble_border_->is_arrow_at_center(arrow)) {
+      // Try to mirror the anchoring if the bubble does not fit on the screen.
+      MirrorArrowIfOffScreen(true, anchor_rect, client_size);
+      MirrorArrowIfOffScreen(false, anchor_rect, client_size);
+    } else {
+      OffsetArrowIfOffScreen(anchor_rect, client_size);
+    }
   }
 
-  // Calculate the bounds with the arrow in its updated location.
+  // Calculate the bounds with the arrow in its updated location and offset.
   return bubble_border_->GetBounds(anchor_rect, client_size);
 }
 
@@ -124,4 +129,49 @@ void BubbleFrameView::MirrorArrowIfOffScreen(
   }
 }
 
+void BubbleFrameView::OffsetArrowIfOffScreen(const gfx::Rect& anchor_rect,
+                                             const gfx::Size& client_size) {
+  BubbleBorder::ArrowLocation arrow = bubble_border()->arrow_location();
+  DCHECK(BubbleBorder::has_arrow(arrow));
+
+  // Get the desired bubble bounds without adjustment.
+  bubble_border_->set_arrow_offset(0);
+  gfx::Rect window_bounds(bubble_border_->GetBounds(anchor_rect, client_size));
+
+  gfx::Rect monitor_rect(GetMonitorBounds(anchor_rect));
+  if (monitor_rect.IsEmpty() || monitor_rect.Contains(window_bounds))
+    return;
+
+  // Calculate off-screen adjustment.
+  const bool is_horizontal = BubbleBorder::is_arrow_on_horizontal(arrow);
+  int offscreen_adjust = 0;
+  if (is_horizontal) {
+    if (window_bounds.x() < monitor_rect.x())
+      offscreen_adjust = monitor_rect.x() - window_bounds.x();
+    else if (window_bounds.right() > monitor_rect.right())
+      offscreen_adjust = monitor_rect.right() - window_bounds.right();
+  } else {
+    if (window_bounds.y() < monitor_rect.y())
+      offscreen_adjust = monitor_rect.y() - window_bounds.y();
+    else if (window_bounds.bottom() > monitor_rect.bottom())
+      offscreen_adjust = monitor_rect.bottom() - window_bounds.bottom();
+  }
+
+  if (BubbleBorder::is_arrow_at_center(arrow) ||

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

After diagramming some cases I think |offscreen_adjust| should *always* be
negated; that tries to move the bubble onscreen, though some resulting offsets
will be clamped correctly by the border. If that's correct, please remove this
block and either flip the calculations above, or subtract |offscreen_adjust|
from the existing offset below.

[xiyuan, 2012/09/19 22:51:36]

Since we only run this function for center arrows, yes, it should always be
negated.

Before I was planning to use the function for right and bottom arrows, e.g.
TOP_RIGHT case. In this situation, if bubble goes off left screen edge,
|offscreen_adjust| is positive and we need to move arrow to left. But because
arrow offset is defined as distance from starting edge, which is right for
TOP_RIGHT case. We actually need to increase arrow offset, that is keep the
positive value and add to existing arrow offset.

[msw, 2012/09/20 00:07:23]

Ahhh, I mistakenly thought the offset always used canonical x/y signs, but
you're right, sorry. Feel free to revert for future-proofing, or leave as-is
since its sufficient for centered arrows, up to you.

[xiyuan, 2012/09/20 16:56:51]

I'll keep it as it is now. I have a DCHECK at the entry to make sure the logic
here only runs for center arrows. We could always add it back when needed later.

+      (is_horizontal && BubbleBorder::is_arrow_on_left(arrow)) ||
+      (!is_horizontal && BubbleBorder::is_arrow_on_top(arrow))) {
+    // For left/top/center arrow, arrow moves closer to starting edge for
+    // positive adjustment (i.e. negative arrow offset adjustment). Similarly,
+    // negative adjustment (bubble right edge offscreen) means arrow should
+    // move further from starting edge (i.e. positive arrow offset adjustment).
+    offscreen_adjust = -offscreen_adjust;
+  }
+
+  int arrow_offset = bubble_border_->GetArrowOffset(window_bounds.size()) +

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

nit: send the result to set_arrow_offset directly; nix local.

[xiyuan, 2012/09/19 22:51:36]

Done.

+      offscreen_adjust;
+  bubble_border_->set_arrow_offset(arrow_offset);
+  if (offscreen_adjust != 0)
+    SchedulePaint();
+}
+
 }  // namespace views