[Mac] 1) Status bubble respects page contents view bounds, not just its vertical size; 2) Bubble expansion preserves bubble's current position; 3) Bubble won't expand if it's going to cover current mouse position in the expanded state.

BUG=54882,56434
TEST=All usual status bubble tests (mouse evasion, fullscreen mode, devtools and download panel interaction)
plus the same with sidebar open.
Status bubble should behave as expected plus it should not cover sidebar content in mouse or screen bottom evading mode.

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

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

6 files changed, 136 insertions, 49 deletions

diff --git a/chrome/browser/cocoa/browser_window_controller.h b/chrome/browser/cocoa/browser_window_controller.h
index 6869397..f0b3363 100644
--- a/chrome/browser/cocoa/browser_window_controller.h
+++ b/chrome/browser/cocoa/browser_window_controller.h
@@ -218,8 +218,8 @@ class TabContents;
 // "chrome/app/chrome_dll_resource.h" file.
 - (void)executeCommand:(int)command;
 
-// Delegate method for the status bubble to query about its vertical offset.
-- (CGFloat)verticalOffsetForStatusBubble;
+// Delegate method for the status bubble to query its base frame.
+- (NSRect)statusBubbleBaseFrame;
 
 // Show the bookmark bubble (e.g. user just clicked on the STAR)
 - (void)showBookmarkBubbleForURL:(const GURL&)url
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index bdec3b6..d33c0ca 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -1013,15 +1013,14 @@
     browser_->ExecuteCommand(command);
 }
 
