Mac: Improve tab strip layout in case of overflow

chrome/browser/ui/cocoa/browser_window_controller_private.mm

 // Copyright (c) 2012 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.
 
 #import "chrome/browser/ui/cocoa/browser_window_controller_private.h"
 
 #include <cmath>
 
 #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"
 #include "chrome/browser/fullscreen.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/ui/bookmarks/bookmark_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window_state.h"
 #import "chrome/browser/ui/cocoa/dev_tools_controller.h"
@@ skipping 25 matching lines @@
 #include "ui/base/ui_base_types.h"
 
 using content::RenderWidgetHostView;
 using content::WebContents;
 
 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.
 const CGFloat kLocBarLeftRightInset = 1;
 const CGFloat kLocBarTopInset = 0;
 const CGFloat kLocBarBottomInset = 1;
 
 }  // namespace
 
 @implementation BrowserWindowController(Private)
@@ skipping 239 matching lines @@
   maxY -= tabStripHeight;
   [tabStripView setFrame:NSMakeRect(0, maxY, width, tabStripHeight)];
 
   // Set left indentation based on fullscreen mode status.
   [tabStripController_ setLeftIndentForControls:(fullscreen ? 0 :
       [[tabStripController_ class] defaultLeftIndentForControls])];
 
   // 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;
     CGFloat buttonHeight = NSHeight([avatarButton frame]);
 
     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 {
       // Actually place the badge *above* |maxY|, by +2 to miss the divider.
       badgeYOffset = 2 * [[avatarButton superview] cr_lineWidth];
     }
 
     [avatarButton setFrameSize:NSMakeSize(NSWidth([avatarButton frame]),
         std::min(buttonHeight, tabStripHeight))];
     NSPoint origin =
         NSMakePoint(width - NSWidth([avatarButton frame]) + badgeXOffset,
                     maxY + badgeYOffset);
     [avatarButton setFrameOrigin:origin];
     [avatarButton setHidden:NO];  // Make sure it's shown.
   }
 
   // Calculate the right indentation.  The default indentation built into the
   // tabstrip leaves enough room for the fullscreen button or presentation mode
   // 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];
 
   // Go ahead and layout the tabs.
   [tabStripController_ layoutTabsWithoutAnimation];
 
   return maxY;
 }
 
 - (CGFloat)layoutToolbarAtMinX:(CGFloat)minX
@@ skipping 669 matching lines @@
   }
 }
 
 - (void)updateInfoBarTipVisibility {
   // If there's no toolbar then hide the infobar tip.
   [infoBarContainerController_
       setShouldSuppressTopInfoBarTip:![self hasToolbar]];
 }
 
 @end  // @implementation BrowserWindowController(Private)