Avoids releasing capture prematurily when dragging tabs from one to another browser window

chrome/browser/ui/views/tabs/tab_drag_controller.cc

 // Copyright (c) 2012 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 "chrome/browser/ui/views/tabs/tab_drag_controller.h"
 
 #include <math.h>
 #include <set>
 
 #include "base/auto_reset.h"
@@ skipping 676 matching lines @@
     // work in all cases. Hence this hack.
     ui::GestureRecognizer::Get()->TransferEventsTo(
         browser_widget->GetNativeView(),
         target_tabstrip->GetWidget()->GetNativeView());
 #endif
 
     // The window is going away. Since the drag is still on going we don't want
     // that to effect the position of any windows.
     SetWindowPositionManaged(browser_widget->GetNativeView(), false);
 
+#if !defined(OS_LINUX)

[Elliot Glaysher, 2014/02/26 20:33:17]

Isn't this code used on chromeos, too?

[varkha, 2014/02/26 20:59:35]

It is, but I did not see any visual impact from skipping the call to Hide() on
Chrome OS. I suspect that the reason for having it here was not relevant on
Chrome OS.

     // EndMoveLoop is going to snap the window back to its original location.
-    // Hide it so users don't see this.
+    // Hide it so users don't see this. Hiding a window in Linux aura causes
+    // it to lose capture so skip it.
     browser_widget->Hide();

[varkha, 2014/02/26 20:30:18]

Not calling Hide on Linux seems to be lesser evil (not even sure if the concern
was valid on Linux).

+#endif
     browser_widget->EndMoveLoop();
 
     // Ideally we would always swap the tabs now, but on non-ash it seems that
     // running the move loop implicitly activates the window when done, leading
     // to all sorts of flicker. So, on non-ash, instead we process the move
     // after the loop completes. But on chromeos, we can do tab swapping now to
     // avoid the tab flashing issue(crbug.com/116329).
     if (host_desktop_type_ == chrome::HOST_DESKTOP_TYPE_ASH) {
       is_dragging_window_ = false;
       Detach(DONT_RELEASE_CAPTURE);
@@ skipping 494 matching lines @@
   // Under chromeos we immediately set the |move_loop_widget_| to NULL.
   if (move_loop_widget_) {
     move_loop_widget_->RemoveObserver(this);
     move_loop_widget_ = NULL;
   }
   is_dragging_window_ = false;
   waiting_for_run_loop_to_exit_ = false;
   if (end_run_loop_behavior_ == END_RUN_LOOP_CONTINUE_DRAGGING) {
     end_run_loop_behavior_ = END_RUN_LOOP_STOP_DRAGGING;
     if (tab_strip_to_attach_to_after_exit_) {
+      tab_strip_to_attach_to_after_exit_->GetWidget()->Activate();

[varkha, 2014/02/26 20:30:18]

Activating before calling Detach (which may close the browser frame) helps to
not lose capture prematurely.

[Elliot Glaysher, 2014/02/26 20:33:17]

What's the effect on Windows here?

[varkha, 2014/02/26 20:59:35]

I didn't have a chance to test this on Windows yet (TBD). I hope that this is
benign since I am moving it earlier.

+      // Activate may trigger a focus loss, destroying us.
+      if (!ref)
+        return;
       gfx::Point point_in_screen(GetCursorScreenPoint());
       Detach(DONT_RELEASE_CAPTURE);
       Attach(tab_strip_to_attach_to_after_exit_, point_in_screen);
       // Move the tabs into position.
       MoveAttached(point_in_screen);
-      attached_tabstrip_->GetWidget()->Activate();
-      // Activate may trigger a focus loss, destroying us.
-      if (!ref)
-        return;
       tab_strip_to_attach_to_after_exit_ = NULL;
     }
     DCHECK(attached_tabstrip_);
     attached_tabstrip_->GetWidget()->SetCapture(attached_tabstrip_);
   } else if (active_) {
     EndDrag(result == views::Widget::MOVE_LOOP_CANCELED ?
             END_DRAG_CANCEL : END_DRAG_COMPLETE);
   }
 }
 
@@ skipping 666 matching lines @@
     aura::Window* dragged_window =
         attached_tabstrip_->GetWidget()->GetNativeView();
     if (dragged_window)
       exclude.insert(dragged_window);
   }
   return GetLocalProcessWindowAtPoint(host_desktop_type_,
                                       screen_point,
                                       exclude);
 
 }