Reenable Tabdragging tests failing because of IsWindowPositionManaged()

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 1095 matching lines @@
           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;
   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;
   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());
       Detach(DONT_RELEASE_CAPTURE);
       Attach(tab_strip_to_attach_to_after_exit_, point_in_screen);
@@ skipping 701 matching lines @@
   // TODO(pkotwicz): Fix this properly (crbug.com/358482)
   for (auto* browser : *BrowserList::GetInstance()) {
     if (browser->tab_strip_model()->empty())
       exclude.insert(browser->window()->GetNativeWindow());
   }
 #endif
   base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
   *window = window_finder_->GetLocalProcessWindowAtPoint(screen_point, exclude);
   return ref ? Liveness::ALIVE : Liveness::DELETED;
 }