fix bug 262815 - Closing tab while bookmarks view is shown can lead to crash [@ objc_msgSend]. sr=pinkerton

camino/src/browser/BrowserTabBarView.h

@@ -56,7 +56,8 @@
   NSView *mDragDest;
   TabButtonCell *mDragDestButton;
   
-  BOOL mOverflowTabs;
+  BOOL mVisible;  // whether tabs are visible or not; used to disable creation of tracking rects when they're not
+  BOOL mOverflowTabs; // track whether there are more tabs than we can fit onscreen
   NSMutableArray *mTrackingCells; // cells which currently have tracking rects in this view
 }
 // destroy the tab bar and recreate it from the tabview
@@ -64,8 +65,12 @@
 // return the height the tab bar should be
 -(float)tabBarHeight;
 -(BrowserTabViewItem*)tabViewItemAtPoint:(NSPoint)location;
-- (void)windowClosed;
-- (IBAction)overflowMenu:(id)sender;
+-(void)windowClosed;
+-(IBAction)overflowMenu:(id)sender;
+-(BOOL)isVisible;
+// show or hide tabs- should be called if this view will be hidden, to give it a chance to register or
+// unregister tracking rects as appropriate
+-(void)setVisible:(BOOL)show;
 
 @end
 

camino/src/browser/BrowserTabBarView.mm

@@ -75,6 +75,10 @@ static const int kOverflowButtonMargin = 1;
     mActiveTabButton = nil;
     mOverflowButton = nil;
     mOverflowTabs = NO;
