Don't rely on subview ordering to be equivalent to tab ordering. Allows us to correctly set z-order on tabs. Add animation to tabs closing. Fix issue where new tab button would not draw correctly by pulling it from the nib. Fix memory leak in TabController. Fix console output about missing image.

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

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

5 files changed, 129 insertions, 52 deletions

diff --git a/chrome/app/nibs/en.lproj/BrowserWindow.xib b/chrome/app/nibs/en.lproj/BrowserWindow.xib
index 42a88a2..eb76ef9 100644
--- a/chrome/app/nibs/en.lproj/BrowserWindow.xib
+++ b/chrome/app/nibs/en.lproj/BrowserWindow.xib
@@ -8,7 +8,7 @@
 		<string key="IBDocument.HIToolboxVersion">352.00</string>
 		<object class="NSMutableArray" key="IBDocument.EditedObjectIDs">
 			<bool key="EncodedWithXMLCoder">YES</bool>
-			<integer value="1"/>
+			<integer value="56"/>
 		</object>
 		<object class="NSArray" key="IBDocument.PluginDependencies">
 			<bool key="EncodedWithXMLCoder">YES</bool>
@@ -44,7 +44,7 @@
 				<nil key="NSViewClass"/>
 				<string key="NSWindowContentMaxSize">{3.40282e+38, 3.40282e+38}</string>
 				<object class="NSView" key="NSWindowView" id="1006">
-					<reference key="NSNextResponder"/>
+					<nil key="NSNextResponder"/>
 					<int key="NSvFlags">256</int>
 					<object class="NSMutableArray" key="NSSubviews">
 						<bool key="EncodedWithXMLCoder">YES</bool>
@@ -94,15 +94,46 @@
 						</object>
 					</object>
 					<string key="NSFrameSize">{750, 600}</string>
-					<reference key="NSSuperview"/>
 				</object>
 				<string key="NSScreenRect">{{0, 0}, {1440, 878}}</string>
 				<string key="NSMaxSize">{3.40282e+38, 3.40282e+38}</string>
 			</object>
 			<object class="NSCustomView" id="1029219716">
-				<nil key="NSNextResponder"/>
+				<reference key="NSNextResponder"/>
 				<int key="NSvFlags">266</int>
+				<object class="NSMutableArray" key="NSSubviews">
+					<bool key="EncodedWithXMLCoder">YES</bool>
+					<object class="NSButton" id="131944810">
+						<reference key="NSNextResponder" ref="1029219716"/>
+						<int key="NSvFlags">-2147483348</int>
+						<string key="NSFrame">{{295, 0}, {40, 27}}</string>
+						<reference key="NSSuperview" ref="1029219716"/>
+						<bool key="NSEnabled">YES</bool>
+						<object class="NSButtonCell" key="NSCell" id="517448702">
+							<int key="NSCellFlags">-2080244224</int>
+							<int key="NSCellFlags2">134217728</int>
+							<string key="NSContents"/>
+							<object class="NSFont" key="NSSupport">
+								<string key="NSName">LucidaGrande</string>
+								<double key="NSSize">1.300000e+01</double>
+								<int key="NSfFlags">1044</int>
+							</object>
+							<reference key="NSControlView" ref="131944810"/>
+							<int key="NSButtonFlags">138690815</int>
+							<int key="NSButtonFlags2">134</int>
+							<object class="NSCustomResource" key="NSNormalImage">
+								<string key="NSClassName">NSImage</string>
+								<string key="NSResourceName">newtab</string>
+							</object>
+							<string key="NSAlternateContents"/>
+							<string key="NSKeyEquivalent"/>
+							<int key="NSPeriodicDelay">400</int>
+							<int key="NSPeriodicInterval">75</int>
+						</object>
+					</object>
+				</object>
 				<string key="NSFrameSize">{483, 25}</string>
+				<reference key="NSSuperview"/>
 				<string key="NSClassName">TabStripView</string>
 			</object>
 		</object>
@@ -200,10 +231,25 @@
 						<reference key="object" ref="1029219716"/>
 						<object class="NSMutableArray" key="children">
 							<bool key="EncodedWithXMLCoder">YES</bool>
+							<reference ref="131944810"/>
 						</object>
 						<reference key="parent" ref="1002"/>
 						<string key="objectName">TabBar</string>
 					</object>
