Mac: Correctly clip tab title

When lots of tabs were created tab titles would sometimes overflow onto the close buttons or onto other tabs.

I think this is a problem with NSView's autoresize code. It doesn't handle the case where a view has been resized to 0 width.

To fix this I'm changing the tab layout code not to depend on NSView's autoresize behavior. The old code would let NSView resize the title view then grow or shrink the title as needed. The new code always sets the title view's frame a specific distance from the super frame.

BUG=32268
TEST=Opened lots of tabs and resized the browser window. Verified that the title never overlapped the close button. Added a new unit test to verify the layout of the title view.

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

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

3 files changed, 93 insertions, 33 deletions

diff --git a/chrome/browser/ui/cocoa/tab_controller.h b/chrome/browser/ui/cocoa/tab_controller.h
index bd515bd..8ed49eb 100644
--- a/chrome/browser/ui/cocoa/tab_controller.h
+++ b/chrome/browser/ui/cocoa/tab_controller.h
@@ -51,7 +51,6 @@ class MenuDelegate;
   BOOL selected_;
   TabLoadingState loadingState_;
   CGFloat iconTitleXOffset_;  // between left edges of icon and title
-  CGFloat titleCloseWidthOffset_;  // between right edges of icon and close btn.
   id<TabControllerTarget> target_;  // weak, where actions are sent
   SEL action_;  // selector sent when tab is selected by clicking
   scoped_ptr<TabMenuModel> contextMenuModel_;
@@ -67,6 +66,9 @@ class MenuDelegate;
 @property(assign, nonatomic) BOOL pinned;
 @property(assign, nonatomic) BOOL selected;
 @property(assign, nonatomic) id target;
+@property(assign, nonatomic) NSView* iconView;
+@property(assign, nonatomic) NSTextField* titleView;
+@property(assign, nonatomic) HoverCloseButton* closeButton;
 
 // Minimum and maximum allowable tab width. The minimum width does not show
 // the icon or the close button. The selected tab always has at least a close
diff --git a/chrome/browser/ui/cocoa/tab_controller.mm b/chrome/browser/ui/cocoa/tab_controller.mm
index 7a6f675..2ae2454 100644
--- a/chrome/browser/ui/cocoa/tab_controller.mm
+++ b/chrome/browser/ui/cocoa/tab_controller.mm
@@ -21,6 +21,9 @@
 @synthesize mini = mini_;
 @synthesize pinned = pinned_;
 @synthesize target = target_;