-// StatusBubble delegate method: tell the status bubble how far above the bottom
-// of the window it should position itself.
-// TODO(alekseys): status bubble should respect web view bounds, not just its
-// vertical size. Now it can overlap sidebar contents. http://crbug.com/54882
-- (CGFloat)verticalOffsetForStatusBubble {
-  NSRect contents_bounds = [[sidebarController_ view] bounds];
-  NSView* baseView = [[self window] contentView];
-  return NSMinY([baseView convertRect:contents_bounds
-                             fromView:[sidebarController_ view]]);
+// StatusBubble delegate method: tell the status bubble the frame it should
+// position itself in.
+- (NSRect)statusBubbleBaseFrame {
+  NSView* baseView = [sidebarController_ view];
+  NSArray* contentsSubviews = [baseView subviews];
+  if ([contentsSubviews count] > 0)
+    baseView = [contentsSubviews objectAtIndex:0];
+  return [[baseView superview] convertRect:[baseView frame] toView:nil];
 }
 
 - (GTMWindowSheetController*)sheetController {
diff --git a/chrome/browser/cocoa/browser_window_controller_unittest.mm b/chrome/browser/cocoa/browser_window_controller_unittest.mm
index a66b4ff..17886c2 100644
--- a/chrome/browser/cocoa/browser_window_controller_unittest.mm
+++ b/chrome/browser/cocoa/browser_window_controller_unittest.mm
@@ -50,8 +50,8 @@
   return [findBarCocoaController_ view];
 }
 
-- (NSView*)devToolsView {
-  return [devToolsController_ view];
+- (NSSplitView*)devToolsView {
+  return static_cast<NSSplitView*>([devToolsController_ view]);
 }
 
 - (NSView*)sidebarView {
@@ -569,6 +569,24 @@ TEST_F(BrowserWindowControllerTest, TestSplitViewsAreNotOpaque) {
   EXPECT_FALSE([[controller_ sidebarView] isOpaque]);
 }
 
+// Tests that status bubble's base frame does move when devTools are docked.
+TEST_F(BrowserWindowControllerTest, TestStatusBubblePositioning) {
+  ASSERT_EQ(1U, [[[controller_ devToolsView] subviews] count]);
+
+  NSPoint bubbleOrigin = [controller_ statusBubbleBaseFrame].origin;
+
+  // Add a fake subview to devToolsView to emulate docked devTools.
+  scoped_nsobject<NSView> view(
+      [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 10, 10)]);
+  [[controller_ devToolsView] addSubview:view];
+  [[controller_ devToolsView] adjustSubviews];
+
+  NSPoint bubbleOriginWithDevTools = [controller_ statusBubbleBaseFrame].origin;
+
+  // Make sure that status bubble frame is moved.
+  EXPECT_FALSE(NSEqualPoints(bubbleOrigin, bubbleOriginWithDevTools));
+}
+
 @interface BrowserWindowControllerFakeFullscreen : BrowserWindowController {
  @private
   // We release the window ourselves, so we don't have to rely on the unittest
diff --git a/chrome/browser/cocoa/status_bubble_mac.h b/chrome/browser/cocoa/status_bubble_mac.h
index 7ec4b51..de331d4 100644
--- a/chrome/browser/cocoa/status_bubble_mac.h
+++ b/chrome/browser/cocoa/status_bubble_mac.h
@@ -161,12 +161,12 @@ class StatusBubbleMac : public StatusBubble {
 
 // Delegate interface
 @interface NSObject(StatusBubbleDelegate)
-// Called to query the delegate about the vertical offset (if any) that should
-// be applied to the StatusBubble's position.
-- (CGFloat)verticalOffsetForStatusBubble;
+// Called to query the delegate about the frame StatusBubble should position
+// itself in. Frame is returned in the parent window coordinates.
+- (NSRect)statusBubbleBaseFrame;
 
 // Called from SetState to notify the delegate of state changes.
 - (void)statusBubbleWillEnterState:(StatusBubbleMac::StatusBubbleState)state;
 @end
 
-#endif  // #ifndef CHROME_BROWSER_COCOA_STATUS_BUBBLE_MAC_H_
+#endif  // CHROME_BROWSER_COCOA_STATUS_BUBBLE_MAC_H_
diff --git a/chrome/browser/cocoa/status_bubble_mac.mm b/chrome/browser/cocoa/status_bubble_mac.mm
index 4b51d56..e937668 100644
--- a/chrome/browser/cocoa/status_bubble_mac.mm
+++ b/chrome/browser/cocoa/status_bubble_mac.mm
@@ -24,7 +24,7 @@ namespace {
 const int kWindowHeight = 18;
 
 // The width of the bubble in relation to the width of the parent window.
-const double kWindowWidthPercent = 1.0 / 3.0;
+const CGFloat kWindowWidthPercent = 1.0 / 3.0;
 
 // How close the mouse can get to the infobubble before it starts sliding
 // off-screen.
@@ -272,18 +272,16 @@ void StatusBubbleMac::MouseMoved(
   NSPoint cursor_location = [NSEvent mouseLocation];
   --cursor_location.y;  // docs say the y coord starts at 1 not 0; don't ask why
 
+  // Bubble's base frame in |parent_| coordinates.
+  NSRect baseFrame;
+  if ([delegate_ respondsToSelector:@selector(statusBubbleBaseFrame)])
+    baseFrame = [delegate_ statusBubbleBaseFrame];
+  else
+    baseFrame = [[parent_ contentView] frame];
+
   // Get the normal position of the frame.
   NSRect window_frame = [window_ frame];
-  window_frame.origin = [parent_ frame].origin;
-
-  bool isShelfVisible = false;
-
-  // Adjust the position to sit on top of download and extension shelves.
-  // |delegate_| can be nil during unit tests.
-  if ([delegate_ respondsToSelector:@selector(verticalOffsetForStatusBubble)]) {
-    window_frame.origin.y += [delegate_ verticalOffsetForStatusBubble];
-    isShelfVisible = [delegate_ verticalOffsetForStatusBubble] > 0;
-  }
+  window_frame.origin = [parent_ convertBaseToScreen:baseFrame.origin];
 
   // Get the cursor position relative to the popup.
   cursor_location.x -= NSMaxX(window_frame);
@@ -312,7 +310,11 @@ void StatusBubbleMac::MouseMoved(
       isOnScreen = false;
     }
 
-    if (isOnScreen && !isShelfVisible) {
+    // If something is shown below tab contents (devtools, download shelf etc.),
+    // adjust the position to sit on top of it.
+    bool isAnyShelfVisible = NSMinY(baseFrame) > 0;
+
+    if (isOnScreen && !isAnyShelfVisible) {
       // Cap the offset and change the visual presentation of the bubble
       // depending on where it ends up (so that rounded corners square off
       // and mate to the edges of the tab content).
@@ -329,12 +331,12 @@ void StatusBubbleMac::MouseMoved(
       }
       window_frame.origin.y -= offset;
     } else {
-      // The bubble will obscure the download shelf.  Move the bubble to the
-      // right and reset Y offset_ to zero.
+      // Cannot move the bubble down without obscuring other content.
+      // Move it to the right instead.
       [[window_ contentView] setCornerFlags:kRoundedTopLeftCorner];
 
       // Subtract border width + bubble width.
-      window_frame.origin.x += NSWidth([parent_ frame]) - NSWidth(window_frame);
+      window_frame.origin.x += NSWidth(baseFrame) - NSWidth(window_frame);
     }
   } else {
     [[window_ contentView] setCornerFlags:kRoundedTopRightCorner];
@@ -632,9 +634,22 @@ void StatusBubbleMac::ExpandBubble() {
     return;
   }
 
+  NSRect actual_window_frame = [window_ frame];
+  // Adjust status bubble origin if bubble was moved to the right.
+  // TODO(alekseys): fix for RTL.
+  if (NSMinX(actual_window_frame) > NSMinX(window_frame)) {
+    actual_window_frame.origin.x =
+        NSMaxX(actual_window_frame) - NSWidth(window_frame);
+  }
+  actual_window_frame.size.width = NSWidth(window_frame);
+
+  // Do not expand if it's going to cover mouse location.
+  if (NSPointInRect([NSEvent mouseLocation], actual_window_frame))
+    return;
+
   [NSAnimationContext beginGrouping];
   [[NSAnimationContext currentContext] setDuration:kExpansionDuration];
-  [[window_ animator] setFrame:window_frame display:YES];
+  [[window_ animator] setFrame:actual_window_frame display:YES];
   [NSAnimationContext endGrouping];
 }
 
@@ -666,18 +681,23 @@ void StatusBubbleMac::SwitchParentWindow(NSWindow* parent) {
 NSRect StatusBubbleMac::CalculateWindowFrame(bool expanded_width) {
   DCHECK(parent_);
 
+  NSRect screenRect;
+  if ([delegate_ respondsToSelector:@selector(statusBubbleBaseFrame)]) {
+    screenRect = [delegate_ statusBubbleBaseFrame];
+    screenRect.origin = [parent_ convertBaseToScreen:screenRect.origin];
+  } else {
+    screenRect = [parent_ frame];
+  }
+
   NSSize size = NSMakeSize(0, kWindowHeight);
   size = [[parent_ contentView] convertSize:size toView:nil];
 
-  NSRect rect = [parent_ frame];
-  rect.size.height = size.height;
-
-  const int kScrollbarWidth = 16; // see BrowserWindowController.
   if (expanded_width) {
-    rect.size.width = NSWidth(rect) - kScrollbarWidth;
+    size.width = screenRect.size.width;
   } else {
-    rect.size.width = static_cast<int>(kWindowWidthPercent * NSWidth(rect));
+    size.width = kWindowWidthPercent * screenRect.size.width;
   }
 
-  return rect;
+  screenRect.size = size;
+  return screenRect;
 }
diff --git a/chrome/browser/cocoa/status_bubble_mac_unittest.mm b/chrome/browser/cocoa/status_bubble_mac_unittest.mm
index 0d87a0c..771dff5 100644
--- a/chrome/browser/cocoa/status_bubble_mac_unittest.mm
+++ b/chrome/browser/cocoa/status_bubble_mac_unittest.mm
@@ -20,11 +20,36 @@
 // transitions.
 @interface StatusBubbleMacTestDelegate : NSObject {
  @private
+  NSWindow* window_;  // Weak.
+  NSPoint baseFrameOffset_;
   std::vector<StatusBubbleMac::StatusBubbleState> states_;
 }
+- (id)initWithWindow:(NSWindow*)window;
+- (void)forceBaseFrameOffset:(NSPoint)baseFrameOffset;
+- (NSRect)statusBubbleBaseFrame;
 - (void)statusBubbleWillEnterState:(StatusBubbleMac::StatusBubbleState)state;
 @end
 @implementation StatusBubbleMacTestDelegate
+- (id)initWithWindow:(NSWindow*)window {
+  if ((self = [super init])) {
+    window_ = window;
+    baseFrameOffset_ = NSMakePoint(0, 0);
+  }
+  return self;
+}
+- (void)forceBaseFrameOffset:(NSPoint)baseFrameOffset {
+  baseFrameOffset_ = baseFrameOffset;
+}
+- (NSRect)statusBubbleBaseFrame {
+  NSView* contentView = [window_ contentView];
+  NSRect baseFrame = [contentView convertRect:[contentView frame] toView:nil];
+  if (baseFrameOffset_.x > 0 || baseFrameOffset_.y > 0) {
+    baseFrame = NSOffsetRect(baseFrame, baseFrameOffset_.x, baseFrameOffset_.y);
+    baseFrame.size.width -= baseFrameOffset_.x;
+    baseFrame.size.height -= baseFrameOffset_.y;
+  }
+  return baseFrame;
+}
 - (void)statusBubbleWillEnterState:(StatusBubbleMac::StatusBubbleState)state {
   states_.push_back(state);
 }
@@ -52,7 +77,8 @@ class StatusBubbleMacTest : public CocoaTest {
     CocoaTest::SetUp();
     NSWindow* window = test_window();
     EXPECT_TRUE(window);
-    delegate_.reset([[StatusBubbleMacTestDelegate alloc] init]);
+    delegate_.reset(
+        [[StatusBubbleMacTestDelegate alloc] initWithWindow: window]);
     EXPECT_TRUE(delegate_.get());
     bubble_ = new StatusBubbleMacIgnoreMouseMoved(window, delegate_);
     EXPECT_TRUE(bubble_);
@@ -483,6 +509,31 @@ TEST_F(StatusBubbleMacTest, MovingWindowUpdatesPosition) {
   EXPECT_TRUE(NSEqualPoints([window frame].origin, [child frame].origin));
 }
 
+TEST_F(StatusBubbleMacTest, StatuBubbleRespectsBaseFrameLimits) {
+  NSWindow* window = test_window();
+
+  // Show the bubble and make sure it has the same origin as |window|.
+  bubble_->SetStatus(UTF8ToUTF16("Showing"));
+  NSWindow* child = GetWindow();
+  EXPECT_TRUE(NSEqualPoints([window frame].origin, [child frame].origin));
+
+  // Hide the bubble, change base frame offset, and show it again.
+  bubble_->Hide();
+
+  NSPoint baseFrameOffset = NSMakePoint(0, [window frame].size.height / 3);
+  EXPECT_GT(baseFrameOffset.y, 0);
+  [delegate_ forceBaseFrameOffset:baseFrameOffset];
+
+  bubble_->SetStatus(UTF8ToUTF16("Reshowing"));
+
+  // The bubble should reattach in the correct location.
+  child = GetWindow();
+  NSPoint expectedOrigin = [window frame].origin;
+  expectedOrigin.x += baseFrameOffset.x;
+  expectedOrigin.y += baseFrameOffset.y;
+  EXPECT_TRUE(NSEqualPoints(expectedOrigin, [child frame].origin));
+}
+
 TEST_F(StatusBubbleMacTest, ExpandBubble) {
   NSWindow* window = test_window();
   ASSERT_TRUE(window);
@@ -495,7 +546,7 @@ TEST_F(StatusBubbleMacTest, ExpandBubble) {
   EXPECT_TRUE(IsVisible());
   bubble_->SetURL(GURL("http://www.battersbox.com/peter_paul_and_mary.html"),
                   string16());
-  EXPECT_NSEQ(@"battersbox.com/peter_paul_and_\u2026", GetURLText());
+  EXPECT_TRUE([GetURLText() hasSuffix:@"\u2026"]);
   bubble_->ExpandBubble();
   EXPECT_TRUE(IsVisible());
   EXPECT_NSEQ(@"www.battersbox.com/peter_paul_and_mary.html", GetURLText());
@@ -505,7 +556,7 @@ TEST_F(StatusBubbleMacTest, ExpandBubble) {
   bubble_->SetStatus(UTF8ToUTF16("Showing"));
   bubble_->SetURL(GURL("http://www.snickersnee.com/pioneer_fishstix.html"),
                   string16());
-  EXPECT_NSEQ(@"snickersnee.com/pioneer_fishstix\u2026", GetURLText());
+  EXPECT_TRUE([GetURLText() hasSuffix:@"\u2026"]);
   // ...and that it expands again properly.
   bubble_->ExpandBubble();
   EXPECT_NSEQ(@"www.snickersnee.com/pioneer_fishstix.html", GetURLText());
@@ -522,13 +573,12 @@ TEST_F(StatusBubbleMacTest, ExpandBubble) {
 
   // Very long URL's will be cut off even in the expanded state.
   bubble_->SetStatus(UTF8ToUTF16("Showing"));
-  bubble_->SetURL(GURL(
-      "http://www.diewahrscheinlichlaengstepralinederwelt.com/duuuuplo.html"),
-          string16());
-  EXPECT_NSEQ(@"diewahrscheinli\u2026", GetURLText());
+  const char veryLongUrl[] =
+      "http://www.diewahrscheinlichlaengstepralinederwelt.com/duuuuplo.html";
+  bubble_->SetURL(GURL(veryLongUrl), string16());
+  EXPECT_TRUE([GetURLText() hasSuffix:@"\u2026"]);
   bubble_->ExpandBubble();
-  EXPECT_NSEQ(@"diewahrscheinlichlaengstepralinederwelt.com/duu\u2026",
-      GetURLText());
+  EXPECT_TRUE([GetURLText() hasSuffix:@"\u2026"]);
 }