Do not release capture when starting a move loop on Desktop Linux

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 "ash/accelerators/accelerator_commands.h"
@@ skipping 149 matching lines @@
 
 // static
 const int TabDragController::kVerticalDetachMagnetism = 15;
 
 TabDragController::TabDragController()
     : event_source_(EVENT_SOURCE_MOUSE),
       source_tabstrip_(NULL),
       attached_tabstrip_(NULL),
       screen_(NULL),
       host_desktop_type_(chrome::HOST_DESKTOP_TYPE_NATIVE),
-      use_aura_capture_policy_(false),
+      can_release_capture_(true),
       offset_to_width_ratio_(0),
       old_focused_view_id_(
           views::ViewStorage::GetInstance()->CreateStorageID()),
       last_move_screen_loc_(0),
       started_drag_(false),
       active_(true),
       source_tab_index_(std::numeric_limits<size_t>::max()),
       initial_move_(true),
       move_behavior_(REORDER),
       mouse_move_direction_(0),
@@ skipping 46 matching lines @@
   DCHECK(!tabs.empty());
   DCHECK(std::find(tabs.begin(), tabs.end(), source_tab) != tabs.end());
   source_tabstrip_ = source_tabstrip;
   was_source_maximized_ = source_tabstrip->GetWidget()->IsMaximized();
   was_source_fullscreen_ = source_tabstrip->GetWidget()->IsFullscreen();
   screen_ = gfx::Screen::GetScreenFor(
       source_tabstrip->GetWidget()->GetNativeView());
   host_desktop_type_ = chrome::GetHostDesktopTypeForNativeView(
       source_tabstrip->GetWidget()->GetNativeView());
 #if defined(OS_LINUX)
-  use_aura_capture_policy_ = true;
+  // Mouse capture is not synchronous on desktop Linux. Chrome makes
+  // transferring capture between widgets without releasing capture appear
+  // synchronous on desktop Linux, so use that.
+  can_release_capture_ = false;

[sky, 2014/08/13 19:14:53]

What about when running ash on linux (not chromeos)?

[pkotwicz, 2014/08/15 20:30:49]

I moved the comments outside of the ifdefs. Should be clearer now.

 #else
-  use_aura_capture_policy_ =
-      (host_desktop_type_ == chrome::HOST_DESKTOP_TYPE_ASH);
+  // Do not release capture when transferring capture on Ash because releasing
+  // capture cancels gestures.
+  can_release_capture_ =
+      (host_desktop_type_ != chrome::HOST_DESKTOP_TYPE_ASH);
 #endif
   start_point_in_screen_ = gfx::Point(source_tab_offset, mouse_offset.y());
   views::View::ConvertPointToScreen(source_tab, &start_point_in_screen_);
   event_source_ = event_source;
   mouse_offset_ = mouse_offset;
   move_behavior_ = move_behavior;
   last_point_in_screen_ = start_point_in_screen_;
   last_move_screen_loc_ = start_point_in_screen_.x();
   initial_tab_positions_ = source_tabstrip->GetTabXCoordinates();
 
@@ skipping 290 matching lines @@
                "point_in_screen", point_in_screen.ToString());
 
   if (!target_tabstrip) {
     DetachIntoNewBrowserAndRunMoveLoop(point_in_screen);
     return DRAG_BROWSER_RESULT_STOP;
   }
   if (is_dragging_window_) {
     // ReleaseCapture() is going to result in calling back to us (because it
     // results in a move). That'll cause all sorts of problems.  Reset the
     // observer so we don't get notified and process the event.
-    if (use_aura_capture_policy_) {
+    if (host_desktop_type_ == chrome::HOST_DESKTOP_TYPE_ASH) {

[sky, 2014/08/13 19:14:53]

Why isn't this can_release_capture_?

[pkotwicz, 2014/08/15 20:30:49]

The behavior described in the comment applies to ash only. Desktop Linux does
not end the move loop upon losing capture. (With or without this CL)

       move_loop_widget_->RemoveObserver(this);
       move_loop_widget_ = NULL;
     }
     views::Widget* browser_widget = GetAttachedBrowserWidget();
     // Need to release the drag controller before starting the move loop as it's
     // going to trigger capture lost, which cancels drag.
     attached_tabstrip_->ReleaseDragController();
     target_tabstrip->OwnDragController(this);
     // Disable animations so that we don't see a close animation on aero.
     browser_widget->SetVisibilityChangedAnimationsEnabled(false);
-    // For aura we can't release capture, otherwise it'll cancel a gesture.
-    // Instead we have to directly change capture.
-    if (use_aura_capture_policy_)
+    if (can_release_capture_)
+      browser_widget->ReleaseCapture();
+    else
       target_tabstrip->GetWidget()->SetCapture(attached_tabstrip_);
-    else
-      browser_widget->ReleaseCapture();
 #if defined(OS_WIN)
     // The Gesture recognizer does not work well currently when capture changes
     // while a touch gesture is in progress. So we need to manually transfer
     // gesture sequence and the GR's touch events queue to the new window. This
     // should really be done somewhere in capture change code and or inside the
     // GR. But we currently do not have a consistent way for doing it that would
     // 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) || defined(OS_CHROMEOS)
     // EndMoveLoop is going to snap the window back to its original location.
     // 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();
 #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 (use_aura_capture_policy_) {
+    // Ideally we would always swap the tabs now, but on non-ash Windows, it

[sky, 2014/08/13 19:14:53]

This is confusing now as it doesn't mention can_release_capture.

[pkotwicz, 2014/08/15 20:30:49]

I changed the comment to make it more accurate. I am unsure whether the part of
the comment wrt to ChromeOS is still true.
On CrOS, line 609 does not activate the widget. http://crbug.com/402272

I can leave the comment as it was if you prefer. I can keep the variable named
|use_aura_capture_policy_| if you prefer too.

+    // seems that running the move loop implicitly activates the window when
+    // done, leading to all sorts of flicker. So, on non-ash Windows, 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 (can_release_capture_) {
+      tab_strip_to_attach_to_after_exit_ = target_tabstrip;
+    } else {
       is_dragging_window_ = false;
       Detach(DONT_RELEASE_CAPTURE);
       Attach(target_tabstrip, point_in_screen);
       // Move the tabs into position.
       MoveAttached(point_in_screen);
       attached_tabstrip_->GetWidget()->Activate();
-    } else {
-      tab_strip_to_attach_to_after_exit_ = target_tabstrip;
     }
 
     waiting_for_run_loop_to_exit_ = true;
     end_run_loop_behavior_ = END_RUN_LOOP_CONTINUE_DRAGGING;
     return DRAG_BROWSER_RESULT_STOP;
   }
   Detach(DONT_RELEASE_CAPTURE);
   Attach(target_tabstrip, point_in_screen);
   return DRAG_BROWSER_RESULT_CONTINUE;
 }
