Don't show favicons or throbbers for the New Tab page on the Mac.

This change nets another 35ms (15%) improvement in the duration between the
renderer requesting the NTP and it having all of the resources for the NTP
available and being completely done with layout on my Mac laptop in release
mode.

For any page which DOMUI indicates no icon should be shown, including the New
Tab page, don't show a favicon or throbber.  When the icon is removed, the
title should expand to the left to fill the void.  Compare to the behavior on
Windows and Linux.

http://groups.google.com/group/chromium-dev/browse_thread/thread/7148074f807dc5f7

BUG=13337 20378
TEST=No throbber or favicon on the New Tab page
Review URL: http://codereview.chromium.org/184003

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

4 files changed, 124 insertions, 56 deletions

diff --git a/chrome/browser/cocoa/tab_controller.h b/chrome/browser/cocoa/tab_controller.h
index 316b985..7d44751 100644
--- a/chrome/browser/cocoa/tab_controller.h
+++ b/chrome/browser/cocoa/tab_controller.h
@@ -40,8 +40,12 @@ enum TabLoadingState {
   IBOutlet NSMenu* contextMenu_;
   IBOutlet NSButton* closeButton_;
 
+  NSRect originalIconFrame_;  // frame of iconView_ as loaded from nib
+  BOOL isIconShowing_;  // last state of iconView_ in updateVisibility
   BOOL selected_;
   TabLoadingState loadingState_;
+  float iconTitleXOffset_;  // between left edges of icon and title
+  float titleCloseWidthOffset_;  // between right edges of icon and close button
   id<TabControllerTarget> target_;  // weak, where actions are sent
   SEL action_;  // selector sent when tab is selected by clicking
 }