+					<object class="IBObjectRecord">
+						<int key="objectID">66</int>
+						<reference key="object" ref="131944810"/>
+						<object class="NSMutableArray" key="children">
+							<bool key="EncodedWithXMLCoder">YES</bool>
+							<reference ref="517448702"/>
+						</object>
+						<reference key="parent" ref="1029219716"/>
+					</object>
+					<object class="IBObjectRecord">
+						<int key="objectID">67</int>
+						<reference key="object" ref="517448702"/>
+						<reference key="parent" ref="131944810"/>
+					</object>
 				</object>
 			</object>
 			<object class="NSMutableDictionary" key="flattenedProperties">
@@ -227,6 +273,8 @@
 					<string>55.IBPluginDependency</string>
 					<string>56.IBEditorWindowLastContentRect</string>
 					<string>56.IBPluginDependency</string>
+					<string>66.IBPluginDependency</string>
+					<string>67.IBPluginDependency</string>
 				</object>
 				<object class="NSMutableArray" key="dict.values">
 					<bool key="EncodedWithXMLCoder">YES</bool>
@@ -250,6 +298,8 @@
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<string>{{160, 204}, {483, 25}}</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
+					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
+					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 				</object>
 			</object>
 			<object class="NSMutableDictionary" key="unlocalizedProperties">
@@ -272,7 +322,7 @@
 				</object>
 			</object>
 			<nil key="sourceID"/>
-			<int key="maxID">65</int>
+			<int key="maxID">67</int>
 		</object>
 		<object class="IBClassDescriber" key="IBDocument.Classes">
 			<object class="NSMutableArray" key="referencedPartialClassDescriptions">
diff --git a/chrome/app/nibs/en.lproj/TabView.xib b/chrome/app/nibs/en.lproj/TabView.xib
index 48fd1c8..4b5a3ca 100644
--- a/chrome/app/nibs/en.lproj/TabView.xib
+++ b/chrome/app/nibs/en.lproj/TabView.xib
@@ -8,7 +8,7 @@
 		<string key="IBDocument.HIToolboxVersion">352.00</string>
 		<object class="NSMutableArray" key="IBDocument.EditedObjectIDs">
 			<bool key="EncodedWithXMLCoder">YES</bool>
-			<integer value="17"/>
+			<integer value="18"/>
 		</object>
 		<object class="NSArray" key="IBDocument.PluginDependencies">
 			<bool key="EncodedWithXMLCoder">YES</bool>
@@ -47,7 +47,7 @@
 						<reference key="NSSuperview" ref="1005"/>
 						<bool key="NSEnabled">YES</bool>
 						<object class="NSButtonCell" key="NSCell" id="707804163">
-							<int key="NSCellFlags">67239424</int>
+							<int key="NSCellFlags">-2080244224</int>
 							<int key="NSCellFlags2">0</int>
 							<string key="NSContents">Label</string>
 							<object class="NSFont" key="NSSupport">
@@ -60,7 +60,7 @@
 							<int key="NSButtonFlags2">134</int>
 							<object class="NSCustomResource" key="NSNormalImage">
 								<string key="NSClassName">NSImage</string>
-								<string key="NSResourceName">gmail</string>
+								<string key="NSResourceName">nav</string>
 							</object>
 							<string key="NSAlternateContents"/>
 							<string key="NSKeyEquivalent"/>
diff --git a/chrome/browser/cocoa/tab_controller.mm b/chrome/browser/cocoa/tab_controller.mm
index 1c10172..0cb4603 100644
--- a/chrome/browser/cocoa/tab_controller.mm
+++ b/chrome/browser/cocoa/tab_controller.mm
@@ -26,6 +26,11 @@
   return self;
 }
 
