Commit df506a43 authored by Andrew Xu's avatar Andrew Xu Committed by Commit Bot

Fix bug that touch-dragging window to the display edge causes system crash

In current code, we have no constraints on gesture event's location.
As result, when window is dragged by touch-gestures to the edge of display,
gesture's location may be in a wrong display. It will lead to system crash.
In this CL, constraint on gesture event's location is added

Change-Id: I2c03d369df0b7bed143edb3f04a9c7aad5968d4a
Test: ash_unittests
Bug: 917060
Reviewed-on: https://chromium-review.googlesource.com/c/1413416
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625716}
parent 0bb97b95
...@@ -524,6 +524,33 @@ TEST_F(ToplevelWindowEventHandlerTest, GestureDrag) { ...@@ -524,6 +524,33 @@ TEST_F(ToplevelWindowEventHandlerTest, GestureDrag) {
window_state->GetRestoreBoundsInScreen().ToString()); window_state->GetRestoreBoundsInScreen().ToString());
} }
// Verifies that window dragged by touch-gestures to the edge of display
// will not lead to system crash (see https://crbug.com/917060).
TEST_F(ToplevelWindowEventHandlerTest, GestureDragMultiDisplays) {
UpdateDisplay("800x600, 800x600");
std::unique_ptr<aura::Window> target(CreateTestWindowInShellWithDelegate(
new TestWindowDelegate(HTCAPTION), 0, gfx::Rect(0, 0, 100, 100)));
wm::WindowState* window_state = wm::GetWindowState(target.get());
ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow(),
target.get());
gfx::Rect old_bounds = target->bounds();
gfx::Point location(5, 5);
gfx::Point end = location;
// On real device, gesture event's location may not be accurate. For example,
// when window is dragged by touch-gestures to the edge of display, it may
// create gesture events with location out of the display bounds. Let |end| be
// out of the primary display's bounds to emulate this situation.
end.Offset(800, 0);
generator.GestureScrollSequence(location, end,
base::TimeDelta::FromMilliseconds(5), 10);
// Verify that the window has moved after the gesture.
EXPECT_NE(old_bounds.ToString(), target->bounds().ToString());
EXPECT_EQ(mojom::WindowStateType::RIGHT_SNAPPED,
window_state->GetStateType());
}
// Tests that a gesture cannot minimize an unminimizeable window. // Tests that a gesture cannot minimize an unminimizeable window.
TEST_F(ToplevelWindowEventHandlerTest, TEST_F(ToplevelWindowEventHandlerTest,
GestureAttemptMinimizeUnminimizeableWindow) { GestureAttemptMinimizeUnminimizeableWindow) {
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "ui/aura/window_observer.h" #include "ui/aura/window_observer.h"
#include "ui/base/hit_test.h" #include "ui/base/hit_test.h"
#include "ui/events/event.h" #include "ui/events/event.h"
#include "ui/wm/core/coordinate_conversion.h"
namespace ash { namespace ash {
namespace wm { namespace wm {
...@@ -312,10 +313,33 @@ void WmToplevelWindowEventHandler::OnGestureEvent(ui::GestureEvent* event, ...@@ -312,10 +313,33 @@ void WmToplevelWindowEventHandler::OnGestureEvent(ui::GestureEvent* event,
return; return;
switch (event->type()) { switch (event->type()) {
case ui::ET_GESTURE_SCROLL_UPDATE: case ui::ET_GESTURE_SCROLL_UPDATE: {
gfx::Rect bounds_in_screen = target->GetRootWindow()->GetBoundsInScreen();
gfx::Point screen_location = event->location();
::wm::ConvertPointToScreen(target, &screen_location);
// It is physically not possible to move a touch pointer from one display
// to another, so constrain the bounds to the display. This is important,
// as it is possible for touch points to extend outside the bounds of the
// display (as happens with gestures on the bezel), and dragging via touch
// should not trigger moving to a new display.(see
// https://crbug.com/917060)
if (!bounds_in_screen.Contains(screen_location)) {
int x = std::max(
std::min(screen_location.x(), bounds_in_screen.right() - 1),
bounds_in_screen.x());
int y = std::max(
std::min(screen_location.y(), bounds_in_screen.bottom() - 1),
bounds_in_screen.y());
gfx::Point updated_location(x, y);
::wm::ConvertPointFromScreen(target, &updated_location);
event->set_location(updated_location);
}
HandleDrag(target, event); HandleDrag(target, event);
event->StopPropagation(); event->StopPropagation();
return; return;
}
case ui::ET_GESTURE_SCROLL_END: case ui::ET_GESTURE_SCROLL_END:
// We must complete the drag here instead of as a result of ET_GESTURE_END // We must complete the drag here instead of as a result of ET_GESTURE_END
// because otherwise the drag will be reverted when EndMoveLoop() is // because otherwise the drag will be reverted when EndMoveLoop() is
......
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