Fixes tab contents crash. This changes a number of things in dragged...

chrome/browser/views/tabs/dragged_tab_controller.cc

Index: chrome/browser/views/tabs/dragged_tab_controller.cc
===================================================================
--- chrome/browser/views/tabs/dragged_tab_controller.cc	(revision 8729)
+++ chrome/browser/views/tabs/dragged_tab_controller.cc	(working copy)
@@ -399,16 +399,23 @@
 void DraggedTabController::ReplaceContents(TabContents* source,
                                            TabContents* new_contents) {
   DCHECK(dragged_contents_ == source);
-  source->set_delegate(NULL);
-  new_contents->set_delegate(this);
 
   // If we're attached to a TabStrip, we need to tell the TabStrip that this
   // TabContents was replaced.
-  if (attached_tabstrip_ && attached_tabstrip_->model() && dragged_contents_) {
-    int index =
-        attached_tabstrip_->model()->GetIndexOfTabContents(dragged_contents_);
-    if (index != TabStripModel::kNoTab)
-      attached_tabstrip_->model()->ReplaceTabContentsAt(index, new_contents);
+  if (attached_tabstrip_ && dragged_contents_) {
+    if (original_delegate_) {
+      original_delegate_->ReplaceContents(source, new_contents);
+      // ReplaceContents on the original delegate is going to reset the delegate
+      // for us. We need to unset original_delegate_ here so that
+      // ChangeDraggedContents doesn't attempt to restore the delegate to the
+      // wrong value.
+      original_delegate_ = NULL;
+    } else if (attached_tabstrip_->model()) {
+      int index =
+          attached_tabstrip_->model()->GetIndexOfTabContents(dragged_contents_);
+      if (index != TabStripModel::kNoTab)
+        attached_tabstrip_->model()->ReplaceTabContentsAt(index, new_contents);
+    }
   }
 
   // Update our internal state.
@@ -557,12 +564,22 @@
     NotificationService::current()->RemoveObserver(this,
       NOTIFY_TAB_CONTENTS_DESTROYED,
       Source<TabContents>(dragged_contents_));
+    if (original_delegate_)
+      dragged_contents_->set_delegate(original_delegate_);
   }
+  original_delegate_ = NULL;
   dragged_contents_ = new_contents;
   if (dragged_contents_) {
     NotificationService::current()->AddObserver(this,
       NOTIFY_TAB_CONTENTS_DESTROYED,
       Source<TabContents>(dragged_contents_));
+
+    // We need to be the delegate so we receive messages about stuff,
+    // otherwise our dragged_contents() may be replaced and subsequently
+    // collected/destroyed while the drag is in process, leading to
+    // nasty crashes.
+    original_delegate_ = dragged_contents_->delegate();
+    dragged_contents_->set_delegate(this);
   }
 }
 
@@ -820,11 +837,7 @@
   if (view_.get())
     view_->Detach(photobooth_.get());
 
-  // We need to be the delegate so we receive messages about stuff,
-  // otherwise our dragged_contents() may be replaced and subsequently
-  // collected/destroyed while the drag is in process, leading to
-  // nasty crashes.
-  original_delegate_ = dragged_contents_->delegate();
+  // Detaching resets the delegate, but we still want to be the delegate.
   dragged_contents_->set_delegate(this);
 
   attached_tabstrip_ = NULL;
@@ -922,6 +935,12 @@
 }
 
 bool DraggedTabController::EndDragImpl(EndDragType type) {
+  // WARNING: this may be invoked multiple times. In particular, if deletion
+  // occurs after a delay (as it does when the tab is released in the original
+  // tab strip) and the navigation controller/tab contents is deleted before
+  // the animation finishes, this is invoked twice. The second time through
+  // type == TAB_DESTROYED.
+
   bring_to_front_timer_.Stop();
 
   // Hide the current dock controllers.
@@ -947,6 +966,8 @@
         destroy_now = CompleteDrag();
       }
     }
+    if (dragged_contents_ && dragged_contents_->delegate() == this)
+      dragged_contents_->set_delegate(original_delegate_);
   } else {
     // If we get here it means the NavigationController is going down. Don't
     // attempt to do any cleanup other than resetting the delegate (if we're
@@ -954,8 +975,14 @@
     if (dragged_contents_ && dragged_contents_->delegate() == this)
       dragged_contents_->set_delegate(NULL);
     dragged_contents_ = NULL;
-    attached_tabstrip_ = NULL;
   }
+
+  // The delegate of the dragged contents should have been reset. Unset the
+  // original delegate so that we don't attempt to reset the delegate when
+  // deleted.
+  DCHECK(!dragged_contents_ || dragged_contents_->delegate() != this);
+  original_delegate_ = NULL;
+
   // If we're not destroyed now, we'll be destroyed asynchronously later.
   if (destroy_now)
     source_tabstrip_->DestroyDragController();