+- (void)dealloc {
+  [image_ release];
+  [super dealloc];
+}
+
 // Called when the tab's nib is done loading and all outlets are hooked up.
 - (void)awakeFromNib {
   [[self view] addSubview:backgroundButton_
diff --git a/chrome/browser/cocoa/tab_strip_controller.h b/chrome/browser/cocoa/tab_strip_controller.h
index 1e76e40..b38bd19 100644
--- a/chrome/browser/cocoa/tab_strip_controller.h
+++ b/chrome/browser/cocoa/tab_strip_controller.h
@@ -30,7 +30,7 @@ class ToolbarModel;
  @private
   TabContents* currentTab_;   // weak, tab for which we're showing state
   TabStripView* tabView_;  // weak
-  NSButton* newTabButton_;
+  NSButton* newTabButton_;  // weak, obtained from the nib.
   TabStripBridge* bridge_;
   TabStripModel* tabModel_;  // weak
   ToolbarModel* toolbarModel_;  // weak, one per browser
diff --git a/chrome/browser/cocoa/tab_strip_controller.mm b/chrome/browser/cocoa/tab_strip_controller.mm
index 49cbb56..fc59a3e 100644
--- a/chrome/browser/cocoa/tab_strip_controller.mm
+++ b/chrome/browser/cocoa/tab_strip_controller.mm
@@ -76,18 +76,17 @@ class TabStripBridge : public TabStripModelObserver {
     tabContentsArray_ = [[NSMutableArray alloc] init];
     tabArray_ = [[NSMutableArray alloc] init];
 
-    // Create the new tab button separate from the nib so we can make sure
-    // it's always at the end of the subview list.
-    NSImage* image = [NSImage imageNamed:@"newtab"];
-    NSRect frame = NSMakeRect(0, 0, [image size].width, [image size].height);
-    newTabButton_ = [[NSButton alloc] initWithFrame:frame];
-    [newTabButton_ setImage:image];
-    [newTabButton_ setImagePosition:NSImageOnly];
+    // Take the only child view present in the nib as the new tab button. For
+    // some reason, if the view is present in the nib apriori, it draws
+    // correctly. If we create it in code and add it to the tab view, it draws
+    // with all sorts of crazy artifacts.
+    newTabButton_ = [[tabView_ subviews] objectAtIndex:0];
+    DCHECK([newTabButton_ isKindOfClass:[NSButton class]]);
     [newTabButton_ setTarget:nil];
     [newTabButton_ setAction:@selector(commandDispatch:)];
     [newTabButton_ setTag:IDC_NEW_TAB];
-    [newTabButton_ setButtonType:NSMomentaryPushInButton];
-    [newTabButton_ setBordered:NO];
+
+    [tabView_ setWantsLayer:YES];
   }
   return self;
 }
@@ -96,7 +95,6 @@ class TabStripBridge : public TabStripModelObserver {
   delete bridge_;
   [tabContentsArray_ release];
   [tabArray_ release];
-  [newTabButton_ release];
   [super dealloc];
 }
 
@@ -146,13 +144,21 @@ class TabStripBridge : public TabStripModelObserver {
 
 // Returns the index of the subview |view|. Returns -1 if not present.
 - (NSInteger)indexForTabView:(NSView*)view {
-  NSInteger index = -1;
-  const int numSubviews = [self numberOfTabViews];
-  for (int i = 0; i < numSubviews; i++) {
-    if ([[tabView_ subviews] objectAtIndex:i] == view)
-      index = i;
+  NSInteger index = 0;
+  for (TabController* current in tabArray_) {
+    if ([current view] == view)
+      return index;
+    ++index;
   }
-  return index;
+  return -1;
+}
+
+// Returns the view at the given index, using the array of TabControllers to
+// get the associated view. Returns nil if out of range.
+- (NSView*)viewAtIndex:(NSUInteger)index {
+  if (index >= [tabArray_ count])
+    return NULL;
+  return [[tabArray_ objectAtIndex:index] view];
 }
 
 // Called when the user clicks a tab. Tell the model the selection has changed,
@@ -172,13 +178,24 @@ class TabStripBridge : public TabStripModelObserver {
 
   short xOffset = kIndentLeavingSpaceForControls;
   if (index > 0) {
-    NSRect previousTab = [[[tabView_ subviews] objectAtIndex:index - 1] frame];
+    NSRect previousTab = [[self viewAtIndex:index - 1] frame];
     xOffset = NSMaxX(previousTab) - kTabOverlap;
   }
 
   return NSMakeRect(xOffset, 0, kNewTabWidth, [tabView_ frame].size.height);
 }
 
+// Positions the new tab button to the right of the last tab.
+- (void)positionNewTabButton {
+  const NSInteger kNewTabXOffset = 10;
+  NSRect lastTab = [[[tabArray_ lastObject] view] frame];
+  NSInteger maxRightEdge = NSMaxX(lastTab);
+  NSRect newTabButtonFrame = [newTabButton_ frame];
+  newTabButtonFrame.origin.x = maxRightEdge + kNewTabXOffset;
+  [newTabButton_ setFrame:newTabButtonFrame];
+  [newTabButton_ setHidden:NO];
+}
+
 // Handles setting the title of the tab based on the given |contents|. Uses
 // a canned string if |contents| is NULL.
 - (void)setTabTitle:(NSViewController*)tab withContents:(TabContents*)contents {
@@ -211,10 +228,6 @@ class TabStripBridge : public TabStripModelObserver {
           autorelease];
   [tabContentsArray_ insertObject:contentsController atIndex:index];
 
-  // Remove the new tab button so the only views present are the tabs,
-  // we'll add it back when we're done
-  [newTabButton_ removeFromSuperview];
-
   // Make a new tab and add it to the strip. Keep track of its controller.
   // TODO(pinkerton): move everyone else over and animate. Also will need to
   // move the "add tab" button over.
@@ -224,17 +237,9 @@ class TabStripBridge : public TabStripModelObserver {
   NSView* newView = [newController view];
   [tabView_ addSubview:newView];
 
-  [self setTabTitle:newController withContents:contents];
+  [self positionNewTabButton];
 
-  // Add the new tab button back in to the right of the last tab.
-  const NSInteger kNewTabXOffset = 10;
-  NSRect lastTab =
-    [[[tabView_ subviews] objectAtIndex:[[tabView_ subviews] count] - 1] frame];
-  NSInteger maxRightEdge = NSMaxX(lastTab);
-  NSRect newTabButtonFrame = [newTabButton_ frame];
-  newTabButtonFrame.origin.x = maxRightEdge + kNewTabXOffset;
-  [newTabButton_ setFrame:newTabButtonFrame];
-  [tabView_ addSubview:newTabButton_];
+  [self setTabTitle:newController withContents:contents];
 
   // Select the newly created tab if in the foreground
   if (inForeground)
@@ -248,12 +253,16 @@ class TabStripBridge : public TabStripModelObserver {
                       atIndex:(NSInteger)index
                   userGesture:(bool)wasUserGesture {
   // De-select all other tabs and select the new tab.
-  const int numSubviews = [self numberOfTabViews];
-  for (int i = 0; i < numSubviews; i++) {
-    TabController* current = [tabArray_ objectAtIndex:i];
+  int i = 0;
+  for (TabController* current in tabArray_) {
     [current setSelected:(i == index) ? YES : NO];
+    ++i;
   }
 
+  // Make this the top-most tab in the strips's z order.
+  NSView* selectedTab = [self viewAtIndex:index];
+  [tabView_ addSubview:selectedTab positioned:NSWindowAbove relativeTo:nil];
+
   // Tell the new tab contents it is about to become the selected tab. Here it
   // can do things like make sure the toolbar is up to date.
   TabContentsController* newController =
@@ -276,18 +285,31 @@ class TabStripBridge : public TabStripModelObserver {
   [tabContentsArray_ removeObjectAtIndex:index];
 
   // Remove the |index|th view from the tab strip
-  NSView* tab = [[tabView_ subviews] objectAtIndex:index];
+  NSView* tab = [self viewAtIndex:index];
   NSInteger tabWidth = [tab frame].size.width;
   [tab removeFromSuperview];
 
-  // Move all the views to the right the width of the tab that was closed.
-  // TODO(pinkerton): Animate!
-  const int numSubviews = [[tabView_ subviews] count];
-  for (int i = index; i < numSubviews; i++) {
-    NSView* curr = [[tabView_ subviews] objectAtIndex:i];
-    NSRect newFrame = [curr frame];
-    newFrame.origin.x -= tabWidth - kTabOverlap;
-    [curr setFrame:newFrame];
+  // Move all the views that are to the right of the tab being removed over
+  // the width of the tab that was closed. Don't bother animating if there is
+  // only 1 tab as everything is going away.
+  if ([self numberOfTabViews] > 1) {
+    int currIndex = 0;
+    for (TabController* curr in tabArray_) {
+      if (currIndex > index) {
+        NSView* shiftingView = [curr view];
+        NSRect newFrame = [shiftingView frame];
+        newFrame.origin.x -= tabWidth - kTabOverlap;
+        [[shiftingView animator] setFrame:newFrame];
+      }
+      ++currIndex;
+    }
+
+    // Move the new tab button. Note we can't just use the position of the
+    // last tab because it will still be at the old location due to the delay
+    // due to animation.
+    NSRect newTabFrame = [newTabButton_ frame];
+    newTabFrame.origin.x -= tabWidth - kTabOverlap;
+    [[newTabButton_ animator] setFrame:newTabFrame];
   }
 
   // Once we're totally done with the tab, delete its controller