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

chrome/browser/views/tabs/dragged_tab_controller.cc

 // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <math.h>
 #include <set>
 
 #include "chrome/browser/views/tabs/dragged_tab_controller.h"
 
 #include "chrome/browser/browser_window.h"
@@ skipping 381 matching lines @@
 
 void DraggedTabController::NavigationStateChanged(const TabContents* source,
                                                   unsigned changed_flags) {
   if (view_.get())
     view_->Update();
 }
 
 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.
   ChangeDraggedContents(new_contents);
 
   if (view_.get())
     view_->Update();
 }
 
 void DraggedTabController::AddNewContents(TabContents* source,
@@ skipping 128 matching lines @@
     dock_controllers_.back()->UpdateInEnabledArea(dock_info_.in_enable_area());
   }
 }
 
 
 void DraggedTabController::ChangeDraggedContents(TabContents* new_contents) {
   if (dragged_contents_) {
     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);
   }
 }
 
 void DraggedTabController::SaveFocus() {
   if (!old_focused_view_) {
     old_focused_view_ = source_tab_->GetRootView()->GetFocusedView();
     source_tab_->GetRootView()->FocusView(source_tab_);
   }
 }
 
@@ skipping 237 matching lines @@
   // TabContents.
   if (!photobooth_.get())
     photobooth_.reset(new HWNDPhotobooth(dragged_contents_->GetContainerHWND()));
 
   // Update the View. This NULL check is necessary apparently in some
   // conditions during automation where the view_ is destroyed inside a
   // function call preceding this point but after it is created.
   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;
 }
 
 int DraggedTabController::GetInsertionIndexForDraggedBounds(
     const gfx::Rect& dragged_bounds) const {
   int right_tab_x = 0;
 
   // If the UI layout of the tab strip is right-to-left, we need to mirror the
@@ skipping 77 matching lines @@
 }
 
 
 Tab* DraggedTabController::GetTabMatchingDraggedContents(
     TabStrip* tabstrip) const {
   int index = tabstrip->model()->GetIndexOfTabContents(dragged_contents_);
   return index == TabStripModel::kNoTab ? NULL : tabstrip->GetTabAt(index);
 }
 
 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.
   for (size_t i = 0; i < dock_controllers_.size(); ++i) {
     // Be sure and clear the controller first, that way if Hide ends up
     // deleting the controller it won't call us back.
     dock_controllers_[i]->clear_controller();
     dock_controllers_[i]->Hide();
   }
   dock_controllers_.clear();
   dock_windows_.clear();
 
   bool destroy_now = true;
   if (type != TAB_DESTROYED) {
     // We only finish up the drag if we were actually dragging. If we never
     // constructed a view, the user just clicked and released and didn't move the
     // mouse enough to trigger a drag.
     if (view_.get()) {
       RestoreFocus();
       if (type == CANCELED) {
         RevertDrag();
       } else {
         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
     // still the delegate).
     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();
 
   return destroy_now;
 }
 
 void DraggedTabController::RevertDrag() {
   // We save this here because code below will modify |attached_tabstrip_|.
   bool restore_frame = attached_tabstrip_ != source_tabstrip_;
@@ skipping 224 matching lines @@
     // Move the window to the front.
     SetWindowPos(hwnd, HWND_TOP, 0, 0, 0, 0,
                  SWP_NOACTIVATE | SWP_NOMOVE | SWP_NOSIZE);
 
     // The previous call made the window appear on top of the dragged window,
     // move the dragged window to the front.
     SetWindowPos(view_->GetWidget()->GetHWND(), HWND_TOP, 0, 0, 0, 0,
                  SWP_NOACTIVATE | SWP_NOMOVE | SWP_NOSIZE);
   }
 }