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>
 
@@ skipping 153 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);
+
+  // 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() {
   // 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())
-          .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));
   browser2->window()->SetBounds(gfx::Rect(
       work_area.x() + half_size.width(), work_area.y(),
       half_size.width(), half_size.height()));
   return browser2;
 }
 
 namespace {
@@ skipping 39 matching lines @@
   }
 
  private:
   aura::Window* root_window_;
 
   DISALLOW_COPY_AND_ASSIGN(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
 // 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 538 matching lines @@
 #define MAYBE_DetachFromFullsizeWindow DISABLED_DetachFromFullsizeWindow
 #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())
-          .work_area();
+      gfx::Screen::GetScreen()->GetPrimaryDisplay().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));
   ASSERT_TRUE(DragInputToNotifyWhenDone(
@@ skipping 338 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();
+  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.
 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 922 matching lines @@
 INSTANTIATE_TEST_CASE_P(TabDragging,
                         DetachToBrowserTabDragControllerTest,
                         ::testing::Values("mouse", "touch"));
 INSTANTIATE_TEST_CASE_P(TabDragging,
                         DetachToBrowserTabDragControllerTestTouch,
                         ::testing::Values("touch"));
 #else
 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