@@ -53,8 +57,8 @@ enum TabLoadingState {
 @property(assign, nonatomic) SEL action;
 
 // Minimum and maximum allowable tab width. The minimum width does not show
-// the icon or the close box. The selected tab always has at least a close box
-// so it has a different minimum width.
+// the icon or the close button. The selected tab always has at least a close
+// button so it has a different minimum width.
 + (float)minTabWidth;
 + (float)maxTabWidth;
 + (float)minSelectedTabWidth;
@@ -88,7 +92,7 @@ enum TabLoadingState {
 - (NSString *)toolTip;
 - (int)iconCapacity;
 - (BOOL)shouldShowIcon;
-- (BOOL)shouldShowCloseBox;
+- (BOOL)shouldShowCloseButton;
 @end  // TabController(TestingAPI)
 
 #endif  // CHROME_BROWSER_COCOA_TAB_CONTROLLER_H_
diff --git a/chrome/browser/cocoa/tab_controller.mm b/chrome/browser/cocoa/tab_controller.mm
index 75f860d..231164a 100644
--- a/chrome/browser/cocoa/tab_controller.mm
+++ b/chrome/browser/cocoa/tab_controller.mm
@@ -21,10 +21,10 @@
 
 // The min widths match the windows values and are sums of left + right
 // padding, of which we have no comparable constants (we draw using paths, not
-// images). The selected tab width includes the close box width.
+// images). The selected tab width includes the close button width.
 + (float)minTabWidth { return 31; }
 + (float)minSelectedTabWidth { return 47; }
-+ (float)maxTabWidth { return 220.0; }
++ (float)maxTabWidth { return 220; }
 
 - (TabView*)tabView {
   return static_cast<TabView*>([self view]);
@@ -33,6 +33,7 @@
 - (id)init {
   self = [super initWithNibName:@"TabView" bundle:mac_util::MainAppBundle()];
   if (self != nil) {
+    isIconShowing_ = YES;
     [[NSNotificationCenter defaultCenter]
         addObserver:self
            selector:@selector(viewResized:)
@@ -59,9 +60,21 @@
 
 // Called when the tab's nib is done loading and all outlets are hooked up.
 - (void)awakeFromNib {
+  // Remember the icon's frame, so that if the icon is ever removed, a new
+  // one can later replace it in the proper location.
+  originalIconFrame_ = [iconView_ frame];
+
+  // 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.
+  NSRect titleFrame = [titleView_ frame];
+  iconTitleXOffset_ = NSMinX(titleFrame) - NSMinX(originalIconFrame_);
+  titleCloseWidthOffset_ = NSMaxX([closeButton_ frame]) - NSMaxX(titleFrame);
+
   // Ensure we don't show favicon if the tab is already too small to begin with.
   [self updateVisibility];
-  [(id)iconView_ setImage:nsimage_cache::ImageNamed(@"nav.pdf")];
+
   [self internalSetSelected:selected_];
 }
 
@@ -103,13 +116,16 @@
 }
 
 - (void)setIconView:(NSView*)iconView {
-  NSRect currentFrame = [iconView_ frame];
   [iconView_ removeFromSuperview];
   iconView_ = iconView;
-  [iconView_ setFrame:currentFrame];
-  // Ensure we don't show favicon if the tab is already too small to begin with.
+  [iconView_ setFrame:originalIconFrame_];
+
+  // Ensure that the icon is suppressed if no icon is set or if the tab is too
+  // narrow to display one.
   [self updateVisibility];
-  [[self view] addSubview:iconView_];
+
+  if (iconView_)
+    [[self view] addSubview:iconView_];
 }
 
 - (NSView*)iconView {
@@ -124,42 +140,81 @@
 // tab. We never actually do this, but it's a helpful guide for determining
 // how much space we have available.
 - (int)iconCapacity {
-  float width = NSWidth([[self view] frame]);
-  float leftPadding = NSMinX([iconView_ frame]);
-  float rightPadding = width - NSMaxX([closeButton_ frame]);
-  float iconWidth = NSWidth([iconView_ frame]);
+  float width = NSMaxX([closeButton_ frame]) - NSMinX(originalIconFrame_);
+  float iconWidth = NSWidth(originalIconFrame_);
 
-  width -= leftPadding + rightPadding;
   return width / iconWidth;
 }
 
 // Returns YES if we should show the icon. When tabs get too small, we clip
-// the favicon before the close box for selected tabs, and prefer the favicon
-// for unselected tabs.
-// TODO(pinkerton): don't show the icon if there's no image data (eg, NTP).
+// the favicon before the close button for selected tabs, and prefer the
+// favicon for unselected tabs.  The icon can also be suppressed more directly
+// by clearing iconView_.
 - (BOOL)shouldShowIcon {
+  if (!iconView_)
+    return NO;
+
   int iconCapacity = [self iconCapacity];
   if ([self selected])
     return iconCapacity >= 2;
   return iconCapacity >= 1;
 }
 
-// Returns YES if we should be showing the close box. The selected tab always
-// shows the close box.
-- (BOOL)shouldShowCloseBox {
+// Returns YES if we should be showing the close button. The selected tab
+// always shows the close button.
+- (BOOL)shouldShowCloseButton {
   return [self selected] || [self iconCapacity] >= 3;
 }
 
-// Call to update the visibility of certain subviews, such as the icon or
-// close box, based on criteria like if the tab is selected and the current
-// tab width.
+// Updates the visibility of certain subviews, such as the icon and close
+// button, based on criteria such as the tab's selected state and its current
+// width.
 - (void)updateVisibility {
-  [iconView_ setHidden:[self shouldShowIcon] ? NO : YES];
-  [closeButton_ setHidden:[self shouldShowCloseBox] ? NO : YES];
+  // 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;
+
+  [iconView_ setHidden:newShowIcon ? NO : YES];
+  isIconShowing_ = newShowIcon;
+
+  BOOL oldShowCloseButton = [closeButton_ isHidden] ? NO : YES;
+  BOOL newShowCloseButton = [self shouldShowCloseButton] ? YES : NO;
+
+  [closeButton_ setHidden:newShowCloseButton ? NO : YES];
+
+  // Adjust the title view based on changes to the icon's and close button's
+  // visibility.
+  NSRect titleFrame = [titleView_ frame];
+
+  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 (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_;
+  }
+
+  [titleView_ setFrame:titleFrame];
 }
 
 // Called when our view is resized. If it gets too small, start by hiding
-// the close box and only show it if tab is selected. Eventually, hide the
+// the close button and only show it if tab is selected. Eventually, hide the
 // icon as well. We know that this is for our view because we only registered
 // for notifications from our specific view.
 - (void)viewResized:(NSNotification*)info {
diff --git a/chrome/browser/cocoa/tab_controller_unittest.mm b/chrome/browser/cocoa/tab_controller_unittest.mm
index cd7fdeb..5d39aa4 100644
--- a/chrome/browser/cocoa/tab_controller_unittest.mm
+++ b/chrome/browser/cocoa/tab_controller_unittest.mm
@@ -226,7 +226,7 @@ TEST_F(TabControllerTest, ShouldShowIcon) {
   frame.size.width = [TabController minTabWidth];
   [[controller view] setFrame:frame];
   EXPECT_FALSE([controller shouldShowIcon]);
-  EXPECT_FALSE([controller shouldShowCloseBox]);
+  EXPECT_FALSE([controller shouldShowCloseButton]);
 
   // Setting the icon when tab is at min width should not show icon (bug 18359).
   scoped_nsobject<NSView> newIcon(
@@ -242,7 +242,7 @@ TEST_F(TabControllerTest, ShouldShowIcon) {
   [[controller view] setFrame:frame];
   EXPECT_FALSE([controller shouldShowIcon]);
   EXPECT_TRUE([newIcon isHidden]);
-  EXPECT_TRUE([controller shouldShowCloseBox]);
+  EXPECT_TRUE([controller shouldShowCloseButton]);
 
   // Test expanding the tab to max width and ensure the icon and close box
   // get put back, even when de-selected.
@@ -250,10 +250,10 @@ TEST_F(TabControllerTest, ShouldShowIcon) {
   [[controller view] setFrame:frame];
   EXPECT_TRUE([controller shouldShowIcon]);
   EXPECT_FALSE([newIcon isHidden]);
-  EXPECT_TRUE([controller shouldShowCloseBox]);
+  EXPECT_TRUE([controller shouldShowCloseButton]);
   [controller setSelected:NO];
   EXPECT_TRUE([controller shouldShowIcon]);
-  EXPECT_TRUE([controller shouldShowCloseBox]);
+  EXPECT_TRUE([controller shouldShowCloseButton]);
 
   cap = [controller iconCapacity];
   EXPECT_GT(cap, 0);
diff --git a/chrome/browser/cocoa/tab_strip_controller.mm b/chrome/browser/cocoa/tab_strip_controller.mm
index 73cd85a..ec84d059 100644
--- a/chrome/browser/cocoa/tab_strip_controller.mm
+++ b/chrome/browser/cocoa/tab_strip_controller.mm
@@ -648,20 +648,15 @@ static const float kUseFullAvailableWidth = -1.0;
   // Either we don't have a valid favicon or there was some issue converting it
   // from an SkBitmap. Either way, just show the default.
   if (!image) {
-    NSBundle* bundle = mac_util::MainAppBundle();
-    image = [[NSImage alloc] initByReferencingFile:
-             [bundle pathForResource:@"nav" ofType:@"pdf"]];
-    [image autorelease];
+    image = nsimage_cache::ImageNamed(@"nav.pdf");
   }
 
   [view setImage:image];
   return view;
 }
 
-// Update the current loading state, replacing the favicon with a throbber, or
-// vice versa. This will get called repeatedly with the same state during a
-// load, so we need to make sure we're not creating the throbber view over and
-// over. However, when the page is done, every state change is important.
+// Updates the current loading state, replacing the icon view with a favicon,
+// a throbber, the default icon, or nothing at all.
 - (void)updateFavIconForContents:(TabContents*)contents
                          atIndex:(NSInteger)index {
   if (!contents)
@@ -675,36 +670,50 @@ static const float kUseFullAvailableWidth = -1.0;
       [nsimage_cache::ImageNamed(@"sadfavicon.png") retain];
 
   TabController* tabController = [tabArray_ objectAtIndex:index];
+
+  bool oldHasIcon = [tabController iconView] != nil;
+  bool newHasIcon = contents->ShouldDisplayFavIcon();
+
   TabLoadingState oldState = [tabController loadingState];
   TabLoadingState newState = kTabDone;
   NSImage* throbberImage = nil;
-  if (contents->waiting_for_response()) {
+  if (contents->is_crashed()) {
+    newState = kTabCrashed;
+    newHasIcon = true;
+  } else if (contents->waiting_for_response()) {
     newState = kTabWaiting;
     throbberImage = throbberWaitingImage;
   } else if (contents->is_loading()) {
     newState = kTabLoading;
     throbberImage = throbberLoadingImage;
-  } else if (contents->is_crashed()) {
-    newState = kTabCrashed;
   }
 
-  if (oldState != newState || newState == kTabDone) {
+  if (oldState != newState)
+    [tabController setLoadingState:newState];
+
+  // While loading, this function is called repeatedly with the same state.
+  // To avoid expensive unnecessary view manipulation, only make changes when
+  // the state is actually changing.  When loading is complete (kTabDone),
+  // every call to this function is significant.
+  if (newState == kTabDone || oldState != newState ||
+      oldHasIcon != newHasIcon) {
     NSView* iconView = nil;
-    if (newState == kTabDone) {
-      iconView = [self favIconImageViewForContents:contents];
-    } else if (newState == kTabCrashed) {
-      NSImage* oldImage = [[self favIconImageViewForContents:contents] image];
-      NSRect frame = NSMakeRect(0, 0, 16, 16);
-      iconView = [ThrobberView toastThrobberViewWithFrame:frame
-                                              beforeImage:oldImage
-                                               afterImage:sadFaviconImage];
-    } else {
-      NSRect frame = NSMakeRect(0, 0, 16, 16);
-      iconView = [ThrobberView filmstripThrobberViewWithFrame:frame
-                                                        image:throbberImage];
+    if (newHasIcon) {
+      if (newState == kTabDone) {
+        iconView = [self favIconImageViewForContents:contents];
+      } else if (newState == kTabCrashed) {
+        NSImage* oldImage = [[self favIconImageViewForContents:contents] image];
+        NSRect frame = NSMakeRect(0, 0, 16, 16);
+        iconView = [ThrobberView toastThrobberViewWithFrame:frame
+                                                beforeImage:oldImage
+                                                 afterImage:sadFaviconImage];
+      } else {
+        NSRect frame = NSMakeRect(0, 0, 16, 16);
+        iconView = [ThrobberView filmstripThrobberViewWithFrame:frame
+                                                          image:throbberImage];
+      }
     }
 
-    [tabController setLoadingState:newState];
     [tabController setIconView:iconView];
   }
 }