Mac: Improve tab strip layout in case of overflow

chrome/browser/ui/cocoa/browser_window_controller_private.mm

Index: chrome/browser/ui/cocoa/browser_window_controller_private.mm
diff --git a/chrome/browser/ui/cocoa/browser_window_controller_private.mm b/chrome/browser/ui/cocoa/browser_window_controller_private.mm
index 00bc9a2d7f6f39bcc704b66cfc704b58e0758ef1..bc1b8f62dd459f49f8a20b6d3f85545c43791782 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller_private.mm
+++ b/chrome/browser/ui/cocoa/browser_window_controller_private.mm
@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/mac/mac_util.h"
 #import "base/mac/scoped_nsobject.h"
+#import "base/mac/sdk_forward_declarations.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/browser_process.h"
@@ -54,14 +55,6 @@ namespace {
 // Space between the incognito badge and the right edge of the window.
 const CGFloat kAvatarRightOffset = 4;
 
-// The amount by which to shrink the tab strip (on the right) when the
-// incognito badge is present.
-const CGFloat kAvatarTabStripShrink = 18;

[Andre, 2014/08/28 16:34:28]

This seems to be originally used as the width of the avatar button, but the code
has evolved to compute the avatar button more dynamically and I think was
incorrectly used.

-
-// Width of the full screen icon. Used to position the AvatarButton to the
-// left of the icon.
-const CGFloat kFullscreenIconWidth = 32;

[Andre, 2014/08/28 16:34:28]

The fullscreen icon is actually 16pt wide.
We were not computing its offset correctly anyway, after the fix we don't need
this width anymore.

-
 // Insets for the location bar, used when the full toolbar is hidden.
 // TODO(viettrungluu): We can argue about the "correct" insetting; I like the
 // following best, though arguably 0 inset is better/more correct.
@@ -321,6 +314,9 @@ willPositionSheet:(NSWindow*)sheet
   // Lay out the icognito/avatar badge because calculating the indentation on
   // the right depends on it.
   NSView* avatarButton = [avatarButtonController_ view];
+  NSButton* fullScreenButton =
+      [[self window] standardWindowButton:NSWindowFullScreenButton];

[Robert Sesek, 2014/09/02 18:17:49]

This returns nil on 10.6, yes? (Rather than say, throwing an exception).

[Andre, 2014/09/02 23:09:05]

Yes it does.
And turns out, it returns nil on Yosemite as well!
I've updated the comments to reflect this.

+
   if ([self shouldShowAvatar]) {
     CGFloat badgeXOffset = -kAvatarRightOffset;
     CGFloat badgeYOffset = 0;
@@ -328,8 +324,8 @@ willPositionSheet:(NSWindow*)sheet
 
     if ([self shouldUseNewAvatarButton]) {
       // The fullscreen icon is displayed to the right of the avatar button.
-      if (![self isFullscreen])
-        badgeXOffset -= kFullscreenIconWidth;

[Andre, 2014/08/28 16:34:28]

Width is not enough, we also need to offset for its origin.
fullScreenButtonOriginAdjustment is not good to use either, because it's the
amount the button is adjusted from its original unknown origin.

+      if (![self isFullscreen] && fullScreenButton)
+        badgeXOffset -= width - NSMinX([fullScreenButton frame]);
       // Center the button vertically on the tabstrip.
       badgeYOffset = (tabStripHeight - buttonHeight) / 2;
     } else {
@@ -351,25 +347,16 @@ willPositionSheet:(NSWindow*)sheet
   // toggle button on Lion.  On non-Lion systems, the right indent needs to be
   // adjusted to make room for the new tab button when an avatar is present.
   CGFloat rightIndent = 0;
-  if (base::mac::IsOSLionOrLater() &&
-      [[self window] isKindOfClass:[FramedBrowserWindow class]]) {
-    FramedBrowserWindow* window =
-        static_cast<FramedBrowserWindow*>([self window]);
-    rightIndent += -[window fullScreenButtonOriginAdjustment].x;

[Andre, 2014/08/28 16:34:28]

fullScreenButtonOriginAdjustment is not the amount of space to the right of the
button, it is the amount of adjustment from its original system defined
position.

+  if (![self isFullscreen] && fullScreenButton) {
+    rightIndent = width - NSMinX([fullScreenButton frame]);
 
     if ([self shouldUseNewAvatarButton]) {
-      // The new avatar is wider than the default indentation, so we need to
-      // account for its width.
-      rightIndent += NSWidth([avatarButton frame]) + kAvatarTabStripShrink;
-
-      // When the fullscreen icon is not displayed, return its width to the
-      // tabstrip.
-      if ([self isFullscreen])
-        rightIndent -= kFullscreenIconWidth;
+      // The new avatar button is to the left of the fullscreen button.
+      // (The old avatar button is to the right).
+      rightIndent += NSWidth([avatarButton frame]) + kAvatarRightOffset;
     }
   } else if ([self shouldShowAvatar]) {
-    rightIndent += kAvatarTabStripShrink +
-        NSWidth([avatarButton frame]) + kAvatarRightOffset;
+    rightIndent += NSWidth([avatarButton frame]) + kAvatarRightOffset;
   }
   [tabStripController_ setRightIndentForControls:rightIndent];