@@ skipping 368 matching lines @@
   std::vector<gfx::Rect> drag_bounds = CalculateBoundsForDraggedTabs();
   OffsetX(GetAttachedDragPoint(point_in_screen).x(), &drag_bounds);
 
   gfx::Vector2d drag_offset;
   Browser* browser = CreateBrowserForDrag(
       attached_tabstrip_, point_in_screen, &drag_offset, &drag_bounds);
 #if defined(OS_WIN)
   gfx::NativeView attached_native_view =
     attached_tabstrip_->GetWidget()->GetNativeView();
 #endif
-  Detach(use_aura_capture_policy_ ? DONT_RELEASE_CAPTURE : RELEASE_CAPTURE);
+  Detach(can_release_capture_ ? RELEASE_CAPTURE : DONT_RELEASE_CAPTURE);
   BrowserView* dragged_browser_view =
       BrowserView::GetBrowserViewForBrowser(browser);
   views::Widget* dragged_widget = dragged_browser_view->GetWidget();
 #if defined(OS_WIN)
     // The Gesture recognizer does not work well currently when capture changes
     // while a touch gesture is in progress. So we need to manually transfer
     // gesture sequence and the GR's touch events queue to the new window. This
     // should really be done somewhere in capture change code and or inside the
     // GR. But we currently do not have a consistent way for doing it that would
     // work in all cases. Hence this hack.
@@ skipping 24 matching lines @@
 void TabDragController::RunMoveLoop(const gfx::Vector2d& drag_offset) {
   // If the user drags the whole window we'll assume they are going to attach to
   // another window and therefore want to reorder.
   move_behavior_ = REORDER;
 
   move_loop_widget_ = GetAttachedBrowserWidget();
   DCHECK(move_loop_widget_);
   move_loop_widget_->AddObserver(this);
   is_dragging_window_ = true;
   base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
-  // Running the move loop releases mouse capture on non-ash, which triggers
-  // destroying the drag loop. Release mouse capture ourself before this while
-  // the DragController isn't owned by the TabStrip.
-  if (host_desktop_type_ != chrome::HOST_DESKTOP_TYPE_ASH) {
+  if (can_release_capture_) {
+    // Running the move loop releases mouse capture, which triggers destroying
+    // the drag loop. Release mouse capture now while the DragController is not
+    // owned by the TabStrip.
     attached_tabstrip_->ReleaseDragController();
     attached_tabstrip_->GetWidget()->ReleaseCapture();
     attached_tabstrip_->OwnDragController(this);
   }
   const views::Widget::MoveLoopSource move_loop_source =
       event_source_ == EVENT_SOURCE_MOUSE ?
       views::Widget::MOVE_LOOP_SOURCE_MOUSE :
       views::Widget::MOVE_LOOP_SOURCE_TOUCH;
   const views::Widget::MoveLoopEscapeBehavior escape_behavior =
       is_dragging_new_browser_ ?
           views::Widget::MOVE_LOOP_ESCAPE_BEHAVIOR_HIDE :
           views::Widget::MOVE_LOOP_ESCAPE_BEHAVIOR_DONT_HIDE;
   views::Widget::MoveLoopResult result =
       move_loop_widget_->RunMoveLoop(
           drag_offset, move_loop_source, escape_behavior);
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_TAB_DRAG_LOOP_DONE,
       content::NotificationService::AllBrowserContextsAndSources(),
       content::NotificationService::NoDetails());
 
   if (!ref)
     return;
-  // 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_) {
       gfx::Point point_in_screen(GetCursorScreenPoint());
@@ skipping 678 matching lines @@
        it != browser_list->end(); ++it) {
     if ((*it)->tab_strip_model()->empty())
       exclude.insert((*it)->window()->GetNativeWindow());
   }
 #endif
   return GetLocalProcessWindowAtPoint(host_desktop_type_,
                                       screen_point,
                                       exclude);
 
 }