+@synthesize iconView = iconView_;
+@synthesize titleView = titleView_;
+@synthesize closeButton = closeButton_;
 
 namespace TabControllerInternal {
 
@@ -114,10 +117,11 @@ class MenuDelegate : public menus::SimpleMenuModel::Delegate {
   // When the icon is removed, the title expands to the left to fill the space
   // left by the icon.  When the close button is removed, the title expands to
   // the right to fill its space.  These are the amounts to expand and contract
-  // titleView_ under those conditions.
+  // titleView_ under those conditions. We don't have to explicilty save the
+  // offset between the title and the close button since we can just get that
+  // value for the close button's frame.
   NSRect titleFrame = [titleView_ frame];
   iconTitleXOffset_ = NSMinX(titleFrame) - NSMinX(originalIconFrame_);
-  titleCloseWidthOffset_ = NSMaxX([closeButton_ frame]) - NSMaxX(titleFrame);
 
   [self internalSetSelected:selected_];
 }
@@ -183,10 +187,6 @@ class MenuDelegate : public menus::SimpleMenuModel::Delegate {
     [[self view] addSubview:iconView_];
 }
 
-- (NSView*)iconView {
-  return iconView_;
-}
-
 - (NSString*)toolTip {
   return [[self view] toolTip];
 }
@@ -230,47 +230,40 @@ class MenuDelegate : public menus::SimpleMenuModel::Delegate {
   // iconView_ may have been replaced or it may be nil, so [iconView_ isHidden]
   // won't work.  Instead, the state of the icon is tracked separately in
   // isIconShowing_.
-  BOOL oldShowIcon = isIconShowing_ ? YES : NO;
-  BOOL newShowIcon = [self shouldShowIcon] ? YES : NO;
+  BOOL newShowIcon = [self shouldShowIcon];
 
-  [iconView_ setHidden:newShowIcon ? NO : YES];
+  [iconView_ setHidden:!newShowIcon];
   isIconShowing_ = newShowIcon;
 
   // If the tab is a mini-tab, hide the title.
   [titleView_ setHidden:[self mini]];
 
-  BOOL oldShowCloseButton = [closeButton_ isHidden] ? NO : YES;
-  BOOL newShowCloseButton = [self shouldShowCloseButton] ? YES : NO;
+  BOOL newShowCloseButton = [self shouldShowCloseButton];
 
-  [closeButton_ setHidden:newShowCloseButton ? NO : YES];
+  [closeButton_ setHidden:!newShowCloseButton];
 
   // Adjust the title view based on changes to the icon's and close button's
   // visibility.
-  NSRect titleFrame = [titleView_ frame];
+  NSRect oldTitleFrame = [titleView_ frame];
+  NSRect newTitleFrame;
+  newTitleFrame.size.height = oldTitleFrame.size.height;
+  newTitleFrame.origin.y = oldTitleFrame.origin.y;
 
-  if (oldShowIcon != newShowIcon) {
-    // Adjust the left edge of the title view according to the presence or
-    // absence of the icon view.
-
-    if (newShowIcon) {
-      titleFrame.origin.x += iconTitleXOffset_;
-      titleFrame.size.width -= iconTitleXOffset_;
-    } else {
-      titleFrame.origin.x -= iconTitleXOffset_;
-      titleFrame.size.width += iconTitleXOffset_;
-    }
+  if (newShowIcon) {
+    newTitleFrame.origin.x = originalIconFrame_.origin.x + iconTitleXOffset_;
+  } else {
+    newTitleFrame.origin.x = originalIconFrame_.origin.x;
   }
 
-  if (oldShowCloseButton != newShowCloseButton) {
-    // Adjust the right edge of the title view according to the presence or
-    // absence of the close button.
-    if (newShowCloseButton)
-      titleFrame.size.width -= titleCloseWidthOffset_;
-    else
-      titleFrame.size.width += titleCloseWidthOffset_;
+  if (newShowCloseButton) {
+    newTitleFrame.size.width = NSMinX([closeButton_ frame]) -
+                               newTitleFrame.origin.x;
+  } else {
+    newTitleFrame.size.width = NSMaxX([closeButton_ frame]) -
+                               newTitleFrame.origin.x;
   }
 
-  [titleView_ setFrame:titleFrame];
+  [titleView_ setFrame:newTitleFrame];
 }
 
 - (void)updateTitleColor {
diff --git a/chrome/browser/ui/cocoa/tab_controller_unittest.mm b/chrome/browser/ui/cocoa/tab_controller_unittest.mm
index e9bafe9..93331ac 100644
--- a/chrome/browser/ui/cocoa/tab_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/tab_controller_unittest.mm
@@ -62,6 +62,14 @@
 
 namespace {
 
+CGFloat LeftMargin(NSRect superFrame, NSRect subFrame) {
+  return NSMinX(subFrame) - NSMinX(superFrame);
+}
+
+CGFloat RightMargin(NSRect superFrame, NSRect subFrame) {
+  return NSMaxX(superFrame) - NSMaxX(subFrame);
+}
+
 // The dragging code in TabView makes heavy use of autorelease pools so
 // inherit from CocoaTest to have one created for us.
 class TabControllerTest : public CocoaTest {
@@ -265,4 +273,61 @@ TEST_F(TabControllerTest, Menu) {
   EXPECT_GT([menu numberOfItems], 0);
 }
 
+// Tests that the title field is correctly positioned and sized when the
+// view is resized.
+TEST_F(TabControllerTest, TitleViewLayout) {
+  NSWindow* window = test_window();
+
+  scoped_nsobject<TabController> controller([[TabController alloc] init]);
+  [[window contentView] addSubview:[controller view]];
+  NSRect tabFrame = [[controller view] frame];
+  tabFrame.size.width = [TabController maxTabWidth];
+  [[controller view] setFrame:tabFrame];
+
+  const NSRect originalTabFrame = [[controller view] frame];
+  const NSRect originalIconFrame = [[controller iconView] frame];
+  const NSRect originalCloseFrame = [[controller closeButton] frame];
+  const NSRect originalTitleFrame = [[controller titleView] frame];
+
+  // Sanity check the start state.
+  EXPECT_FALSE([[controller iconView] isHidden]);
+  EXPECT_FALSE([[controller closeButton] isHidden]);
+  EXPECT_GT(NSWidth([[controller view] frame]),
+            NSWidth([[controller titleView] frame]));
+
+  // Resize the tab so that that the it shrinks.
+  tabFrame.size.width = [TabController minTabWidth];
+  [[controller view] setFrame:tabFrame];
+
+  // The icon view and close button should be hidden and the title view should
+  // be resize to take up their space.
+  EXPECT_TRUE([[controller iconView] isHidden]);
+  EXPECT_TRUE([[controller closeButton] isHidden]);
+  EXPECT_GT(NSWidth([[controller view] frame]),
+            NSWidth([[controller titleView] frame]));
+  EXPECT_EQ(LeftMargin(originalTabFrame, originalIconFrame),
+            LeftMargin([[controller view] frame],
+                       [[controller titleView] frame]));
+  EXPECT_EQ(RightMargin(originalTabFrame, originalCloseFrame),
+            RightMargin([[controller view] frame],
+                        [[controller titleView] frame]));
+
+  // Resize the tab so that that the it grows.
+  tabFrame.size.width = [TabController maxTabWidth] * 0.75;
+  [[controller view] setFrame:tabFrame];
+
+  // The icon view and close button should be visible again and the title view
+  // should be resized to make room for them.
+  EXPECT_FALSE([[controller iconView] isHidden]);
+  EXPECT_FALSE([[controller closeButton] isHidden]);
+  EXPECT_GT(NSWidth([[controller view] frame]),
+            NSWidth([[controller titleView] frame]));
+  EXPECT_EQ(LeftMargin(originalTabFrame, originalTitleFrame),
+            LeftMargin([[controller view] frame],
+                       [[controller titleView] frame]));
+  EXPECT_EQ(RightMargin(originalTabFrame, originalTitleFrame),
+            RightMargin([[controller view] frame],
+                        [[controller titleView] frame]));
+}
+
 }  // namespace