Reenable Tabdragging tests failing because of IsWindowPositionManaged()

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

Index: chrome/browser/ui/views/tabs/tab_drag_controller.cc
diff --git a/chrome/browser/ui/views/tabs/tab_drag_controller.cc b/chrome/browser/ui/views/tabs/tab_drag_controller.cc
index 52d3b4baa578791dc7ee93fc4c5d08c680d3518c..fd06efa29053dd38b96345720d3fe5da5f151fa1 100644
--- a/chrome/browser/ui/views/tabs/tab_drag_controller.cc
+++ b/chrome/browser/ui/views/tabs/tab_drag_controller.cc
@@ -1113,6 +1113,8 @@ void TabDragController::RunMoveLoop(const gfx::Vector2d& drag_offset) {
     return;
   if (move_loop_widget_) {
     move_loop_widget_->RemoveObserver(this);
+    if (result != views::Widget::MOVE_LOOP_CANCELED)
+      SetWindowPositionManaged(move_loop_widget_->GetNativeWindow(), true);

[sky, 2016/11/19 14:55:03]

Can you clarify why tab dragging code needs to explicitly call
SetWindowPositionManaged (anywhere in ths file)? It seems like this code should
be entirely isolated in ash. Ash knows the drag is happening by way of
RunMoveLoop() and if ash needs to disable anything at this point ash should key
of RunMoveLoop.

[afakhry, 2016/11/22 18:15:05]

I was hoping that you would answer this question as I'm not familiar with this
code. I don't know why SetWindowPositionManaged() is called from several places
in this file. As you said, SetWindowPositionManaged() seems ASH specific ...
it's a no-op for non-ASH:
(https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/tab_drag_co...).
The question is, can we unify things? There are many places where we remove the
observer from |move_loop_widget_| and clear it, but they're not all exactly the
same. Sometimes we do that only when |added_observer_to_move_loop_widget_| is
true, sometimes we call SetWindowPositionManaged(), and sometimes only #if
USE_ASH. See:
https://cs.chromium.org/search/?q=f:+tab_drag_controller.cc+move_loop_widget_...

     move_loop_widget_ = NULL;
   }
   is_dragging_window_ = false;