Applying patch for Mohamed Mansour (reviewed by pkasting): http://codereview.chromium.org/16246

CRASH at Tab::OnMouseReleased

Some strange thing is happening that it crashes for view::HitTest, it can never
ENTER that function. It seems that the tab is being destroyed and as pkasting
stated on IRC: 
"Part of this may be because our retarded Views designs can't distinguish which
buttons are being held versus clicked in these sorts of cases (I filed a bug on
this about a year and a half ago, internally)"

When a tab has ended dragging (EndDrag), if something was being dragged and you
end it, it actually cleans the TabDelegate and assigns its value to 'freed
memory' which is 0xfeeefeee. Therefore it crashes while dragging, because the
object no longer exists. I couldn't do if (delegate_) cause that always returns
true since delegate has 0xfeeefeee. 

So I just changed the return type from void to bool for underlying EndDragImpl
and pumped it to tab.cc. That way, we can know if a tab is destroyed or not.

BUG=5819 (http://crbug.com/5819)

TEST=Dragging tabs around, closing while dragging, and closing tabs.


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

6 files changed, 22 insertions, 16 deletions

diff --git a/chrome/browser/views/tabs/dragged_tab_controller.cc b/chrome/browser/views/tabs/dragged_tab_controller.cc
index 2b3c22c..f0b6d4a 100644
--- a/chrome/browser/views/tabs/dragged_tab_controller.cc
+++ b/chrome/browser/views/tabs/dragged_tab_controller.cc
@@ -355,8 +355,8 @@ void DraggedTabController::Drag() {
   }
 }
 
-void DraggedTabController::EndDrag(bool canceled) {
-  EndDragImpl(canceled ? CANCELED : NORMAL);
+bool DraggedTabController::EndDrag(bool canceled) {
+  return EndDragImpl(canceled ? CANCELED : NORMAL);
 }
 
 Tab* DraggedTabController::GetDragSourceTabForContents(
@@ -921,7 +921,7 @@ Tab* DraggedTabController::GetTabMatchingDraggedContents(
   return index == TabStripModel::kNoTab ? NULL : tabstrip->GetTabAt(index);
 }
 
-void DraggedTabController::EndDragImpl(EndDragType type) {
+bool DraggedTabController::EndDragImpl(EndDragType type) {
   bring_to_front_timer_.Stop();
 
   // Hide the current dock controllers.
@@ -959,6 +959,8 @@ void DraggedTabController::EndDragImpl(EndDragType type) {
   // If we're not destroyed now, we'll be destroyed asynchronously later.
   if (destroy_now)
     source_tabstrip_->DestroyDragController();
+
+  return destroy_now;
 }
 
 void DraggedTabController::RevertDrag() {
diff --git a/chrome/browser/views/tabs/dragged_tab_controller.h b/chrome/browser/views/tabs/dragged_tab_controller.h
index 8141c67..a509aff 100644
--- a/chrome/browser/views/tabs/dragged_tab_controller.h
+++ b/chrome/browser/views/tabs/dragged_tab_controller.h
@@ -57,8 +57,8 @@ class DraggedTabController : public TabContentsDelegate,
   // Complete the current drag session. If the drag session was canceled
   // because the user pressed Escape or something interrupted it, |canceled|
   // is true so the helper can revert the state to the world before the drag
-  // begun.
-  void EndDrag(bool canceled);
+  // begun. Returns whether the tab has been destroyed.
+  bool EndDrag(bool canceled);
 
   // Retrieve the source Tab if the TabContents specified matches the one being
   // dragged by this controller, or NULL if the specified TabContents is not
@@ -186,8 +186,8 @@ class DraggedTabController : public TabContentsDelegate,
   // dragged TabContents.
   Tab* GetTabMatchingDraggedContents(TabStrip* tabstrip) const;
 
-  // Does the work for EndDrag.
-  void EndDragImpl(EndDragType how_end);
+  // Does the work for EndDrag. Returns whether the tab has been destroyed.
+  bool EndDragImpl(EndDragType how_end);
 
   // If the drag was aborted for some reason, this function is called to un-do
   // the changes made during the drag operation.
diff --git a/chrome/browser/views/tabs/tab.cc b/chrome/browser/views/tabs/tab.cc
index 5591869..cdc3502 100644
--- a/chrome/browser/views/tabs/tab.cc
+++ b/chrome/browser/views/tabs/tab.cc
@@ -178,10 +178,14 @@ bool Tab::OnMouseDragged(const views::MouseEvent& event) {
 void Tab::OnMouseReleased(const views::MouseEvent& event, bool canceled) {
   // Notify the drag helper that we're done with any potential drag operations.
   // Clean up the drag helper, which is re-created on the next mouse press.
-  delegate_->EndDrag(canceled);
+  // In some cases, ending the drag will schedule the tab for destruction; if
+  // so, bail immediately, since our members are already dead and we shouldn't
+  // do anything else except drop the tab where it is.
+  if (delegate_->EndDrag(canceled))
+    return;
 
-  // Close tabs on middle click, but only if the button is released over the tab 
-  // (normal windows behavior is to discard presses of a UI element where the 
+  // Close tab on middle click, but only if the button is released over the tab
+  // (normal windows behavior is to discard presses of a UI element where the
   // releases happen off the element).
   if (event.IsMiddleMouseButton() && HitTest(event.location()))
     delegate_->CloseTab(this);
diff --git a/chrome/browser/views/tabs/tab.h b/chrome/browser/views/tabs/tab.h
index 17134e1..de3c981 100644
--- a/chrome/browser/views/tabs/tab.h
+++ b/chrome/browser/views/tabs/tab.h
@@ -66,8 +66,9 @@ class Tab : public TabRenderer,
     virtual void ContinueDrag(const views::MouseEvent& event) = 0;
 
     // Ends dragging a Tab. |canceled| is true if the drag was aborted in a way
-    // other than the user releasing the mouse.
-    virtual void EndDrag(bool canceled) = 0;
+    // other than the user releasing the mouse. Returns whether the tab has been
+    // destroyed.
+    virtual bool EndDrag(bool canceled) = 0;
   };
 
   explicit Tab(TabDelegate* delegate);
diff --git a/chrome/browser/views/tabs/tab_strip.cc b/chrome/browser/views/tabs/tab_strip.cc
index 5569de3..eff78d2 100644
--- a/chrome/browser/views/tabs/tab_strip.cc
+++ b/chrome/browser/views/tabs/tab_strip.cc
@@ -1022,9 +1022,8 @@ void TabStrip::ContinueDrag(const views::MouseEvent& event) {
     drag_controller_->Drag();
 }
 
-void TabStrip::EndDrag(bool canceled) {
-  if (drag_controller_.get())
-    drag_controller_->EndDrag(canceled);
+bool TabStrip::EndDrag(bool canceled) {
+  return drag_controller_.get() ? drag_controller_->EndDrag(canceled) : false;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/views/tabs/tab_strip.h b/chrome/browser/views/tabs/tab_strip.h
index e3389ab..36398c7 100644
--- a/chrome/browser/views/tabs/tab_strip.h
+++ b/chrome/browser/views/tabs/tab_strip.h
@@ -141,7 +141,7 @@ class TabStrip : public views::View,
   virtual void StopAllHighlighting();
   virtual void MaybeStartDrag(Tab* tab, const views::MouseEvent& event);
   virtual void ContinueDrag(const views::MouseEvent& event);
-  virtual void EndDrag(bool canceled);
+  virtual bool EndDrag(bool canceled);
 
   // views::Button::ButtonListener implementation:
   virtual void ButtonPressed(views::BaseButton* sender);