Fixes crash in tab dragging. Specifically capture wasn't being reset

correctly so that when we activated a window we would prematurely
cancelling the drag and end up in a bad state.

BUG=129460
TEST=covered by tests now
R=ben@chromium.org

Review URL: https://chromiumcodereview.appspot.com/10443016

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

2 files changed, 51 insertions, 2 deletions

diff --git a/chrome/browser/ui/views/tabs/tab_drag_controller.cc b/chrome/browser/ui/views/tabs/tab_drag_controller.cc
index 4b06ea6b..0554ec1 100644
--- a/chrome/browser/ui/views/tabs/tab_drag_controller.cc
+++ b/chrome/browser/ui/views/tabs/tab_drag_controller.cc
@@ -1117,6 +1117,14 @@ void TabDragController::Attach(TabStrip* attached_tabstrip,
                        tabs[source_tab_index_]->width());
   mouse_offset_.set_x(new_x);
 
+  // Transfer ownership of us to the new tabstrip as well as making sure the
+  // window has mouse capture. This is important so that if activation changes
+  // the drag isn't prematurely canceled.
+  if (detach_into_browser_) {
+    attached_tabstrip_->GetWidget()->SetMouseCapture(attached_tabstrip_);
+    attached_tabstrip_->OwnDragController(this);
+  }
+
   // Redirect all mouse events to the TabStrip so that the tab that originated
   // the drag can safely be deleted.
   if (detach_into_browser_ || attached_tabstrip_ == source_tabstrip_) {
@@ -1127,6 +1135,13 @@ void TabDragController::Attach(TabStrip* attached_tabstrip,
 }
 
 void TabDragController::Detach() {
+  // Release ownership of the drag controller and mouse capture. When we
+  // reattach ownership is transfered.
+  if (detach_into_browser_) {
+    attached_tabstrip_->ReleaseDragController();
+    attached_tabstrip_->GetWidget()->ReleaseMouseCapture();
+  }
+
   mouse_move_direction_ = kMovedMouseLeft | kMovedMouseRight;
 
   // Prevent the WebContents HWND from being hidden by any of the model
@@ -1214,14 +1229,12 @@ void TabDragController::DetachIntoNewBrowserAndRunMoveLoop(
 
   Browser* browser = CreateBrowserForDrag(
       attached_tabstrip_, screen_point, &drag_bounds);
-  attached_tabstrip_->ReleaseDragController();
   Detach();
   BrowserView* dragged_browser_view =
       BrowserView::GetBrowserViewForBrowser(browser);
   dragged_browser_view->GetWidget()->SetVisibilityChangedAnimationsEnabled(
       false);
   Attach(dragged_browser_view->tabstrip(), gfx::Point());
-  attached_tabstrip_->OwnDragController(this);
   // TODO: come up with a cleaner way to do this.
   attached_tabstrip_->SetTabBoundsForDrag(drag_bounds);
 
diff --git a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc b/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
index 1f8082f..a8db3a9 100644
--- a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
+++ b/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
@@ -621,3 +621,39 @@ IN_PROC_BROWSER_TEST_F(DetachToBrowserTabDragControllerTest,
   // browser() will have been destroyed, but browser2 should remain.
   ASSERT_EQ(1u, BrowserList::size());
 }
+
+// Creates two browsers, drags from first into the second in such a way that
+// no detaching should happen.
+IN_PROC_BROWSER_TEST_F(DetachToBrowserTabDragControllerTest,
+                       DragDirectlyToSecondWindow) {
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+
+  // Create another browser.
+  Browser* browser2 = CreateAnotherWindowBrowserAndRelayout();
+  TabStrip* tab_strip2 = GetTabStripForBrowser(browser2);
+
+  // Move to the first tab and drag it enough so that it detaches, but not
+  // enough that it attaches to browser2.
+  gfx::Point tab_0_center(GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
+  ASSERT_TRUE(ui_test_utils::SendMouseMoveSync(tab_0_center));
+  ASSERT_TRUE(ui_test_utils::SendMouseEventsSync(
+                  ui_controls::LEFT, ui_controls::DOWN));
+
+  gfx::Point b2_location(5, 0);
+  views::View::ConvertPointToScreen(tab_strip2, &b2_location);
+  ASSERT_TRUE(ui_test_utils::SendMouseMoveSync(b2_location));
+
+  // Should now be attached to tab_strip2.
+  ASSERT_TRUE(tab_strip2->IsDragSessionActive());
+  ASSERT_FALSE(tab_strip->IsDragSessionActive());
+  ASSERT_TRUE(TabDragController::IsActive());
+
+  // Release the mouse, stopping the drag session.
+  ASSERT_TRUE(ui_test_utils::SendMouseEventsSync(
+                  ui_controls::LEFT, ui_controls::UP));
+  ASSERT_FALSE(tab_strip2->IsDragSessionActive());
+  ASSERT_FALSE(tab_strip->IsDragSessionActive());
+  ASSERT_FALSE(TabDragController::IsActive());
+  EXPECT_EQ("0 100", IDString(browser2->tab_strip_model()));
+  EXPECT_EQ("1", IDString(browser()->tab_strip_model()));
+}