MacViews: Implement Tab Dragging

chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.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_interactive_uitest.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 
 #include "ash/wm/window_state.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/platform_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/native_browser_frame_factory.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
 #include "chrome/browser/ui/views/tabs/tab_drag_controller.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
@@ skipping 134 matching lines @@
 }
 
 void TabDragControllerTest::StopAnimating(TabStrip* tab_strip) {
   tab_strip->StopAnimating(true);
 }
 
 void TabDragControllerTest::AddTabAndResetBrowser(Browser* browser) {
   AddBlankTabAndShow(browser);
   StopAnimating(GetTabStripForBrowser(browser));
   ResetIDs(browser->tab_strip_model(), 0);
+#if defined(OS_MACOSX)
+  // Currently MacViews' browser windows are shown in the background anc could
+  // be obscured by other windows if there are any. This should be fixed in
+  // order to be consistent with other platforms.
+  EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser));

[tapted, 2016/04/13 08:28:12]

Let's move this to TabDragControllerTest::SetUpOnMainThread and ensure
DetachToBrowserTabDragControllerTest::SetUpOnMainThread calls its superclass's
TabDragControllerTest::SetUpOnMainThread()

nothing else should have to call BringBrowserWindowToFront then

[themblsha, 2016/04/18 09:30:00]

Done.

+#endif  // OS_MACOSX
 }
 
 Browser* TabDragControllerTest::CreateAnotherWindowBrowserAndRelayout() {
   // Create another browser.
   Browser* browser2 = CreateBrowser(browser()->profile());
   ResetIDs(browser2->tab_strip_model(), 100);
 
   // Resize the two windows so they're right next to each other.
   gfx::Rect work_area =
       gfx::Screen::GetScreen()
-          ->GetDisplayNearestWindow(browser()->window()->GetNativeWindow())
+          ->GetDisplayNearestWindow(platform_util::GetViewForWindow(
+              browser()->window()->GetNativeWindow()))
           .work_area();
   gfx::Size half_size =
       gfx::Size(work_area.width() / 3 - 10, work_area.height() / 2 - 10);
   browser()->window()->SetBounds(gfx::Rect(work_area.origin(), half_size));
   browser2->window()->SetBounds(gfx::Rect(
       work_area.x() + half_size.width(), work_area.y(),
       half_size.width(), half_size.height()));
   return browser2;
 }
 
@@ skipping 47 matching lines @@
   }
 
  private:
   aura::Window* root_window_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenEventGeneratorDelegate);
 };
 
 #endif
 