+    // initialize to YES so that awakeFromNib: will set the right size; awakeFromNib uses setVisible which
+    // will only be effective if visibility changes. initializing to YES causes the right thing to happen even
+    // if this view is visible in a nib file.
+    mVisible = YES;
     // this will not likely have any result here
     [self rebuildTabBar];
     [self registerForDraggedTypes:[NSArray arrayWithObjects: @"MozURLType", @"MozBookmarkType", NSStringPboardType,
@@ -85,6 +89,8 @@ static const int kOverflowButtonMargin = 1;
 
 -(void)awakeFromNib
 {
+  // start off with the tabs hidden, and allow our controller to show or hide as appropriate.
+  [self setVisible:NO];
   // this needs to be called again since our tabview should be non-nil now
   [self rebuildTabBar];
 }
@@ -261,7 +267,7 @@ static const int kOverflowButtonMargin = 1;
 // allows tab button cells to react to mouse events
 -(void)registerTabButtonsForTracking
 {
-  if ([self window]) {
+  if ([self window] && mVisible) {
     NSArray * tabItems = [mTabView tabViewItems];
     if(mTrackingCells) [self unregisterTabButtonsForTracking];
     mTrackingCells = [NSMutableArray arrayWithCapacity:[tabItems count]];
@@ -420,6 +426,38 @@ static const int kOverflowButtonMargin = 1;
   rect.size.width -= 2 * kTabBarMargin + (mOverflowTabs ? kOverflowButtonWidth : 0.0);
   return rect;
 }
+
+-(BOOL)isVisible
+{
+  return mVisible;
+}
+
+// show or hide tabs- should be called if this view will be hidden, to give it a chance to register or
+// unregister tracking rects as appropriate.
+//
+// Does not actually remove the view from the hierarchy; simply hides it.
+-(void)setVisible:(BOOL)show
+{
+  // only change anything if the new state is different from the current state
+  if (show && !mVisible) {
+    mVisible = show;
+    NSRect newFrame = [self frame];
+    newFrame.size.height = [self tabBarHeight];
+    [self setFrame:newFrame];
+    [self rebuildTabBar];
+    // set up tracking rects
+    [self registerTabButtonsForTracking];
+  } else if (!show && mVisible) { // being hi
+    mVisible = show;
+    NSRect newFrame = [self frame];
+    newFrame.size.height = 0.0;
+    [self setFrame:newFrame];
+    // destroy tracking rects
+    [self unregisterTabButtonsForTracking];
+  }
+}
+    
+    
     
 
 #pragma mark -

camino/src/browser/BrowserTabView.h

@@ -26,6 +26,7 @@
   BOOL autoHides;
   BOOL mIsDropTarget;
   BOOL mLastClickIsPotentialDrag;
+  BOOL mVisible; // YES if the view is in the hierarchy
   int  maxNumberOfTabs;			// 0 means 'no max'
   IBOutlet BrowserTabBarView * mTabBar;
 }
@@ -48,7 +49,9 @@
 
 - (BrowserTabViewItem*)itemWithTag:(int)tag;
 - (void)refreshTabBar:(BOOL)rebuild;
-
+- (BOOL)isVisible;
+// inform the view that it will be shown or hidden; e.g. prior to showing or hiding the bookmarks
+- (void)setVisible:(BOOL)show;
 - (void)windowClosed;
 
 @end

camino/src/browser/BrowserTabView.mm

@@ -58,7 +58,6 @@
 //////////////////////////
 
 @interface BrowserTabView (Private)
-
 - (void)showOrHideTabsAsAppropriate;
 - (BOOL)handleDropOnTab:(NSTabViewItem*)overTabViewItem overContent:(BOOL)overContentArea withURL:(NSString*)url;
 - (BrowserTabViewItem*)getTabViewItemFromWindowPoint:(NSPoint)point;
@@ -81,12 +80,14 @@
     if ( (self = [super initWithFrame:frameRect]) ) {
       autoHides = YES;
       maxNumberOfTabs = 0;		// no max
+      //mVisible = YES;
     }
     return self;
 }
 
 - (void)awakeFromNib
 {
+    mVisible = YES;
     [self showOrHideTabsAsAppropriate];
     [self registerForDraggedTypes:[NSArray arrayWithObjects:
         @"MozURLType", @"MozBookmarkType", NSStringPboardType, NSFilenamesPboardType, NSURLPboardType, nil]];
@@ -214,33 +215,32 @@
 // Only to be used with the 2 types of tab view which we use in Camino.
 - (void)showOrHideTabsAsAppropriate
 {
-  //if ( autoHides == YES )
-  {
+  // don't bother if we're not visible
+  if (mVisible) {
     BOOL tabVisibilityChanged = NO;
-    BOOL tabsVisible = NO;
+    BOOL tabsVisible = [mTabBar isVisible];
+    int numItems = [[self tabViewItems] count];
     
-    if ([[self tabViewItems] count] < 2) {
-      if ([mTabBar frame].size.height != 0)
-        tabVisibilityChanged = YES;
-      tabsVisible = NO;
-    } else {
-      if ([mTabBar frame].size.height == 0)
-        tabVisibilityChanged = YES;
-      tabsVisible = YES;
-    }
-
-    // tell the tabs that visibility changed
-    NSArray* tabViewItems = [self tabViewItems];
-    for (unsigned int i = 0; i < [tabViewItems count]; i ++) {
-      NSTabViewItem* tabItem = [tabViewItems objectAtIndex:i];
-      if ([tabItem isMemberOfClass:[BrowserTabViewItem class]])
-        [(BrowserTabViewItem*)tabItem updateTabVisibility:tabsVisible];
+    if (numItems < 2 && tabsVisible) {
+      tabVisibilityChanged = YES;
+      // hide the tabs and give the view a chance to kill its tracking rects
+      [mTabBar setVisible:NO];
+    } else if (numItems >= 2 && !tabsVisible) {
+      // show the tabs allow the view to set up tracking rects
+      [mTabBar setVisible:YES];
+      tabVisibilityChanged = YES;
     }
+    tabsVisible = [mTabBar isVisible];
     
     if (tabVisibilityChanged) {
-      NSRect newTabBarFrame = [mTabBar frame];
-      newTabBarFrame.size.height = tabsVisible ? [mTabBar tabBarHeight]:0.0;
-      [mTabBar setFrame:newTabBarFrame];
+      // tell the tabs that visibility changed
+      NSArray* tabViewItems = [self tabViewItems];
+      for (unsigned int i = 0; i < numItems; i ++) {
+        NSTabViewItem* tabItem = [tabViewItems objectAtIndex:i];
+        if ([tabItem isMemberOfClass:[BrowserTabViewItem class]])
+          [(BrowserTabViewItem*)tabItem updateTabVisibility:tabsVisible];
+      }
+      
       // tell the superview to resize its subviews
       [[self superview] resizeSubviewsWithOldSize:[[self superview] frame].size];
       [self setNeedsDisplay:YES];
@@ -265,7 +265,24 @@
 
 - (BOOL)tabsVisible
 {
-  return ([[self tabViewItems] count] > 1);
+  return ([mTabBar isVisible]);
+}
+
+- (BOOL)isVisible
+{
+  return mVisible;
+}
+
+// inform the view that it will be shown or hidden; e.g. prior to showing or hiding the bookmarks
+- (void)setVisible:(BOOL)show
+{
+  mVisible = show;
+  // if the tabs' visibility is different, and we're being, hidden explicitly hide them.
+  // otherwise show or hide them according to current settings
+  if (([mTabBar isVisible] != mVisible) && !mVisible)
+    [mTabBar setVisible:show];
+  else
+    [self showOrHideTabsAsAppropriate];
 }
 
 - (BOOL)handleDropOnTab:(NSTabViewItem*)overTabViewItem overContent:(BOOL)overContentArea withURL:(NSString*)url

camino/src/browser/BrowserWindowController.mm

@@ -3250,6 +3250,8 @@ enum BWCOpenDest {
   
   // deactivate any gecko view that might think it has focus
   if ([self isResponderGeckoView:[[self window] firstResponder]]) {
+    // inform the tab view that it will be hidden so that it can perform any necessary cleanup
+    [mTabBrowser setVisible:NO];
     CHBrowserView* browserView = [mBrowserView getBrowserView];
     if (browserView)
       [browserView setActive:NO];
@@ -3277,7 +3279,11 @@ enum BWCOpenDest {
     CHBrowserView* browserView = [mBrowserView getBrowserView];
     if (browserView)
       [browserView setActive:YES];
+    // inform the tab view that it will be visible, so that it can adjust to any changes that occurred
+    // when it was out of the hierarchy
+    [mTabBrowser setVisible:YES];
   }
+  
 
   // we have to manually update the bookmarks menu items, because we
   // turn autoenabling off for that menu