MacViews: Implement Tab Dragging

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

Index: chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
diff --git a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc b/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
index 85648dbc0840a3903ffe0e6e9941a4c15d96a6e0..94c4ac7a78c423b005402d9840e7bce574b3010a 100644
--- a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
+++ b/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
@@ -171,6 +171,11 @@ void TabDragControllerTest::AddTabAndResetBrowser(Browser* browser) {
   AddBlankTabAndShow(browser);
   StopAnimating(GetTabStripForBrowser(browser));
   ResetIDs(browser->tab_strip_model(), 0);
+
+  // On Macs if we try to drag on an inactive window it won't work. Two

[tapted, 2016/03/10 11:51:19]

Hm - this should work. Or at least, it works with the Cocoa browser. Is it just
in the test situation that it doesn't work? Maybe the tab is obscured by another
window - that would probably break the simulated events. But we should try to
explain why it doesn't work in the comment.

[themblsha, 2016/03/10 17:18:58]

Did some tests a couple more times: in order for dragging to work window needs
to be activated. In non-MacViews Chromium (as well as in Safari) we *can* start
drag on an inactive window.

I tried to investigate why windows in interactive_ui_tests can't be activated
using normal means: BrowserWindow::Activate() doesn't work, mouse clicking
doesn't work. NSBundle seems to be swizzled the same way as in non-MacViews
builds, NSApp's activationPolicy is the same. [NSApp
activateIgnoringOtherApps:YES] didn't seem to work.

Added -[BridgedContentView acceptsFirstMouse:], and now it works just fine. But
ui_test_utils::DragAndDrop() won't be able to monitor incoming MouseMove events,
so I've moved explicit browser window activation to that test and added a
comment.

Also acceptsFirstMouse: == YES is enabled only for handful of objects in Cocoa
browser, and there seems to be nothing at the moment that would enable us to
enable acceptsFirstMouse: only for Views that require it. Probably this should
be handled in another issue?

[tapted, 2016/03/11 09:38:28]

Yeah I think acceptsFirstMouse returning YES is good -- playing around in the
browser most things seem to respond immediately.

+  // solutions are either to activate the window explicitly, or to click on a
+  // window once before starting dragging.
+  EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser));
 }
 
 Browser* TabDragControllerTest::CreateAnotherWindowBrowserAndRelayout() {
@@ -180,9 +185,7 @@ Browser* TabDragControllerTest::CreateAnotherWindowBrowserAndRelayout() {
 
   // Resize the two windows so they're right next to each other.
   gfx::Rect work_area =
-      gfx::Screen::GetScreen()
-          ->GetDisplayNearestWindow(browser()->window()->GetNativeWindow())
-          .work_area();
+      gfx::Screen::GetScreen()->GetPrimaryDisplay().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));
@@ -242,7 +245,7 @@ class ScreenEventGeneratorDelegate
 
 #endif
 
-#if !defined(OS_CHROMEOS)
+#if !defined(OS_CHROMEOS) && !defined(OS_MACOSX)

[tapted, 2016/03/10 11:51:19]

I'd go with && defined(USE_AURA) instead - since the code in-between relies a
lot on Aura APIs

[themblsha, 2016/03/10 17:18:58]

Done.

 
 // 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
@@ -801,9 +804,7 @@ 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())
-          .work_area();
+      gfx::Screen::GetScreen()->GetPrimaryDisplay().work_area();
   browser()->window()->SetBounds(work_area);
   const gfx::Rect initial_bounds(browser()->window()->GetBounds());
   // Add another tab.
@@ -1162,6 +1163,7 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest, MAYBE_DragAll) {
   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.
@@ -1185,8 +1187,57 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest, MAYBE_DragAll) {
 
   // 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());
 }
 
+#if defined(OS_MACOSX)
+// Makes sure we can drag the window using WindowServer by dragging on a tab

[tapted, 2016/03/10 11:51:19]

nit: full-stop on the end. Also say why this is only relevant for Mac (e.g. will
it fail, or is it that ui_test_utils::DragAndDrop is special)

[themblsha, 2016/03/10 17:18:58]

Done.

+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
+                       DragsWindowUsingCocoaMoveLoop) {
+  // 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)));
+  const unsigned int steps = 10;  // use anything greater than 1 to verify that

[tapted, 2016/03/10 11:51:19]

we tend not to use `unsigned` on Chrome if int works - I think there's something
in the style guide about it. So perhaps `cont int kSteps = 10;`

[themblsha, 2016/03/10 17:18:58]

Done.

+                                  // CocoaWindowMoveLoop is working correctly

[tapted, 2016/03/10 11:51:18]

nit: Comment above the declaration. Also sentence-style.

[themblsha, 2016/03/10 17:18:58]

Done.

+  ui_test_utils::DragAndDrop(
+      tab_0_center,
+      gfx::Point(tab_0_center.x(), tab_0_center.y() + GetDetachY(tab_strip)),
+      base::TimeDelta(), steps);
+
+  // Should not be dragging.
+  ASSERT_FALSE(tab_strip->IsDragSessionActive());

[tapted, 2016/03/10 11:51:19]

nit: typically we just use EXPECT_ unless things really explode, since it can
allow a tryjob to capture more debugging output upon a failure.

[themblsha, 2016/03/10 17:18:58]

Done. But I used the DragAll test as a base, and it uses ASSERTs.

+  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();
+  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());
+}
+#endif  // OS_MACOSX
+
 namespace {
 
 // Invoked from the nested message loop.
@@ -1415,6 +1466,8 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
   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(
@@ -2357,4 +2410,8 @@ INSTANTIATE_TEST_CASE_P(TabDragging,
 INSTANTIATE_TEST_CASE_P(TabDragging,
                         DetachToBrowserTabDragControllerTest,
                         ::testing::Values("mouse"));
+#elif defined(OS_MACOSX)

[tapted, 2016/03/10 11:51:19]

this shouldn't be needed any more -- we can just use the `else` (also I'm amazed
that compilers don't complain about #elif coming after an #else :)

[themblsha, 2016/03/10 17:18:58]

Whoops, clang indeed complains about that, but at the time of 'git cl upload' it
was still recompiling on top of recent Chromium.

+INSTANTIATE_TEST_CASE_P(TabDragging,
+                        DetachToBrowserTabDragControllerTest,
+                        ::testing::Values("mouse"));
 #endif