-#if !defined(OS_CHROMEOS)
+#if !defined(OS_CHROMEOS) && defined(USE_AURA)
 
 // Following classes verify a crash scenario. Specifically on Windows when focus
 // changes it can trigger capture being lost. This was causing a crash in tab
 // dragging as it wasn't set up to handle this scenario. These classes
 // synthesize this scenario.
 
 // Allows making ClearNativeFocus() invoke ReleaseCapture().
 class TestDesktopBrowserFrameAura : public DesktopBrowserFrameAura {
  public:
   TestDesktopBrowserFrameAura(
@@ skipping 592 matching lines @@
 #else
 #define MAYBE_DetachFromFullsizeWindow DetachFromFullsizeWindow
 #endif
 // Tests that a tab can be dragged from a browser window that is resized to full
 // screen.
 IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
                        MAYBE_DetachFromFullsizeWindow) {
   // Resize the browser window so that it is as big as the work area.
   gfx::Rect work_area =
       gfx::Screen::GetScreen()
-          ->GetDisplayNearestWindow(browser()->window()->GetNativeWindow())
+          ->GetDisplayNearestWindow(platform_util::GetViewForWindow(
+              browser()->window()->GetNativeWindow()))
           .work_area();
   browser()->window()->SetBounds(work_area);
   const gfx::Rect initial_bounds(browser()->window()->GetBounds());
   // Add another tab.
   AddTabAndResetBrowser(browser());
   TabStrip* tab_strip = GetTabStripForBrowser(browser());
 
   // Move to the first tab and drag it enough so that it detaches.
   gfx::Point tab_0_center(GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
   ASSERT_TRUE(PressInput(tab_0_center));
@@ skipping 25 matching lines @@
 
   EXPECT_FALSE(GetIsDragged(browser()));
   EXPECT_FALSE(GetIsDragged(new_browser));
   // After this both windows should still be manageable.
   EXPECT_TRUE(IsWindowPositionManaged(browser()->window()->GetNativeWindow()));
   EXPECT_TRUE(
       IsWindowPositionManaged(new_browser->window()->GetNativeWindow()));
 
   // Only second window should be maximized.
   EXPECT_FALSE(browser()->window()->IsMaximized());
   EXPECT_TRUE(new_browser->window()->IsMaximized());

[tapted, 2016/04/13 08:28:12]

this will fail on Mac (likewise a similar check on line 942)  - I haven't mapped
out exactly what "maximized" means on Mac - for other platforms it typically
changes the window frame, but that doesn't happen for Mac's "zoom" - only
fullscreen. We also can't just fill the screen with the browser window, since
that's not how "zoom" works.

I think this needs a separate crbug filed to track, and this line should be
#fidef'd out on Mac since, regardless of how it works, it doesn't seem right to
automatically maximize a window when there were no maximized windows to start
with - not on mac at least.

[themblsha, 2016/04/18 09:30:00]

Yeah, the two test that fail deal with maximization.

Maybe IsMaximized / Maximize functions need to be #ifdef'd on Mac as well, so
all the places would have to be explicitly audited?

Created http://crbug.com/603562, but can't edit its fields to match the rest of
MacViews issues.

 
   // The tab strip should no longer have capture because the drag was ended and
   // mouse/touch was released.
   EXPECT_FALSE(tab_strip->GetWidget()->HasCapture());
   EXPECT_FALSE(tab_strip2->GetWidget()->HasCapture());
 }
 
 #if defined(OS_CHROMEOS) || defined(OS_LINUX)
 // TODO(sky,sad): Disabled as it fails due to resize locks with a real
 // compositor. crbug.com/331924
 #define MAYBE_DetachToOwnWindowFromMaximizedWindow \
   DISABLED_DetachToOwnWindowFromMaximizedWindow
 #else
 #define MAYBE_DetachToOwnWindowFromMaximizedWindow \
   DetachToOwnWindowFromMaximizedWindow
 #endif
 // Drags from browser to a separate window and releases mouse.
 IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
                        MAYBE_DetachToOwnWindowFromMaximizedWindow) {
   // Maximize the initial browser window.
   browser()->window()->Maximize();
   ASSERT_TRUE(browser()->window()->IsMaximized());

[tapted, 2016/04/13 08:28:12]

fails - I think here this entire test can be can be disabled on mac with a
comment citing that bug

[themblsha, 2016/04/18 09:30:00]

Done.

 
   // Add another tab.
   AddTabAndResetBrowser(browser());
   TabStrip* tab_strip = GetTabStripForBrowser(browser());
 
   // Move to the first tab and drag it enough so that it detaches.
   gfx::Point tab_0_center(
       GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
   ASSERT_TRUE(PressInput(tab_0_center));
   ASSERT_TRUE(DragInputToNotifyWhenDone(
@@ skipping 271 matching lines @@
 #else
 #define MAYBE_DragAll DragAll
 #endif
 // Selects multiple tabs and starts dragging the window.
 IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest, MAYBE_DragAll) {
   // Add another tab.
   AddTabAndResetBrowser(browser());
   TabStrip* tab_strip = GetTabStripForBrowser(browser());
   browser()->tab_strip_model()->AddTabAtToSelection(0);
   browser()->tab_strip_model()->AddTabAtToSelection(1);
+  const gfx::Rect initial_bounds = browser()->window()->GetBounds();
 
   // Move to the first tab and drag it enough so that it would normally
   // detach.
   gfx::Point tab_0_center(GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
   ASSERT_TRUE(PressInput(tab_0_center));
   ASSERT_TRUE(DragInputToNotifyWhenDone(
       tab_0_center.x(), tab_0_center.y() + GetDetachY(tab_strip),
       base::Bind(&DragAllStep2, this, browser_list)));
   QuitWhenNotDragging();
 
   // Should not be dragging.
   ASSERT_FALSE(tab_strip->IsDragSessionActive());
   ASSERT_FALSE(TabDragController::IsActive());
 
   // And there should only be one window.
   EXPECT_EQ(1u, browser_list->size());
 
   EXPECT_EQ("0 1", IDString(browser()->tab_strip_model()));
 
   EXPECT_FALSE(GetIsDragged(browser()));
 
   // Remaining browser window should not be maximized
   EXPECT_FALSE(browser()->window()->IsMaximized());
+
+  const gfx::Rect final_bounds = browser()->window()->GetBounds();

[tapted, 2016/04/13 08:28:12]

Comment ~

// Size unchanged, but it should have moved down.

[themblsha, 2016/04/18 09:30:01]

Done.

+  EXPECT_EQ(initial_bounds.size(), final_bounds.size());
+  EXPECT_EQ(initial_bounds.origin().x(), final_bounds.origin().x());
+  EXPECT_EQ(initial_bounds.origin().y() + GetDetachY(tab_strip),
+            final_bounds.origin().y());
 }
 
+#if defined(OS_MACOSX)
+// Makes sure we can drag the window using WindowServer by dragging on a tab.
+//
+// All other tests move the windows without relying on the WindowServer, because
+// BridgedNativeWidget::RunMoveLoop() immediately offsets the window on first
+// mouse drag. If we generate more mouse move events after that, it's presumed
+// that the WindowServer will move the window. In order for that to work we need
+// to generate global CGEvents instead of app-specific NSEvents, that's what
+// ui_test_utils::DragAndDrop() is for.
+//

[tapted, 2016/04/13 08:28:12]

Perhaps a note here, since it will be confusing for those unfamiliar, to say

// Note that Mac doesn't simply rely on NSEvents, since they would not allow
// correct interactions with the window server. E.g., a window could not be
// dragged to another desktop Space.

[themblsha, 2016/04/18 09:30:01]

Done.

+// If the WindowServer fails to move the window, the final window bounds won't
+// match the expected ones.
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,

[tapted, 2016/04/13 08:28:12]

A comment specific to this test?

E.g. 

// Test dragging all the tabs in the window.

[themblsha, 2016/04/18 09:30:01]

Done.

+                       MacDragsWindowUsingCocoaMoveLoop) {
+  // Add another tab.
+  AddTabAndResetBrowser(browser());
+
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  browser()->tab_strip_model()->AddTabAtToSelection(0);
+  browser()->tab_strip_model()->AddTabAtToSelection(1);
+  const gfx::Rect initial_bounds = browser()->window()->GetBounds();
+
+  // Move to the first tab and drag it enough so that it would normally
+  // detach.
+  gfx::Point tab_0_center(GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
+
+  // If we don't make the interactive_ui_tests the active application, we won't
+  // be able to monitor NSMouseMoved events and ui_test_utils::DragAndDrop()
+  // will deadlock.
+  EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+  // We need to move the window using multiple intermediate events in order
+  // to verify that CocoaWindowMoveLoop is working correctly.
+  const int steps = 10;
+  ui_test_utils::DragAndDrop(
+      tab_0_center,
+      gfx::Point(tab_0_center.x(), tab_0_center.y() + GetDetachY(tab_strip)),
+      steps);
+
+  // Should not be dragging.
+  EXPECT_FALSE(tab_strip->IsDragSessionActive());
+  EXPECT_FALSE(TabDragController::IsActive());
+  EXPECT_FALSE(GetIsDragged(browser()));
+
+  // And there should only be one window.
+  EXPECT_EQ(1u, browser_list->size());
+  EXPECT_EQ("0 1", IDString(browser()->tab_strip_model()));
+
+  // Remaining browser window should not be maximized
+  EXPECT_FALSE(browser()->window()->IsMaximized());
+
+  const gfx::Rect final_bounds = browser()->window()->GetBounds();
+  EXPECT_EQ(initial_bounds.size(), final_bounds.size());
+  EXPECT_EQ(initial_bounds.origin().x(), final_bounds.origin().x());
+  EXPECT_EQ(initial_bounds.origin().y() + GetDetachY(tab_strip),
+            final_bounds.origin().y());
+}
+
+// Tests that when the first mouse event that starts RunMoveLoop does not
+// overlap the Mac menu bar. BridgedNativeWidget::RunMoveLoop() shifts both the
+// mouse position and the expected window frame after detachment.
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
+                       MacDetachesWindowOnTopOfMacMenuBar) {
+  // Add another tab.
+  AddTabAndResetBrowser(browser());
+
+  // Make sure there's enough space to trigger detachment.
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  browser()->window()->SetBounds(gfx::Rect(100, 100, 400, 200));
+  DCHECK_GT(browser()->window()->GetBounds().y(), GetDetachY(tab_strip));
+
+  // If we don't make the interactive_ui_tests the active application, we won't
+  // be able to monitor NSMouseMoved events and ui_test_utils::DragAndDrop()
+  // will deadlock.
+  EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+  gfx::Point tab_0_center(GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
+  gfx::Point on_top_of_menu_bar(tab_0_center.x(), 0);
+  ui_test_utils::DragAndDrop(tab_0_center, on_top_of_menu_bar);

[tapted, 2016/04/13 08:28:12]

using a sequence of DragAndDropOperation in place of this causes a failure for
me. E.g. I did


  browser()->window()->SetBounds(gfx::Rect(100, 200, 400, 200));
  DCHECK_GT(browser()->window()->GetBounds().y(), GetDetachY(tab_strip));

  gfx::Point tab_0_center(GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
  gfx::Point at_top_of_screen(tab_0_center.x(), 0);
  gfx::Point below_top_of_screen(tab_0_center.x(), 100);

  std::list<DragAndDropOperation> operations;
  operations.push_back(DragAndDropOperation::Move(tab_0_center));
  operations.push_back(DragAndDropOperation::MouseDown());
  operations.push_back(DragAndDropOperation::Move(at_top_of_screen));
  operations.push_back(DragAndDropOperation::Move(below_top_of_screen));
  operations.push_back(DragAndDropOperation::MouseUp());
  ui_test_utils::DragAndDrop(operations);

If at_top_of_screen is 50px instead of 0px from the top, the test works fine.
But with 0px the test fails - the window moves up, but doesn't initiate a drag.

[themblsha, 2016/04/18 09:30:01]

Added additional check to my code:

  EXPECT_GE(work_area.y(), 22)
      << "Without the MenuBar on top of work_area this test is pointless.";

Maybe somehow you don't have the menu bar on top of your screen?

[tapted, 2016/04/20 07:30:22]

On one of them :).

If you run two screens side-by-side, and don't use "Displays have separate
spaces" then one of the monitors will not have a menu bar at the top. And the
screen edge restricts movement in the same way as the menubar does.

But the point of my comment was that the test previously failed (regardless of
the menu bar presence). The checks were not enough to pick up the problem that
required fixes to move the cursor not just into the work area, but to ensure it
was in the window in the same place as the work area would have forced it to
move to.

So we should make sure we have a test case for this. In fact I wrote one already
at 
https://codereview.chromium.org/1877043003/diff2/20001:60001/chrome/browser/u...

[themblsha, 2016/04/20 13:38:40]

Curiously enough, your test only passes when I have two 27" monitors plugged in,
when only a single one is plugged it fails. But after replacing 2500 with 100 it
works fine even in single-monitor configuration.

And your test will fail if the BridgedNativeWidget::RunMoveLoop() mouse y
position shifting is removed even when the menu bar is not present, so it's
better than my version.

Replaced my test function with yours.

+
+  // Should not be dragging.
+  EXPECT_FALSE(tab_strip->IsDragSessionActive());
+  EXPECT_FALSE(TabDragController::IsActive());
+  EXPECT_FALSE(GetIsDragged(browser()));
+
+  // Second tab should successfully detach.
+  EXPECT_EQ(2u, browser_list->size());
+  EXPECT_EQ("1", IDString(browser()->tab_strip_model()));
+}
+
+// Detaches and reattaches second tab. Will fail if detached window is not moved
+// synchronously at the start of BridgedNativeWidget::RunMoveLoop(): the check
+// for expected WindowServerFrame() will fail.
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
+                       MacDetachesAndReattachesSecondTab) {
+  using namespace ui_test_utils;

[tapted, 2016/04/13 08:28:12]

nit: using ui_test_utils::DragAndDropOperation, and move it to above the
std::list declaration

[themblsha, 2016/04/18 09:30:01]

Done.

+
+  // Add another tab.
+  AddTabAndResetBrowser(browser());
+
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  browser()->window()->SetBounds(gfx::Rect(100, 100, 400, 200));
+  const gfx::Rect initial_bounds = browser()->window()->GetBounds();
+
+  gfx::Point tab_1_center(GetCenterInScreenCoordinates(tab_strip->tab_at(1)));
+  gfx::Point tab_1_shake(tab_1_center.x(),
+                         tab_1_center.y() + GetDetachY(tab_strip) * 2);
+
+  // If we don't make the interactive_ui_tests the active application, we won't
+  // be able to monitor NSMouseMoved events and ui_test_utils::DragAndDrop()
+  // will deadlock.

[tapted, 2016/04/13 08:28:12]

nit: this comment doesn't need to repeat

[themblsha, 2016/04/18 09:30:01]

Done.

+  EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+  std::list<DragAndDropOperation> operations;
+  operations.emplace_back(DragAndDropOperation::Move(tab_1_center));

[tapted, 2016/04/13 08:28:12]

The rule at Google is to use push_back unless that doesn't work.
DragAndDropOperation is copyable, so it should be push_back here (The downside
of emplace_back is that it invisibly calls explicit constructors which can lead
to unexpected stuff like

vector<vector<int>> baz;
baz.emplace_back(1 << 10);  // Inserts a vector of size 1024.

[themblsha, 2016/04/18 09:30:00]

Done. Style Guide is absolute :-)

+  operations.emplace_back(DragAndDropOperation::MouseDown());
+  operations.emplace_back(DragAndDropOperation::Move(tab_1_shake));
+  operations.emplace_back(DragAndDropOperation::Move(tab_1_center));
+  operations.emplace_back(DragAndDropOperation::MouseUp());
+
+  ui_test_utils::DragAndDrop(operations);
+
+  // Should not be dragging.
+  EXPECT_FALSE(tab_strip->IsDragSessionActive());
+  EXPECT_FALSE(TabDragController::IsActive());
+  EXPECT_FALSE(GetIsDragged(browser()));
+
+  // And there should only be one window.
+  EXPECT_EQ(1u, browser_list->size());
+  EXPECT_EQ("0 1", IDString(browser()->tab_strip_model()));
+
+  // The window should remain in its original place.
+  const gfx::Rect final_bounds = browser()->window()->GetBounds();
+  EXPECT_EQ(initial_bounds, final_bounds);
+}
+#endif  // OS_MACOSX
+
 namespace {
 
 // Invoked from the nested message loop.
 void DragAllToSeparateWindowStep2(DetachToBrowserTabDragControllerTest* test,
                                   TabStrip* attached_tab_strip,
                                   TabStrip* target_tab_strip,
                                   const BrowserList* browser_list) {
   ASSERT_TRUE(attached_tab_strip->IsDragSessionActive());
   ASSERT_FALSE(target_tab_strip->IsDragSessionActive());
   ASSERT_TRUE(TabDragController::IsActive());
@@ skipping 208 matching lines @@
                        MAYBE_DragSingleTabToSeparateWindow) {
   TabStrip* tab_strip = GetTabStripForBrowser(browser());
 
   ResetIDs(browser()->tab_strip_model(), 0);
 
   // Create another browser.
   Browser* browser2 = CreateAnotherWindowBrowserAndRelayout();
   TabStrip* tab_strip2 = GetTabStripForBrowser(browser2);
   const gfx::Rect initial_bounds(browser2->window()->GetBounds());
 
+  EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
   // Move to the first tab and drag it enough so that it detaches, but not
   // enough that it attaches to browser2.
   gfx::Point tab_0_center(
       GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
   ASSERT_TRUE(PressInput(tab_0_center));
   ASSERT_TRUE(DragInputToNotifyWhenDone(
       tab_0_center.x(), tab_0_center.y() + GetDetachY(tab_strip),
       base::Bind(&DragAllToSeparateWindowStep2, this, tab_strip, tab_strip2,
                  browser_list)));
   QuitWhenNotDragging();
@@ skipping 26 matching lines @@
 namespace {
 
 // Invoked from the nested message loop.
 void CancelOnNewTabWhenDraggingStep2(
     DetachToBrowserTabDragControllerTest* test,
     const BrowserList* browser_list) {
   ASSERT_TRUE(TabDragController::IsActive());
   ASSERT_EQ(2u, browser_list->size());
 
   // Add another tab. This should trigger exiting the nested loop. Add at the
   // to exercise past crash when model/tabstrip got out of sync (474082).

[tapted, 2016/04/13 08:28:12]

This comment should move as well, since it talks about the bug
(http://crbug.com/474082) which has "a new webcontents being created in the
middle of a drag, resulting in a canceled drag where the tabstrip view and its
model are out of sync." - i.e. it needs the LOAD_STOP stuff to exercise that
properly

[themblsha, 2016/04/18 09:30:00]

Done, thanks!

-  content::WindowedNotificationObserver observer(
-      content::NOTIFICATION_LOAD_STOP,
-      content::NotificationService::AllSources());
   chrome::AddTabAt(browser_list->GetLastActive(), GURL(url::kAboutBlankURL),
                    0, false);
-  observer.Wait();
 }
 
 }  // namespace
 
 #if defined(OS_CHROMEOS) || defined(OS_LINUX)
 // TODO(sky,sad): Disabled as it fails due to resize locks with a real
 // compositor. crbug.com/331924
 #define MAYBE_CancelOnNewTabWhenDragging DISABLED_CancelOnNewTabWhenDragging
 #else
 #define MAYBE_CancelOnNewTabWhenDragging CancelOnNewTabWhenDragging
 #endif
 // Adds another tab, detaches into separate window, adds another tab and
 // verifies the run loop ends.
 IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
                        MAYBE_CancelOnNewTabWhenDragging) {
   TabStrip* tab_strip = GetTabStripForBrowser(browser());
 
   // Add another tab to browser().
   AddTabAndResetBrowser(browser());
 
   // Move to the first tab and drag it enough so that it detaches.
   gfx::Point tab_0_center(
       GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
   ASSERT_TRUE(PressInput(tab_0_center));
+  content::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP,
+      content::NotificationService::AllSources());
   ASSERT_TRUE(DragInputToNotifyWhenDone(
       tab_0_center.x(), tab_0_center.y() + GetDetachY(tab_strip),
       base::Bind(&CancelOnNewTabWhenDraggingStep2, this, browser_list)));
-  QuitWhenNotDragging();
+  observer.Wait();
+
+  // On MacViews the Drag ends while waiting for NOTIFICATION_LOAD_STOP.

[tapted, 2016/04/13 08:28:12]

can you say more about this? (i.e. why it ends early on Mac)

[themblsha, 2016/04/18 09:30:00]

Maybe after moving the stuff around it's not Mac-specific anymore but I haven't
tested. Previously it issued an async command to drag, then immediately executed
QuitWhenNotDragging().

Now on Mac NOTIFICATION_LOAD_STOP executes after the drag'n'drop was cancelled.

Thinking about it now I think this should be removed, and testbots will show if
my assumption is wrong.

+  if (TabDragController::IsActive()) {

[tapted, 2016/04/13 08:28:12]

nit: no curlies

+    QuitWhenNotDragging();
+  }
 
   // Should be two windows and not dragging.
   ASSERT_FALSE(TabDragController::IsActive());
   ASSERT_EQ(2u, browser_list->size());
   for (auto* browser : *BrowserList::GetInstance()) {
     EXPECT_FALSE(GetIsDragged(browser));
     // Should not be maximized
     EXPECT_FALSE(browser->window()->IsMaximized());
   }
 }
@@ skipping 565 matching lines @@
                                            ui_controls::DOWN);
   }
 
   bool DragTabAndExecuteTaskWhenDone(const gfx::Point& position,
                                      const base::Closure& task) {
     return ui_controls::SendMouseMoveNotifyWhenDone(
         position.x(), position.y(), task);
   }
 
   void QuitWhenNotDragging() {
+    DCHECK(TabDragController::IsActive());
     test::QuitWhenNotDraggingImpl();
     base::MessageLoop::current()->Run();
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(
       DetachToBrowserInSeparateDisplayAndCancelTabDragControllerTest);
 };
 
 // Invoked from the nested message loop.
@@ skipping 257 matching lines @@
                         DetachToBrowserTabDragControllerTest,
                         ::testing::Values("mouse", "touch"));
 INSTANTIATE_TEST_CASE_P(TabDragging,
                         DetachToBrowserTabDragControllerTestTouch,
                         ::testing::Values("touch"));
 #else
 INSTANTIATE_TEST_CASE_P(TabDragging,
                         DetachToBrowserTabDragControllerTest,
                         ::testing::Values("mouse"));
 #endif