This CL makes the InfoBubble not change the arrow when parts of it

is offscreen if it makes it even more offscreen.

BUG=None
TEST=Open the app launcher on ChromeOS. It should display correctly.

Review URL: http://codereview.chromium.org/2025008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@47053 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 359 insertions, 25 deletions

diff --git a/chrome/browser/views/bubble_border.h b/chrome/browser/views/bubble_border.h
index 753d1e6..c31d115 100644
--- a/chrome/browser/views/bubble_border.h
+++ b/chrome/browser/views/bubble_border.h
@@ -52,12 +52,13 @@ class BubbleBorder : public views::Border {
   void set_arrow_location(ArrowLocation arrow_location) {
     arrow_location_ = arrow_location;
   }
+  ArrowLocation arrow_location() const { return arrow_location_; }
 
-  static ArrowLocation rtl_mirror(ArrowLocation loc) {
+  static ArrowLocation horizontal_mirror(ArrowLocation loc) {
     return loc >= NONE ? loc : static_cast<ArrowLocation>(loc ^ 1);
   }
 
-  static ArrowLocation horizontal_mirror(ArrowLocation loc) {
+  static ArrowLocation vertical_mirror(ArrowLocation loc) {
     return loc >= NONE ? loc : static_cast<ArrowLocation>(loc ^ 2);
   }
 
diff --git a/chrome/browser/views/info_bubble.cc b/chrome/browser/views/info_bubble.cc
index 6c4aca1..d1414040 100644
--- a/chrome/browser/views/info_bubble.cc
+++ b/chrome/browser/views/info_bubble.cc
@@ -35,7 +35,7 @@ void BorderContents::Init() {
   // Default arrow location.
   BubbleBorder::ArrowLocation arrow_location = BubbleBorder::TOP_LEFT;
   if (base::i18n::IsRTL())
-    arrow_location = BubbleBorder::rtl_mirror(arrow_location);
+    arrow_location = BubbleBorder::horizontal_mirror(arrow_location);
   DCHECK(!bubble_border_);
   bubble_border_ = new BubbleBorder(arrow_location);
   set_border(bubble_border_);
@@ -50,7 +50,7 @@ void BorderContents::SizeAndGetBounds(
     gfx::Rect* contents_bounds,
     gfx::Rect* window_bounds) {
   if (base::i18n::IsRTL())
-    arrow_location = BubbleBorder::rtl_mirror(arrow_location);
+    arrow_location = BubbleBorder::horizontal_mirror(arrow_location);
   bubble_border_->set_arrow_location(arrow_location);
   // Set the border.
   set_border(bubble_border_);
@@ -64,28 +64,19 @@ void BorderContents::SizeAndGetBounds(
   // Try putting the arrow in its initial location, and calculating the bounds.
   *window_bounds =
       bubble_border_->GetBounds(position_relative_to, local_contents_size);
-
   if (!allow_bubble_offscreen) {
-    // See if those bounds will fit on the monitor.
-    scoped_ptr<WindowSizer::MonitorInfoProvider> monitor_provider(
-        WindowSizer::CreateDefaultMonitorInfoProvider());
-    gfx::Rect monitor_bounds(
-        monitor_provider->GetMonitorWorkAreaMatching(position_relative_to));
-    if (!monitor_bounds.IsEmpty() && !monitor_bounds.Contains(*window_bounds)) {
-      // The bounds don't fit.  Move the arrow to try and improve things.
-      if (window_bounds->bottom() > monitor_bounds.bottom())
-        arrow_location = BubbleBorder::horizontal_mirror(arrow_location);
-      else if (BubbleBorder::is_arrow_on_left(arrow_location) ?
-               (window_bounds->right() > monitor_bounds.right()) :
-               (window_bounds->x() < monitor_bounds.x())) {
-        arrow_location = BubbleBorder::rtl_mirror(arrow_location);
-      }
-
-      bubble_border_->set_arrow_location(arrow_location);
-
-      // Now get the recalculated bounds.
-      *window_bounds = bubble_border_->GetBounds(position_relative_to,
-                                                 local_contents_size);
+    gfx::Rect monitor_bounds = GetMonitorBounds(position_relative_to);
+    if (!monitor_bounds.IsEmpty()) {
+      // Try to resize vertically if this does not fit on the screen.
+      MirrorArrowIfOffScreen(true,  // |vertical|.
+                             position_relative_to, monitor_bounds,
+                             local_contents_size, &arrow_location,
+                             window_bounds);
+      // Then try to resize horizontally if it still does not fit on the screen.
+      MirrorArrowIfOffScreen(false,  // |vertical|.
+                             position_relative_to, monitor_bounds,
+                             local_contents_size, &arrow_location,
+                             window_bounds);
     }
   }
 
@@ -98,6 +89,13 @@ void BorderContents::SizeAndGetBounds(
       insets.right() + kRightMargin, insets.bottom() + kBottomMargin);
 }
 
+gfx::Rect BorderContents::GetMonitorBounds(const gfx::Rect& rect) {
+  scoped_ptr<WindowSizer::MonitorInfoProvider> monitor_provider(
+      WindowSizer::CreateDefaultMonitorInfoProvider());
+  return monitor_provider->GetMonitorWorkAreaMatching(rect);
+
+}
+
 void BorderContents::Paint(gfx::Canvas* canvas) {
   // The border of this view creates an anti-aliased round-rect region for the
   // contents, which we need to fill with the background color.
@@ -122,6 +120,75 @@ void BorderContents::Paint(gfx::Canvas* canvas) {
   PaintBorder(canvas);
 }
 
+void BorderContents::MirrorArrowIfOffScreen(
+    bool vertical,
+    const gfx::Rect& position_relative_to,
+    const gfx::Rect& monitor_bounds,
+    const gfx::Size& local_contents_size,
+    BubbleBorder::ArrowLocation* arrow_location,
+    gfx::Rect* window_bounds) {
+  // If the bounds don't fit, move the arrow to its mirrored position to see if
+  // it improves things.
+  gfx::Insets offscreen_insets;
+  if (ComputeOffScreenInsets(monitor_bounds, *window_bounds,
+                             &offscreen_insets) &&
+      GetInsetsLength(offscreen_insets, vertical) > 0) {
+    BubbleBorder::ArrowLocation original_arrow_location = *arrow_location;
+    *arrow_location =
+        vertical ? BubbleBorder::vertical_mirror(*arrow_location) :
+                   BubbleBorder::horizontal_mirror(*arrow_location);
+
+    // Change the arrow and get the new bounds.
+    bubble_border_->set_arrow_location(*arrow_location);
+    *window_bounds = bubble_border_->GetBounds(position_relative_to,
+                                               local_contents_size);
+    gfx::Insets new_offscreen_insets;
+    // If there is more of the window offscreen, we'll keep the old arrow.
+    if (ComputeOffScreenInsets(monitor_bounds, *window_bounds,
+                               &new_offscreen_insets) &&
+        GetInsetsLength(new_offscreen_insets, vertical) >=
+            GetInsetsLength(offscreen_insets, vertical)) {
+      *arrow_location = original_arrow_location;
+      bubble_border_->set_arrow_location(*arrow_location);
+      *window_bounds = bubble_border_->GetBounds(position_relative_to,
+                                                 local_contents_size);
+    }
+  }
+}
+
+// static
+bool BorderContents::ComputeOffScreenInsets(const gfx::Rect& monitor_bounds,
+                                            const gfx::Rect& window_bounds,
+                                            gfx::Insets* offscreen_insets) {
+  if (monitor_bounds.Contains(window_bounds))
+    return false;
+
+  if (!offscreen_insets)
+    return true;
+
+  int top = 0;
+  int left = 0;
+  int bottom = 0;
+  int right = 0;
+
+  if (window_bounds.y() < monitor_bounds.y())
+    top = monitor_bounds.y() - window_bounds.y();
+  if (window_bounds.x() < monitor_bounds.x())
+    left = monitor_bounds.x() - window_bounds.x();
+  if (window_bounds.bottom() > monitor_bounds.bottom())
+    bottom = window_bounds.bottom() - monitor_bounds.bottom();
+  if (window_bounds.right() > monitor_bounds.right())
+    right = window_bounds.right() - monitor_bounds.right();
+
+  offscreen_insets->Set(top, left, bottom, right);
+  return true;
+}
+
+// static
+int BorderContents::GetInsetsLength(const gfx::Insets& insets, bool vertical) {
+  return vertical ? insets.height() : insets.width();
+}
+
 #if defined(OS_WIN)
 // BorderWidget ---------------------------------------------------------------
 
diff --git a/chrome/browser/views/info_bubble.h b/chrome/browser/views/info_bubble.h
index f6d774d..6dad21f 100644
--- a/chrome/browser/views/info_bubble.h
+++ b/chrome/browser/views/info_bubble.h
@@ -63,6 +63,10 @@ class BorderContents : public views::View {
  protected:
   virtual ~BorderContents() { }
 
+  // Returns the bounds for the monitor showing the specified |rect|.
+  // Overriden in unit-tests.
+  virtual gfx::Rect GetMonitorBounds(const gfx::Rect& rect);
+
   // Margins between the contents and the inside of the border, in pixels.
   static const int kLeftMargin = 6;
   static const int kTopMargin = 6;
@@ -75,6 +79,28 @@ class BorderContents : public views::View {
   // Overridden from View:
   virtual void Paint(gfx::Canvas* canvas);
 
+  // Changes |arrow_location| to its mirrored version, vertically if |vertical|
+  // is true, horizontally otherwise, if |window_bounds| don't fit in
+  // |monitor_bounds|.
+  void MirrorArrowIfOffScreen(bool vertical,
+                              const gfx::Rect& position_relative_to,
+                              const gfx::Rect& monitor_bounds,
+                              const gfx::Size& local_contents_size,
+                              BubbleBorder::ArrowLocation* arrow_location,
+                              gfx::Rect* window_bounds);
+
+  // Computes how much |window_bounds| is off-screen of the monitor bounds
+  // |monitor_bounds| and puts the values in |offscreen_insets|.
+  // Returns false if |window_bounds| is actually contained in |monitor_bounds|,
+  // in which case |offscreen_insets| is not modified.
+  static bool ComputeOffScreenInsets(const gfx::Rect& monitor_bounds,
+                                     const gfx::Rect& window_bounds,
+                                     gfx::Insets* offscreen_insets);
+
+  // Convenience methods that returns the height of |insets| if |vertical| is
+  // true, its width otherwise.
+  static int GetInsetsLength(const gfx::Insets& insets, bool vertical);
+
   DISALLOW_COPY_AND_ASSIGN(BorderContents);
 };
 
diff --git a/chrome/browser/views/info_bubble_unittest.cc b/chrome/browser/views/info_bubble_unittest.cc
new file mode 100644
index 0000000..8b55422
--- /dev/null
+++ b/chrome/browser/views/info_bubble_unittest.cc
@@ -0,0 +1,234 @@
+// Copyright (c) 2010 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 "chrome/browser/views/info_bubble.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+typedef testing::Test InfoBubbleTest;
+
+class TestBorderContents : public BorderContents {
+ public:
+  TestBorderContents() {}
+
+  void set_monitor_bounds(const gfx::Rect& bounds) {
+    monitor_bounds_ = bounds;
+  }
+
+  BubbleBorder* bubble_border() const { return bubble_border_; }
+
+ protected:
+  virtual gfx::Rect GetMonitorBounds(const gfx::Rect& rect) {
+    return monitor_bounds_;
+  }
+
+ private:
+  gfx::Rect monitor_bounds_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestBorderContents);
+};
+
+// Tests that the arrow is moved appropriately when the info-bubble does not fit
+// the screen.
+TEST_F(InfoBubbleTest, BorderContentsSizeAndGetBounds) {
+  TestBorderContents border_contents;
+  border_contents.Init();
+
+  // Test that the info bubble displays normally when it fits.
+  gfx::Rect contents_bounds;
+  gfx::Rect window_bounds;
+  border_contents.set_monitor_bounds(gfx::Rect(0, 0, 1000, 1000));
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(100, 100, 50, 50),  // |position_relative_to|
+      BubbleBorder::TOP_LEFT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  // The arrow shouldn't have changed from TOP_LEFT.
+  BubbleBorder::ArrowLocation arrow_location =
+      border_contents.bubble_border()->arrow_location();
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_left(arrow_location));
+  EXPECT_GT(window_bounds.x(), 100);
+  EXPECT_GT(window_bounds.y(), 140);  // 140 because the arrow overlaps a bit on
+                                      // the position_relative_to.
+
+  // Test bubble not fitting on left.
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(100, 100, 50, 50),  // |position_relative_to|
+      BubbleBorder::TOP_RIGHT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  arrow_location = border_contents.bubble_border()->arrow_location();
+  // The arrow should have changed to TOP_LEFT.
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_left(arrow_location));
+  EXPECT_GT(window_bounds.x(), 100);
+  EXPECT_GT(window_bounds.y(), 140);
+
+  // Test bubble not fitting on left or top.
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(100, 100, 50, 50),  // |position_relative_to|
+      BubbleBorder::BOTTOM_RIGHT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  arrow_location = border_contents.bubble_border()->arrow_location();
+  // The arrow should have changed to TOP_LEFT.
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_left(arrow_location));
+  EXPECT_GT(window_bounds.x(), 100);
+  EXPECT_GT(window_bounds.y(), 140);
+
+  // Test bubble not fitting on top.
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(100, 100, 50, 50),  // |position_relative_to|
+      BubbleBorder::BOTTOM_LEFT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  arrow_location = border_contents.bubble_border()->arrow_location();
+  // The arrow should have changed to TOP_LEFT.
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_left(arrow_location));
+  EXPECT_GT(window_bounds.x(), 100);
+  EXPECT_GT(window_bounds.y(), 140);
+
+  // Test bubble not fitting on top and right.
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(900, 100, 50, 50),  // |position_relative_to|
+      BubbleBorder::BOTTOM_LEFT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  arrow_location = border_contents.bubble_border()->arrow_location();
+  // The arrow should have changed to TOP_RIGHT.
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_FALSE(BubbleBorder::is_arrow_on_left(arrow_location));
+  // The window should be right aligned with the position_relative_to.
+  EXPECT_LT(window_bounds.x(), 900 + 50 / 2 - 500);
+  EXPECT_GT(window_bounds.y(), 140);
+
+ // Test bubble not fitting on right.
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(900, 100, 50, 50),  // |position_relative_to|
+      BubbleBorder::TOP_LEFT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  arrow_location = border_contents.bubble_border()->arrow_location();
+  // The arrow should have changed to TOP_RIGHT.
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_FALSE(BubbleBorder::is_arrow_on_left(arrow_location));
+  // The window should be right aligned with the position_relative_to.
+  EXPECT_LT(window_bounds.x(), 900 + 50 / 2 - 500);
+  EXPECT_GT(window_bounds.y(), 140);
+
+  // Test bubble not fitting on bottom and right.
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(900, 900, 50, 50),  // |position_relative_to|
+      BubbleBorder::TOP_LEFT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  arrow_location = border_contents.bubble_border()->arrow_location();
+  // The arrow should have changed to BOTTOM_RIGHT.
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_FALSE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_FALSE(BubbleBorder::is_arrow_on_left(arrow_location));
+  // The window should be right aligned with the position_relative_to.
+  EXPECT_LT(window_bounds.x(), 900 + 50 / 2 - 500);
+  EXPECT_LT(window_bounds.y(), 900 - 500 - 15);  // -15 to roughly compensate
+                                                 // for arrow height.
+
+  // Test bubble not fitting at the bottom.
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(100, 900, 50, 50),  // |position_relative_to|
+      BubbleBorder::TOP_LEFT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  arrow_location = border_contents.bubble_border()->arrow_location();
+  // The arrow should have changed to BOTTOM_LEFT.
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_FALSE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_left(arrow_location));
+  // The window should be right aligned with the position_relative_to.
+  EXPECT_LT(window_bounds.x(), 900 + 50 / 2 - 500);
+  EXPECT_LT(window_bounds.y(), 900 - 500 - 15);  // -15 to roughly compensate
+                                                 // for arrow height.
+
+  // Test bubble not fitting at the bottom and left.
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(100, 900, 50, 50),  // |position_relative_to|
+      BubbleBorder::TOP_RIGHT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+  arrow_location = border_contents.bubble_border()->arrow_location();
+  // The arrow should have changed to BOTTOM_LEFT.
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_FALSE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_left(arrow_location));
+  // The window should be right aligned with the position_relative_to.
+  EXPECT_LT(window_bounds.x(), 900 + 50 / 2 - 500);
+  EXPECT_LT(window_bounds.y(), 900 - 500 - 15);  // -15 to roughly compensate
+                                                 // for arrow height.
+}
+
+// Tests that the arrow is not moved when the info-bubble does not fit the
+// screen but moving it would make matter worse.
+TEST_F(InfoBubbleTest, BorderContentsSizeAndGetBoundsDontMoveArrow) {
+  TestBorderContents border_contents;
+  border_contents.Init();
+  gfx::Rect contents_bounds;
+  gfx::Rect window_bounds;
+  border_contents.set_monitor_bounds(gfx::Rect(0, 0, 1000, 1000));
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(400, 100, 50, 50),  // |position_relative_to|
+      BubbleBorder::TOP_LEFT,
+      false,                        // |allow_bubble_offscreen|
+      gfx::Size(500, 700),          // |contents_size|
+      &contents_bounds, &window_bounds);
+
+  // The arrow should not have changed, as it would make it the bubble even more
+  // offscreen.
+  BubbleBorder::ArrowLocation arrow_location =
+      border_contents.bubble_border()->arrow_location();
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_left(arrow_location));
+}
+
+// Test that the 'allow offscreen' prevents the bubble from moving.
+TEST_F(InfoBubbleTest, BorderContentsSizeAndGetBoundsAllowOffscreen) {
+  TestBorderContents border_contents;
+  border_contents.Init();
+  gfx::Rect contents_bounds;
+  gfx::Rect window_bounds;
+  border_contents.set_monitor_bounds(gfx::Rect(0, 0, 1000, 1000));
+  border_contents.SizeAndGetBounds(
+      gfx::Rect(100, 900, 50, 50),  // |position_relative_to|
+      BubbleBorder::TOP_RIGHT,
+      true,                         // |allow_bubble_offscreen|
+      gfx::Size(500, 500),          // |contents_size|
+      &contents_bounds, &window_bounds);
+
+  // The arrow should not have changed (eventhough the bubble does not fit).
+  BubbleBorder::ArrowLocation arrow_location =
+      border_contents.bubble_border()->arrow_location();
+  EXPECT_TRUE(BubbleBorder::has_arrow(arrow_location));
+  EXPECT_TRUE(BubbleBorder::is_arrow_on_top(arrow_location));
+  EXPECT_FALSE(BubbleBorder::is_arrow_on_left(arrow_location));
+  // The coordinates should be pointing to 'positive relative to' from
+  // TOP_RIGHT.
+  EXPECT_LT(window_bounds.x(), 100 + 50 / 2 - 500);
+  EXPECT_GT(window_bounds.y(), 900 + 50 - 10);
+}
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 43f19e1..59c0059 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -952,6 +952,7 @@
         'browser/views/bookmark_editor_view_unittest.cc',
         'browser/views/extensions/browser_action_drag_data_unittest.cc',
         'browser/views/generic_info_view_unittest.cc',
+        'browser/views/info_bubble_unittest.cc',
         'browser/views/status_icons/status_tray_win_unittest.cc',
         'browser/visitedlink_unittest.cc',
         'browser/webdata/web_data_service_test_util.h',
@@ -1034,6 +1035,11 @@
             ['exclude', '^browser/chromeos'],
           ],
         }],
+        ['chromeos==0 and toolkit_views==0 and OS!="win"', {
+          'sources/': [
+             ['exclude', 'browser/views/info_bubble_unittest.cc'],
+          ]
+        }],
         ['OS=="linux" and selinux==0', {
           'dependencies': [
             '../sandbox/sandbox.gyp:*',