Fix a bug where dragging a window that's been closed by JS back into another window would crash. Be more specific about when the window will be closed/deleted during dragging in hopes of making future crashes a little more clear.

BUG=none
TEST=dragging tabs between windows should work. tabs closed via window.close() on a timer should work.
Review URL: http://codereview.chromium.org/246035

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

5 files changed, 37 insertions, 8 deletions

diff --git a/chrome/browser/cocoa/browser_window_cocoa.mm b/chrome/browser/cocoa/browser_window_cocoa.mm
index fe4e024..a83e091 100644
--- a/chrome/browser/cocoa/browser_window_cocoa.mm
+++ b/chrome/browser/cocoa/browser_window_cocoa.mm
@@ -53,8 +53,13 @@ void BrowserWindowCocoa::SetBounds(const gfx::Rect& bounds) {
 // The controller implementing the window delegate methods called from
 // |-performClose:| must take precautions to ensure that.
 void BrowserWindowCocoa::Close() {
-  [window_ orderOut:controller_];
-  [window_ performClose:controller_];
+  // If there is an overlay window, we contain a tab being dragged between
+  // windows. Don't hide the window as it makes the UI extra confused. We can
+  // still close the window, as that will happen when the drag completes.
+  if ([controller_ overlayWindow])
+    [controller_ deferPerformClose];
+  else
+    [window_ performClose:controller_];
 }
 
 void BrowserWindowCocoa::Activate() {
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index b8236e4..95fc140 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -707,6 +707,10 @@ willPositionSheet:(NSWindow*)sheet
     int index = [dragBWC->tabStripController_ indexForTabView:view];
     TabContents* contents =
         dragBWC->browser_->tabstrip_model()->GetTabContentsAt(index);
+    // The tab contents may have gone away if given a window.close() while it
+    // is being dragged. If so, bail, we've got nothing to drop.
+    if (!contents)
+      return;
 
     // Now that we have enough information about the tab, we can remove it from
     // the dragging window. We need to do this *before* we add it to the new
diff --git a/chrome/browser/cocoa/tab_view.mm b/chrome/browser/cocoa/tab_view.mm
index 6e741da..f44a482 100644
--- a/chrome/browser/cocoa/tab_view.mm
+++ b/chrome/browser/cocoa/tab_view.mm
@@ -491,12 +491,14 @@ static const CGFloat kRapidCloseDist = 2.5;
     [targetController_ showWindow:nil];
   } else {
     // Tab dragging did move window only.
-    [dragWindow_ setAlphaValue:1.0];
-    [dragOverlay_ setHasShadow:NO];
-    [dragWindow_ setHasShadow:YES];
     [draggedController_ removeOverlay];
-    [dragWindow_ makeKeyAndOrderFront:nil];
-
+    // Don't want to re-show the window if it was closed during the drag.
+    if ([dragWindow_ isVisible]) {
+      [dragWindow_ setAlphaValue:1.0];
+      [dragOverlay_ setHasShadow:NO];
+      [dragWindow_ setHasShadow:YES];
+      [dragWindow_ makeKeyAndOrderFront:nil];
+    }
     [[draggedController_ window] setLevel:NSNormalWindowLevel];
     [draggedController_ removePlaceholder];
   }
diff --git a/chrome/browser/cocoa/tab_window_controller.h b/chrome/browser/cocoa/tab_window_controller.h
index 9025018..17c1a543 100644
--- a/chrome/browser/cocoa/tab_window_controller.h
+++ b/chrome/browser/cocoa/tab_window_controller.h
@@ -35,12 +35,14 @@
                                // view is the proper content area in the overlay
                                // (weak)
   scoped_nsobject<NSMutableSet> lockedTabs_;
+  BOOL closeDeferred_;  // If YES, call performClose: in removeOverlay:.
 }
 @property(readonly, nonatomic) TabStripView* tabStripView;
 @property(readonly, nonatomic) NSView* tabContentArea;
 
 // Used during tab dragging to turn on/off the overlay window when a tab
-// is torn off.
+// is torn off. If -deferPerformClose (below) is used, -removeOverlay will
+// cause the controller to be autoreleased before returning.
 - (void)showOverlay;
 - (void)removeOverlay;
 - (void)removeOverlayAfterDelay:(NSTimeInterval)delay;
@@ -109,9 +111,15 @@
 // default implementation returns YES.
 - (BOOL)isNormalWindow;
 
+// Get/set whether a particular tab is draggable between windows.
 - (BOOL)isTabDraggable:(NSView*)tabView;
 - (void)setTab:(NSView*)tabView isDraggable:(BOOL)draggable;
 
+// Tell the window that it needs to call performClose: as soon as the current
+// drag is complete. This prevents a window (and its overlay) from going away
+// during a drag.
+- (void)deferPerformClose;
+
 @end
 
 @interface TabWindowController(ProtectedMethods)
diff --git a/chrome/browser/cocoa/tab_window_controller.mm b/chrome/browser/cocoa/tab_window_controller.mm
index 5e9ce23..e405887 100644
--- a/chrome/browser/cocoa/tab_window_controller.mm
+++ b/chrome/browser/cocoa/tab_window_controller.mm
@@ -42,8 +42,11 @@
 
 - (void)removeOverlay {
   [self setUseOverlay:NO];
+  if (closeDeferred_)
+    [[self window] performClose:self];  // Autoreleases the controller.
 }
 
+// TODO(pinkerton): Nobody calls this, can we remove it?
 - (void)removeOverlayAfterDelay:(NSTimeInterval)delay {
   [NSObject cancelPreviousPerformRequestsWithTarget:self
                                            selector:@selector(removeOverlay)
@@ -199,4 +202,11 @@
     [lockedTabs_ addObject:tabView];
 }
 
+// Tell the window that it needs to call performClose: as soon as the current
+// drag is complete. This prevents a window (and its overlay) from going away
+// during a drag.
+- (void)deferPerformClose {
+  closeDeferred_ = YES;
+}
+
 @end