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);
     move_loop_widget_ = NULL;

[Peter Kasting, 2016/11/17 01:32:49]

Hmm.  This block of code looks similar to code in the destructor (which calls
RemoveObserver(), but only if |added_observer_to_move_loop_widget_| -- should we
be checking that elsewhere too?), as well as DragBrowserToNewTabStrip() (which
does all these things, but has some of them in an earlier #if ASH block --
should they indeed be ifdefed?).

This makes me wonder if these sorts of things should be pulled into some kind of
common "reset move loop widget" function that does the right magic voodoo.

[afakhry, 2016/11/17 22:51:05]

I'm also not very familiar with this code. So I agree we should involve +sky@ to
take a look at this.

I like your suggestion of having one function that does the magic. But I'm not
sure if it can be used everywhere instead of the current code that seemingly
does similar things (i.e. Not all of them do exactly the same thing. For
example, some depends on |added_observer_to_move_loop_widget_| some not. Some
set the WindowPositionManaged to true, some not. That #if ASH you mentioned). So
I think it would be better to defer to sky@ with this question.

   }
   is_dragging_window_ = false;