Commit ddac4c3c authored by Xiaoqian Dai's avatar Xiaoqian Dai Committed by Commit Bot

overview gesture: Avoid accidental window snap in splitscreen.

Modified based on the new spec:
A window has to be dragged toward the direction of the edge of the
screen for a minimum of 96dp to a point within 48dp of the edge of
the screen, or dragged inside 8dp from edge to be snapped.

Bug: 1021294
Change-Id: I2d79854b37bfdf36bcd5319a856b874233159982
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037825Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738444}
parent 041271d5
...@@ -473,7 +473,8 @@ DragWindowFromShelfController::GetSnapPosition( ...@@ -473,7 +473,8 @@ DragWindowFromShelfController::GetSnapPosition(
aura::Window* root_window = Shell::GetPrimaryRootWindow(); aura::Window* root_window = Shell::GetPrimaryRootWindow();
SplitViewController::SnapPosition snap_position = ::ash::GetSnapPosition( SplitViewController::SnapPosition snap_position = ::ash::GetSnapPosition(
root_window, window_, gfx::ToRoundedPoint(location_in_screen)); root_window, window_, gfx::ToRoundedPoint(location_in_screen),
kScreenEdgeInsetForSnap, kScreenEdgeInsetForSnap);
// For portrait mode, since the drag starts from the bottom of the screen, // For portrait mode, since the drag starts from the bottom of the screen,
// we should only allow the window to snap to the top of the screen. // we should only allow the window to snap to the top of the screen.
...@@ -485,40 +486,37 @@ DragWindowFromShelfController::GetSnapPosition( ...@@ -485,40 +486,37 @@ DragWindowFromShelfController::GetSnapPosition(
snap_position = SplitViewController::NONE; snap_position = SplitViewController::NONE;
} }
// If the drag does not start in the screen edge, the window has to be dragged // A window has to be dragged toward the direction of the edge of the screen
// toward the snap position by |min_drag_distance| to allow to be snapped. // for a minimum of |kMinDragDistance| to a point within
// |kScreenEdgeInsetForSnap| of the edge of the screen, or dragged inside
// |kDistanceFromEdge| from edge.
if (snap_position != SplitViewController::NONE) { if (snap_position != SplitViewController::NONE) {
// Check if the drag starts within 16dp from screen edge. Only need to const gfx::Rect area =
// consider about landscape case as in portrait mode, the drag always starts screen_util::GetDisplayWorkAreaBoundsInScreenForActiveDeskContainer(
// from bottom and we don't allow to snap in bottom. root_window);
bool started_in_screen_edge = false;
const int initial_x = initial_location_in_screen_.x(); // Check if the drag ends inside |kDistanceFromEdge| from edge. If so, the
const int initial_y = initial_location_in_screen_.y(); // window should be get snapped no matter how far it's been dragged.
if (is_landscape) { bool drag_end_near_edge = false;
const gfx::Rect work_area = if ((is_landscape &&
screen_util::GetDisplayWorkAreaBoundsInScreenForActiveDeskContainer( (location_in_screen.x() < kDistanceFromEdge ||
root_window); location_in_screen.x() > area.right() - kDistanceFromEdge)) ||
started_in_screen_edge = (!is_landscape &&
initial_x <= work_area.x() + kDistanceFromEdge || (location_in_screen.y() < kDistanceFromEdge ||
initial_x > work_area.right() - kDistanceFromEdge; location_in_screen.y() > area.bottom() - kDistanceFromEdge))) {
drag_end_near_edge = true;
} }
if (!started_in_screen_edge) { if (!drag_end_near_edge) {
// Check if the drag starts in the snap region. // Check how far the window has been dragged.
const bool started_in_snap_region = const int initial_x = initial_location_in_screen_.x();
::ash::GetSnapPosition( const int initial_y = initial_location_in_screen_.y();
root_window, window_,
gfx::ToRoundedPoint(initial_location_in_screen_)) !=
SplitViewController::NONE;
const int distance = is_landscape ? location_in_screen.x() - initial_x const int distance = is_landscape ? location_in_screen.x() - initial_x
: location_in_screen.y() - initial_y; : location_in_screen.y() - initial_y;
const int min_drag_distance = started_in_snap_region
? kMinDragDistanceInSnapRegion
: kMinDragDistanceOutsideSnapRegion;
if ((SplitViewController::IsPhysicalLeftOrTop(snap_position) && if ((SplitViewController::IsPhysicalLeftOrTop(snap_position) &&
distance > -min_drag_distance) || distance > -kMinDragDistance) ||
(!SplitViewController::IsPhysicalLeftOrTop(snap_position) && (!SplitViewController::IsPhysicalLeftOrTop(snap_position) &&
distance < min_drag_distance)) { distance < kMinDragDistance)) {
snap_position = SplitViewController::NONE; snap_position = SplitViewController::NONE;
} }
} }
......
...@@ -52,15 +52,14 @@ class ASH_EXPORT DragWindowFromShelfController : public aura::WindowObserver { ...@@ -52,15 +52,14 @@ class ASH_EXPORT DragWindowFromShelfController : public aura::WindowObserver {
// If the window drag starts within |kDistanceFromEdge| from screen edge, it // If the window drag starts within |kDistanceFromEdge| from screen edge, it
// will get snapped if the drag ends in the snap region, no matter how far // will get snapped if the drag ends in the snap region, no matter how far
// the window has been dragged. // the window has been dragged.
static constexpr int kDistanceFromEdge = 16; static constexpr int kDistanceFromEdge = 8;
// If the window drag starts in a snap region, it needs to be dragged
// |kMinDragDistanceInSnapRegion| toward the snap direction for the window to // A window has to be dragged toward the direction of the edge of the screen
// be snapped. // for a minimum of |kMinDragDistance| to a point within
static constexpr int kMinDragDistanceInSnapRegion = 48; // |kScreenEdgeInsetForSnap| of the edge of the screen, or dragged inside
// If the window drag starts outside a snap region, it needs to be dragged // |kDistanceFromEdge| from edge to be snapped.
// |kMinDragDistanceOutsideSnapRegion| toward the snap direction for the static constexpr int kScreenEdgeInsetForSnap = 48;
// window to be snapped. static constexpr int kMinDragDistance = 96;
static constexpr int kMinDragDistanceOutsideSnapRegion = 96;
// The distance for the dragged window to pass over the bottom of the display // The distance for the dragged window to pass over the bottom of the display
// so that it can be dragged into home launcher or overview. If not pass this // so that it can be dragged into home launcher or overview. If not pass this
......
...@@ -677,9 +677,8 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) { ...@@ -677,9 +677,8 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) {
const gfx::Rect display_bounds = display::Screen::GetScreen() const gfx::Rect display_bounds = display::Screen::GetScreen()
->GetDisplayNearestWindow(window1.get()) ->GetDisplayNearestWindow(window1.get())
.bounds(); .bounds();
int snap_edge_inset = const int snap_edge_inset =
display_bounds.width() * kHighlightScreenPrimaryAxisRatio + DragWindowFromShelfController::kScreenEdgeInsetForSnap;
kHighlightScreenEdgePaddingDp;
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
histogram_tester.ExpectBucketCount(kHandleDragWindowFromShelfHistogramName, histogram_tester.ExpectBucketCount(kHandleDragWindowFromShelfHistogramName,
...@@ -699,9 +698,7 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) { ...@@ -699,9 +698,7 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) {
window_drag_controller()); window_drag_controller());
// Drag into the snap region and release. // Drag into the snap region and release.
gfx::Point end = gfx::Point( gfx::Point end = gfx::Point(
start.x() - start.x() - DragWindowFromShelfController::kMinDragDistance + 10, 200);
DragWindowFromShelfController::kMinDragDistanceOutsideSnapRegion + 10,
200);
EndDrag(end, base::nullopt); EndDrag(end, base::nullopt);
OverviewController* overview_controller = Shell::Get()->overview_controller(); OverviewController* overview_controller = Shell::Get()->overview_controller();
EXPECT_TRUE(overview_controller->InOverviewSession()); EXPECT_TRUE(overview_controller->InOverviewSession());
...@@ -718,8 +715,8 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) { ...@@ -718,8 +715,8 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) {
EXPECT_FALSE(overview_controller->InOverviewSession()); EXPECT_FALSE(overview_controller->InOverviewSession());
EXPECT_FALSE(split_view_controller()->InSplitViewMode()); EXPECT_FALSE(split_view_controller()->InSplitViewMode());
// If the drag starts outside of the snap region and then into snap region, // If the drag starts outside of the snap region and then into snap region
// and the drag distance is long enough. // (kScreenEdgeInsetForSnap), and the drag distance is long enough.
StartDrag(window1.get(), start, HotseatState::kExtended); StartDrag(window1.get(), start, HotseatState::kExtended);
Drag(start + gfx::Vector2d(0, 100), 0.f, 1.f); Drag(start + gfx::Vector2d(0, 100), 0.f, 1.f);
...@@ -727,8 +724,7 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) { ...@@ -727,8 +724,7 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) {
window_drag_controller()); window_drag_controller());
// Drag into the snap region and release. // Drag into the snap region and release.
end.set_x(start.x() - 10 - end.set_x(start.x() - 10 - DragWindowFromShelfController::kMinDragDistance);
DragWindowFromShelfController::kMinDragDistanceOutsideSnapRegion);
EndDrag(end, base::nullopt); EndDrag(end, base::nullopt);
EXPECT_TRUE(overview_controller->InOverviewSession()); EXPECT_TRUE(overview_controller->InOverviewSession());
...@@ -746,8 +742,8 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) { ...@@ -746,8 +742,8 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) {
EXPECT_FALSE(overview_controller->InOverviewSession()); EXPECT_FALSE(overview_controller->InOverviewSession());
EXPECT_FALSE(split_view_controller()->InSplitViewMode()); EXPECT_FALSE(split_view_controller()->InSplitViewMode());
// If the drag starts inside of the snap region, but the drag distance is not // If the drag starts inside of the snap region (kScreenEdgeInsetForSnap), but
// long enough. // the drag distance is not long enough.
start = gfx::Point(display_bounds.x() + snap_edge_inset - 5, start = gfx::Point(display_bounds.x() + snap_edge_inset - 5,
shelf_bounds.CenterPoint().y()); shelf_bounds.CenterPoint().y());
StartDrag(window1.get(), start, HotseatState::kExtended); StartDrag(window1.get(), start, HotseatState::kExtended);
...@@ -755,8 +751,7 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) { ...@@ -755,8 +751,7 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) {
DragWindowFromShelfControllerTestApi().WaitUntilOverviewIsShown( DragWindowFromShelfControllerTestApi().WaitUntilOverviewIsShown(
window_drag_controller()); window_drag_controller());
// Drag for a small distance and release. // Drag for a small distance and release.
end.set_x(start.x() - end.set_x(start.x() - 10);
DragWindowFromShelfController::kMinDragDistanceInSnapRegion + 10);
EndDrag(end, base::nullopt); EndDrag(end, base::nullopt);
EXPECT_TRUE(overview_controller->InOverviewSession()); EXPECT_TRUE(overview_controller->InOverviewSession());
EXPECT_FALSE(split_view_controller()->InSplitViewMode()); EXPECT_FALSE(split_view_controller()->InSplitViewMode());
...@@ -772,7 +767,8 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) { ...@@ -772,7 +767,8 @@ TEST_F(DragWindowFromShelfControllerTest, DragToSnapMinDistance) {
EXPECT_FALSE(overview_controller->InOverviewSession()); EXPECT_FALSE(overview_controller->InOverviewSession());
EXPECT_FALSE(split_view_controller()->InSplitViewMode()); EXPECT_FALSE(split_view_controller()->InSplitViewMode());
// If the drag starts near the screen edge, the window should snap directly. // If the drag starts near the screen edge (kDistanceFromEdge), the window
// should snap directly.
start = gfx::Point( start = gfx::Point(
display_bounds.x() + DragWindowFromShelfController::kDistanceFromEdge - 5, display_bounds.x() + DragWindowFromShelfController::kDistanceFromEdge - 5,
shelf_bounds.CenterPoint().y()); shelf_bounds.CenterPoint().y());
...@@ -849,9 +845,7 @@ TEST_F(DragWindowFromShelfControllerTest, ...@@ -849,9 +845,7 @@ TEST_F(DragWindowFromShelfControllerTest,
// to the home screen. // to the home screen.
gfx::Point end = gfx::Point end =
start - start -
gfx::Vector2d( gfx::Vector2d(10 + DragWindowFromShelfController::kMinDragDistance, 200);
10 + DragWindowFromShelfController::kMinDragDistanceOutsideSnapRegion,
200);
EndDrag(end, base::nullopt); EndDrag(end, base::nullopt);
EXPECT_FALSE(Shell::Get()->overview_controller()->InOverviewSession()); EXPECT_FALSE(Shell::Get()->overview_controller()->InOverviewSession());
......
...@@ -733,7 +733,11 @@ SplitViewController::SnapPosition OverviewWindowDragController::GetSnapPosition( ...@@ -733,7 +733,11 @@ SplitViewController::SnapPosition OverviewWindowDragController::GetSnapPosition(
return ::ash::GetSnapPosition( return ::ash::GetSnapPosition(
GetRootWindowBeingDraggedIn(), item_->GetWindow(), GetRootWindowBeingDraggedIn(), item_->GetWindow(),
gfx::Point(location_in_screen.x(), location_in_screen.y())); gfx::Point(location_in_screen.x(), location_in_screen.y()),
area.width() * kHighlightScreenPrimaryAxisRatio +
kHighlightScreenEdgePaddingDp,
area.height() * kHighlightScreenPrimaryAxisRatio +
kHighlightScreenEdgePaddingDp);
} }
void OverviewWindowDragController::SnapWindow( void OverviewWindowDragController::SnapWindow(
......
...@@ -415,7 +415,9 @@ void ShowAppCannotSnapToast() { ...@@ -415,7 +415,9 @@ void ShowAppCannotSnapToast() {
SplitViewController::SnapPosition GetSnapPosition( SplitViewController::SnapPosition GetSnapPosition(
aura::Window* root_window, aura::Window* root_window,
aura::Window* window, aura::Window* window,
const gfx::Point& location_in_screen) { const gfx::Point& location_in_screen,
int horizontal_edge_inset,
int vertical_edge_inset) {
if (!ShouldAllowSplitView() || if (!ShouldAllowSplitView() ||
!SplitViewController::Get(root_window)->CanSnapWindow(window)) { !SplitViewController::Get(root_window)->CanSnapWindow(window)) {
return SplitViewController::NONE; return SplitViewController::NONE;
...@@ -430,10 +432,7 @@ SplitViewController::SnapPosition GetSnapPosition( ...@@ -430,10 +432,7 @@ SplitViewController::SnapPosition GetSnapPosition(
screen_util::GetDisplayWorkAreaBoundsInScreenForActiveDeskContainer( screen_util::GetDisplayWorkAreaBoundsInScreenForActiveDeskContainer(
root_window)); root_window));
if (horizontal) { if (horizontal) {
const int screen_edge_inset_for_drag = area.Inset(horizontal_edge_inset, 0);
area.width() * kHighlightScreenPrimaryAxisRatio +
kHighlightScreenEdgePaddingDp;
area.Inset(screen_edge_inset_for_drag, 0);
if (location_in_screen.x() <= area.x()) { if (location_in_screen.x() <= area.x()) {
return right_side_up ? SplitViewController::LEFT return right_side_up ? SplitViewController::LEFT
: SplitViewController::RIGHT; : SplitViewController::RIGHT;
...@@ -445,10 +444,7 @@ SplitViewController::SnapPosition GetSnapPosition( ...@@ -445,10 +444,7 @@ SplitViewController::SnapPosition GetSnapPosition(
return SplitViewController::NONE; return SplitViewController::NONE;
} }
const int screen_edge_inset_for_drag = area.Inset(0, vertical_edge_inset);
area.height() * kHighlightScreenPrimaryAxisRatio +
kHighlightScreenEdgePaddingDp;
area.Inset(0, screen_edge_inset_for_drag);
if (location_in_screen.y() <= area.y()) if (location_in_screen.y() <= area.y())
return right_side_up ? SplitViewController::LEFT return right_side_up ? SplitViewController::LEFT
: SplitViewController::RIGHT; : SplitViewController::RIGHT;
......
...@@ -142,11 +142,14 @@ ASH_EXPORT void ShowAppCannotSnapToast(); ...@@ -142,11 +142,14 @@ ASH_EXPORT void ShowAppCannotSnapToast();
// return value is not |SplitViewController::NONE|), |window| must first of all // return value is not |SplitViewController::NONE|), |window| must first of all
// satisfy |SplitViewController::CanSnapWindow| on the split view controller for // satisfy |SplitViewController::CanSnapWindow| on the split view controller for
// |root_window|, and secondly be dragged near a suitable edge of the work area // |root_window|, and secondly be dragged near a suitable edge of the work area
// of |root_window|. // of |root_window| (|horizontal_edge_inset| if dragged horizontally to snap, or
// |vertical_edge_inset| if dragged vertically to snap).
ASH_EXPORT SplitViewController::SnapPosition GetSnapPosition( ASH_EXPORT SplitViewController::SnapPosition GetSnapPosition(
aura::Window* root_window, aura::Window* root_window,
aura::Window* window, aura::Window* window,
const gfx::Point& location_in_screen); const gfx::Point& location_in_screen,
int horizontal_edge_inset,
int vertical_edge_inset);
} // namespace ash } // namespace ash
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "ash/display/screen_orientation_controller.h" #include "ash/display/screen_orientation_controller.h"
#include "ash/public/cpp/window_backdrop.h" #include "ash/public/cpp/window_backdrop.h"
#include "ash/root_window_controller.h" #include "ash/root_window_controller.h"
#include "ash/screen_util.h"
#include "ash/shelf/shelf_layout_manager.h" #include "ash/shelf/shelf_layout_manager.h"
#include "ash/shell.h" #include "ash/shell.h"
#include "ash/system/overview/overview_button_tray.h" #include "ash/system/overview/overview_button_tray.h"
...@@ -355,9 +356,16 @@ SplitViewController::SnapPosition TabletModeWindowDragDelegate::GetSnapPosition( ...@@ -355,9 +356,16 @@ SplitViewController::SnapPosition TabletModeWindowDragDelegate::GetSnapPosition(
return SplitViewController::NONE; return SplitViewController::NONE;
} }
const gfx::Rect area =
screen_util::GetDisplayWorkAreaBoundsInScreenForActiveDeskContainer(
dragged_window_);
SplitViewController::SnapPosition snap_position = SplitViewController::SnapPosition snap_position =
::ash::GetSnapPosition(Shell::GetPrimaryRootWindow(), dragged_window_, ::ash::GetSnapPosition(Shell::GetPrimaryRootWindow(), dragged_window_,
gfx::ToRoundedPoint(location_in_screen)); gfx::ToRoundedPoint(location_in_screen),
area.width() * kHighlightScreenPrimaryAxisRatio +
kHighlightScreenEdgePaddingDp,
area.height() * kHighlightScreenPrimaryAxisRatio +
kHighlightScreenEdgePaddingDp);
// For portrait mode, since the drag always starts from the top of the // For portrait mode, since the drag always starts from the top of the
// screen, we only allow the window to be dragged to snap to the bottom of // screen, we only allow the window to be dragged to snap to the bottom of
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment