GTK: fix a tab dragging crasher.

The crash was a result of the early exit in EndDragImpl. When EndDragImpl is called due to a TAB_CONTENTS_DESTROYED notification and we exit early the DragController() doesn't get destroyed. We get a Drag() call afterwords, and call into a stale dragged_contents_. I fixed this by removing the early exit (instead checking dragged_tab_ before trying to clean up the drag, in case it hadn't started).

The change to EnsureDraggedTab() should not have any functional effect; it just makes the control flow clearer.

The new dragged_controller_ check should also not have any functional effect; it just seems safer to check it.

BUG=25326
TEST=manual (see bug for repro steps)
Review URL: http://codereview.chromium.org/668010

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

2 files changed, 19 insertions, 22 deletions

diff --git a/chrome/browser/gtk/tabs/dragged_tab_controller_gtk.cc b/chrome/browser/gtk/tabs/dragged_tab_controller_gtk.cc
index 9835423..5e2c110 100644
--- a/chrome/browser/gtk/tabs/dragged_tab_controller_gtk.cc
+++ b/chrome/browser/gtk/tabs/dragged_tab_controller_gtk.cc
@@ -74,12 +74,14 @@ void DraggedTabControllerGtk::CaptureDragInfo(const gfx::Point& mouse_offset) {
 }
 
 void DraggedTabControllerGtk::Drag() {
-  if (!source_tab_)
+  if (!source_tab_ || !dragged_contents_)
     return;
 
   bring_to_front_timer_.Stop();
   pin_timer_.Stop();
 
+  EnsureDraggedTab();
+
   // Before we get to dragging anywhere, ensure that we consider ourselves
   // attached to the source tabstrip.
   if (source_tab_->IsVisible()) {
@@ -239,8 +241,6 @@ void DraggedTabControllerGtk::SetDraggedContents(TabContents* new_contents) {
 }
 
 void DraggedTabControllerGtk::ContinueDragging() {
-  EnsureDraggedTab();
-
   // TODO(jhawkins): We don't handle the situation where the last tab is dragged
   // out of a window, so we'll just go with the way Windows handles dragging for
   // now.
@@ -476,7 +476,6 @@ void DraggedTabControllerGtk::Attach(TabStripGtk* attached_tabstrip,
   double unselected_width = 0, selected_width = 0;
   attached_tabstrip_->GetDesiredTabWidths(tab_count, pinned_tab_count,
                                           &unselected_width, &selected_width);
-  EnsureDraggedTab();
   int dragged_tab_width = static_cast<int>(selected_width);
   dragged_tab_->set_tab_width(dragged_tab_width);
   bool pinned = false;
@@ -697,12 +696,6 @@ TabGtk* DraggedTabControllerGtk::GetTabMatchingDraggedContents(
 }
 
 bool DraggedTabControllerGtk::EndDragImpl(EndDragType type) {
-  // In gtk, it's possible to receive a drag-begin signal and an drag-end signal
-  // without ever getting a drag-motion signal.  In this case, dragged_tab_ has
-  // never been created, so bail out.
-  if (!dragged_tab_.get())
-    return true;
-
   pin_timer_.Stop();
   bring_to_front_timer_.Stop();
 
@@ -713,22 +706,27 @@ bool DraggedTabControllerGtk::EndDragImpl(EndDragType type) {
   // type == TAB_DESTROYED.
 
   bool destroy_now = true;
-  if (type != TAB_DESTROYED) {
-    if (type == CANCELED) {
-      RevertDrag();
-    } else {
-      destroy_now = CompleteDrag();
-    }
-
-    if (dragged_contents_ && dragged_contents_->delegate() == this)
-      dragged_contents_->set_delegate(original_delegate_);
-  } else {
+  if (type == TAB_DESTROYED) {
     // 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;
+  } else {
+    // If we never received a drag-motion event, the drag will never have
+    // started in the sense that |dragged_tab_| will be NULL. We don't need to
+    // revert or complete the drag in that case.
+    if (dragged_tab_.get()) {
+      if (type == CANCELED) {
+        RevertDrag();
+      } else {
+        destroy_now = CompleteDrag();
+      }
+    }
+
+    if (dragged_contents_ && dragged_contents_->delegate() == this)
+      dragged_contents_->set_delegate(original_delegate_);
   }
 
   // The delegate of the dragged contents should have been reset. Unset the
diff --git a/chrome/browser/gtk/tabs/tab_strip_gtk.cc b/chrome/browser/gtk/tabs/tab_strip_gtk.cc
index 3a811c8..b038fc8 100644
--- a/chrome/browser/gtk/tabs/tab_strip_gtk.cc
+++ b/chrome/browser/gtk/tabs/tab_strip_gtk.cc
@@ -667,8 +667,7 @@ bool TabStripGtk::IsAnimating() const {
 }
 
 void TabStripGtk::DestroyDragController() {
-  if (IsDragSessionActive())
-    drag_controller_.reset(NULL);
+  drag_controller_.reset();
 }
 
 void TabStripGtk::DestroyDraggedSourceTab(TabGtk* tab) {