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);
     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;
   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);
       // Move the tabs into position.
@@ skipping 700 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;
 }