Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7750)

Unified Diff: chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc

Issue 1747803003: MacViews: Implement Tab Dragging (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Removed click simulation, reimplemented CocoaWindowMoveLoop without relying on the WindowServer. Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 bec924f538d65571b25309b3b421de2a538ab6e6..ff9df65b7e75b25bb01f66c3dd3eeeb3b2887fc9 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
@@ -17,6 +17,7 @@
#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"
@@ -185,7 +186,8 @@ Browser* TabDragControllerTest::CreateAnotherWindowBrowserAndRelayout() {
// Resize the two windows so they're right next to each other.
gfx::Rect work_area =
display::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);
@@ -253,7 +255,7 @@ class 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
@@ -376,6 +378,12 @@ class DetachToBrowserTabDragControllerTest
event_generator_.reset(
new ui::test::EventGenerator(ash::Shell::GetPrimaryRootWindow()));
#endif
+#if defined(OS_MACOSX)
+ // Currently MacViews' browser windows are shown in the background and 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()));
+#endif // OS_MACOSX
}
InputSource input_source() const {
@@ -853,9 +861,11 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
EXPECT_FALSE(tab_strip2->GetWidget()->HasCapture());
}
-#if defined(OS_CHROMEOS) || defined(OS_LINUX)
+#if defined(OS_CHROMEOS) || defined(OS_LINUX) || defined(OS_MACOSX)
// TODO(sky,sad): Disabled as it fails due to resize locks with a real
// compositor. crbug.com/331924
+// TODO(tapted,mblsha): Disabled as the IsMaximized behavior is not consistent
tapted 2016/05/23 07:29:27 nit: add "Mac, parens" in ".. as the Mac IsMaximiz
themblsha 2016/05/26 15:13:24 Done.
+// with other platforms. crbug.com/603562
#define MAYBE_DetachFromFullsizeWindow DISABLED_DetachFromFullsizeWindow
#else
#define MAYBE_DetachFromFullsizeWindow DetachFromFullsizeWindow
@@ -867,7 +877,8 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
// Resize the browser window so that it is as big as the work area.
gfx::Rect work_area =
display::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());
@@ -921,9 +932,11 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
EXPECT_FALSE(tab_strip2->GetWidget()->HasCapture());
}
-#if defined(OS_CHROMEOS) || defined(OS_LINUX)
+#if defined(OS_CHROMEOS) || defined(OS_LINUX) || defined(OS_MACOSX)
// TODO(sky,sad): Disabled as it fails due to resize locks with a real
// compositor. crbug.com/331924
+// TODO(tapted,mblsha): Disabled as the IsMaximized behavior is not consistent
tapted 2016/05/23 07:29:27 ".. as the Mac IsMaximized() behavior ..".
themblsha 2016/05/26 15:13:24 Done.
+// with other platforms. crbug.com/603562
#define MAYBE_DetachToOwnWindowFromMaximizedWindow \
DISABLED_DetachToOwnWindowFromMaximizedWindow
#else
@@ -1283,6 +1296,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.
@@ -1306,7 +1320,161 @@ 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();
+ // Size unchanged, but it should have moved down.
+ 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.
+//
+// If the WindowServer fails to move the window, the final window bounds won't
+// match the expected ones.
+//
+// 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.
+
+// Test dragging all the tabs in the window, this should move the entire window.
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
+ 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,
+ MacDetachesWindowAtTopOfScreen) {
+ // 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, 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);
+
+ using ui_test_utils::DragAndDropOperation;
+ 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);
+
+ // 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()));
+
+ Browser* browser2 = browser_list->get(1);
+ EXPECT_EQ(browser2->window()->GetBounds().size(),
+ browser()->window()->GetBounds().size());
+}
+
+// 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) {
+ // 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);
+
+ EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+ using ui_test_utils::DragAndDropOperation;
+ std::list<DragAndDropOperation> operations;
+ operations.push_back(DragAndDropOperation::Move(tab_1_center));
+ operations.push_back(DragAndDropOperation::MouseDown());
+ operations.push_back(DragAndDropOperation::Move(tab_1_shake));
+ operations.push_back(DragAndDropOperation::Move(tab_1_center));
+ operations.push_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 {
@@ -1536,6 +1704,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(
@@ -1581,14 +1751,8 @@ void CancelOnNewTabWhenDraggingStep2(
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).
- content::WindowedNotificationObserver observer(
- content::NOTIFICATION_LOAD_STOP,
- content::NotificationService::AllSources());
chrome::AddTabAt(browser_list->GetLastActive(), GURL(url::kAboutBlankURL),
0, false);
- observer.Wait();
}
} // namespace
@@ -1613,12 +1777,19 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
gfx::Point tab_0_center(
GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
ASSERT_TRUE(PressInput(tab_0_center));
+
+ // Add another tab. This should trigger exiting the nested loop. Add at the
tapted 2016/05/23 07:29:27 I realise this is just moved, but there's a word m
themblsha 2016/05/26 15:13:24 Done.
+ // to exercise past crash when model/tabstrip got out of sync (474082).
tapted 2016/05/23 07:29:27 nit: update bug reference: crbug.com/474082
themblsha 2016/05/26 15:13:24 Done.
+ 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();
// Should be two windows and not dragging.
+ ASSERT_FALSE(tab_strip->IsDragSessionActive());
ASSERT_FALSE(TabDragController::IsActive());
ASSERT_EQ(2u, browser_list->size());
for (auto* browser : *BrowserList::GetInstance()) {
@@ -2202,6 +2373,7 @@ class DetachToBrowserInSeparateDisplayAndCancelTabDragControllerTest
}
void QuitWhenNotDragging() {
+ DCHECK(TabDragController::IsActive());
test::QuitWhenNotDraggingImpl();
base::MessageLoop::current()->Run();
}

Powered by Google App Engine
This is Rietveld 408576698