Constrain event location to root window bounds

if
a) the mouse event is not captured, or
b) even if captured, it's considered a plain click.

  see bug for repro step.

Bug: 900614
Test: updated tests to match this expectation. Also tested manually.
Change-Id: I2668fa5692a0e4381e72d2d22bf5543914a8ca34
Reviewed-on: https://chromium-review.googlesource.com/c/1328709
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607431}

ash/display/window_tree_host_manager_unittest.cc

@@ -1287,13 +1287,14 @@ TEST_F(WindowTreeHostManagerTest, ConvertHostToRootCoords) {
 
   ui::test::EventGenerator generator(root_windows[0]);
   generator.MoveMouseToInHost(0, 0);
-  EXPECT_EQ("0,375", event_handler.GetLocationAndReset());
+  // The mouse location must be inside the root bounds in dp.
+  EXPECT_EQ("0,374", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(599, 0);
   EXPECT_EQ("0,0", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(599, 399);
   EXPECT_EQ("249,0", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(0, 399);
-  EXPECT_EQ("249,375", event_handler.GetLocationAndReset());
+  EXPECT_EQ("249,374", event_handler.GetLocationAndReset());
 
   UpdateDisplay("600x400*2/u@0.8");
   display1 = display::Screen::GetScreen()->GetPrimaryDisplay();
@@ -1303,13 +1304,13 @@ TEST_F(WindowTreeHostManagerTest, ConvertHostToRootCoords) {
   EXPECT_EQ(0.8f, GetStoredZoomScale(display1.id()));
 
   generator.MoveMouseToInHost(0, 0);
-  EXPECT_EQ("375,250", event_handler.GetLocationAndReset());
+  EXPECT_EQ("374,249", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(599, 0);
-  EXPECT_EQ("0,250", event_handler.GetLocationAndReset());
+  EXPECT_EQ("0,249", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(599, 399);
   EXPECT_EQ("0,0", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(0, 399);
-  EXPECT_EQ("375,0", event_handler.GetLocationAndReset());
+  EXPECT_EQ("374,0", event_handler.GetLocationAndReset());
 
   UpdateDisplay("600x400*2/l@0.8");
   display1 = display::Screen::GetScreen()->GetPrimaryDisplay();
@@ -1319,9 +1320,9 @@ TEST_F(WindowTreeHostManagerTest, ConvertHostToRootCoords) {
   EXPECT_EQ(0.8f, GetStoredZoomScale(display1.id()));
 
   generator.MoveMouseToInHost(0, 0);
-  EXPECT_EQ("250,0", event_handler.GetLocationAndReset());
+  EXPECT_EQ("249,0", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(599, 0);
-  EXPECT_EQ("250,374", event_handler.GetLocationAndReset());
+  EXPECT_EQ("249,374", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(599, 399);
   EXPECT_EQ("0,374", event_handler.GetLocationAndReset());
   generator.MoveMouseToInHost(0, 399);
@@ -1468,10 +1469,10 @@ TEST_F(WindowTreeHostManagerTest, UpdateMouseLocationAfterDisplayChange) {
 
   aura::Env* env = Shell::Get()->aura_env();
 
-  ui::test::EventGenerator generator(root_windows[0]);
+  ui::test::EventGenerator generator_on_2nd(root_windows[1]);
 
   // Set the initial position.
-  generator.MoveMouseToInHost(350, 150);
+  generator_on_2nd.MoveMouseToInHost(150, 150);
   EXPECT_EQ("350,150", env->last_mouse_location().ToString());
 
   // A mouse pointer will stay in the 2nd display.
@@ -1494,7 +1495,8 @@ TEST_F(WindowTreeHostManagerTest, UpdateMouseLocationAfterDisplayChange) {
   EXPECT_EQ("150,150", env->last_mouse_location().ToString());
 
   // Move the mouse pointer to the bottom of 1st display.
-  generator.MoveMouseToInHost(150, 290);
+  ui::test::EventGenerator generator_on_1st(root_windows[0]);
+  generator_on_1st.MoveMouseToInHost(150, 290);
   EXPECT_EQ("150,290", env->last_mouse_location().ToString());
 
   // The mouse pointer is now on 2nd display.

ash/highlighter/highlighter_controller_unittest.cc

@@ -421,8 +421,9 @@ TEST_F(HighlighterControllerTest, SelectionInsideScreen) {
   constexpr float display_scales[] = {1.f, 1.5f, 2.0f};
 
   for (size_t i = 0; i < sizeof(display_scales) / sizeof(float); ++i) {
-    std::string display_spec =
-        base::StringPrintf("1000x1000*%.2f", display_scales[i]);
+    // 2nd display is for offscreen test.
+    std::string display_spec = base::StringPrintf(
+        "1000x1000*%.2f,500x1000*%.2f", display_scales[i], display_scales[i]);
     SCOPED_TRACE(display_spec);
     UpdateDisplayAndWaitForCompositingEnded(display_spec);
 
@@ -462,11 +463,11 @@ TEST_F(HighlighterControllerTest, SelectionInsideScreen) {
     EXPECT_TRUE(controller_test_api_->HandleSelectionCalled());
     EXPECT_TRUE(screen.Contains(controller_test_api_->selection()));
 
-    // Horizontal stroke completely offscreen.
+    // Vertical stroke completely offscreen.
     controller_test_api_->ResetSelection();
-    event_generator->MoveTouch(gfx::Point(0, -100));
+    event_generator->MoveTouch(gfx::Point(1100, 100));
     event_generator->PressTouch();
-    event_generator->MoveTouch(gfx::Point(1000, -100));
+    event_generator->MoveTouch(gfx::Point(1100, 500));
     event_generator->ReleaseTouch();
     controller_test_api_->SimulateInterruptedStrokeTimeout();
     EXPECT_FALSE(controller_test_api_->HandleSelectionCalled());

ash/magnifier/docked_magnifier_controller_unittest.cc

@@ -131,6 +131,14 @@ class DockedMagnifierTest : public NoSessionAshTestBase {
               gfx::ToFlooredPoint(point_of_interest_in_root_f));
   }
 
+  void TouchPoint(const gfx::Point& touch_point_in_screen) {
+    // TODO(oshima): Currently touch event doesn't update the
+    // event dispatcher in the event generator. Fix it and use
+    // touch event insteead.
+    auto* generator = GetEventGenerator();
+    generator->GestureTapAt(touch_point_in_screen);
+  }
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 
@@ -446,7 +454,7 @@ TEST_F(DockedMagnifierTest, TouchEvents) {
   // Generate some touch events in both displays and expect the magnifier
   // viewport moves accordingly.
   gfx::Point touch_point(200, 350);
-  GetEventGenerator()->PressMoveAndReleaseTouchTo(touch_point);
+  TouchPoint(touch_point);
   const views::Widget* viewport_widget =
       controller()->GetViewportWidgetForTesting();
   EXPECT_EQ(root_windows[0], viewport_widget->GetNativeView()->GetRootWindow());
@@ -454,7 +462,8 @@ TEST_F(DockedMagnifierTest, TouchEvents) {
 
   // Touch a new point in the other display.
   touch_point = gfx::Point(900, 200);
-  GetEventGenerator()->PressMoveAndReleaseTouchTo(touch_point);
+  TouchPoint(touch_point);
+
   // New viewport widget is created in the second display.
   ASSERT_NE(viewport_widget, controller()->GetViewportWidgetForTesting());
   viewport_widget = controller()->GetViewportWidgetForTesting();

ash/root_window_controller.cc

@@ -66,6 +66,7 @@
 #include "ui/aura/client/drag_drop_client.h"
 #include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/client/window_types.h"
+#include "ui/aura/null_window_targeter.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_observer.h"
@@ -259,6 +260,80 @@ bool ShouldDestroyWindowInCloseChildWindows(aura::Window* window) {
   return window->owned_by_parent();
 }
 
+class RootWindowTargeter : public aura::WindowTargeter {
+ public:
+  RootWindowTargeter() = default;
+  ~RootWindowTargeter() override = default;
+
+ protected:
+  aura::Window* FindTargetForLocatedEvent(aura::Window* window,
+                                          ui::LocatedEvent* event) override {
+    if (!window->parent() && !window->bounds().Contains(event->location()) &&
+        IsEventInsideDisplayForTelemetryHack(window, event)) {
+      auto* dispatcher = window->GetHost()->dispatcher();
+      bool has_capture_target = !!dispatcher->mouse_pressed_handler() ||
+                                !!aura::client::GetCaptureWindow(window);
+
+      // Make sure that event location is within the root window bounds if
+      // 1) mouse event isn't captured.
+      // 2) A mouse is clicked without movement and capture.
+      //
+      // The event can be outside on some scale factor due to rounding, or due
+      // to not well calibrated a touch screen, or Detect this situation and
+      // adjust the location.
+      bool bounded_click = ShouldConstrainMouseClick(event, has_capture_target);
+      if (!has_capture_target || bounded_click) {
+        gfx::Point new_location =
+            FitPointToBounds(event->location(), window->bounds());
+        // Do not change |location_f|. It's used to compute pixel position and
+        // such client should know what they're doing.
+        event->set_location(new_location);
+        event->set_root_location(new_location);
+      }
+    }
+    return aura::WindowTargeter::FindTargetForLocatedEvent(window, event);
+  }
+
+  // Stop-gap workaround for telemetry tests that send events far outside of the
+  // display (e.g. 512, -4711). Fix the test and remove this (crbgu.com/904623).
+  bool IsEventInsideDisplayForTelemetryHack(aura::Window* window,
+                                            ui::LocatedEvent* event) {
+    constexpr int ExtraMarginForTelemetryTest = -10;
+    gfx::Rect bounds = window->bounds();
+    bounds.Inset(ExtraMarginForTelemetryTest, ExtraMarginForTelemetryTest);
+    return bounds.Contains(event->location());
+  }
+
+ private:
+  // Returns true if the mouse event should be constrainted.
+  bool ShouldConstrainMouseClick(ui::LocatedEvent* event,
+                                 bool has_capture_target) {
+    if (event->type() == ui::ET_MOUSE_PRESSED && !has_capture_target) {
+      last_mouse_event_type_ = ui::ET_MOUSE_PRESSED;
+      return true;
+    }
+    if (last_mouse_event_type_ == ui::ET_MOUSE_PRESSED &&
+        event->type() == ui::ET_MOUSE_RELEASED && has_capture_target) {
+      last_mouse_event_type_ = ui::ET_UNKNOWN;
+      return true;
+    }
+    // For other cases, reset the state
+    if (event->type() != ui::ET_MOUSE_CAPTURE_CHANGED)
+      last_mouse_event_type_ = ui::ET_UNKNOWN;
+    return false;
+  }
+
+  gfx::Point FitPointToBounds(const gfx::Point p, const gfx::Rect& bounds) {
+    return gfx::Point(
+        std::min(std::max(bounds.x(), p.x()), bounds.right() - 1),
+        std::min(std::max(bounds.y(), p.y()), bounds.bottom() - 1));
+  }
+
+  ui::EventType last_mouse_event_type_ = ui::ET_UNKNOWN;
+
+  DISALLOW_COPY_AND_ASSIGN(RootWindowTargeter);
+};
+
 }  // namespace
 
 // static
@@ -429,6 +504,9 @@ const aura::Window* RootWindowController::GetContainer(int container_id) const {
 }
 
 void RootWindowController::Shutdown() {
+  auto targeter = GetRootWindow()->SetEventTargeter(
+      std::make_unique<aura::NullWindowTargeter>());
+
   touch_exploration_manager_.reset();
 
   ResetRootForNewWindowsIfNecessary();
@@ -447,6 +525,12 @@ void RootWindowController::Shutdown() {
   system_wallpaper_.reset();
   lock_screen_action_background_controller_.reset();
   aura::client::SetScreenPositionClient(root_window, nullptr);
+
+  // The targeter may still on the stack, so delete it later.
+  if (targeter) {
+    base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
+                                                    std::move(targeter));
+  }
 }
 
 void RootWindowController::CloseChildWindows() {
@@ -616,6 +700,9 @@ void RootWindowController::Init(RootWindowType root_window_type) {
   aura::Window* root_window = GetRootWindow();
   Shell* shell = Shell::Get();
   shell->InitRootWindow(root_window);
+  auto old_targeter =
+      root_window->SetEventTargeter(std::make_unique<RootWindowTargeter>());
+  DCHECK(!old_targeter);
 
   CreateContainers();
   CreateSystemWallpaper(root_window_type);

ash/root_window_controller_unittest.cc

@@ -748,26 +748,31 @@ TEST_F(VirtualKeyboardRootWindowControllerTest,
 
   aura::Window* contents_window =
       keyboard::KeyboardController::Get()->GetKeyboardWindow();
-  contents_window->SetBounds(gfx::Rect());
   contents_window->Show();
+  keyboard::KeyboardController::Get()->ShowKeyboard(false);
 
   // Make sure no pending mouse events in the queue.
   RunAllPendingInMessageLoop();
 
+  // TODO(oshima|yhanada): This simply make sure that targeting logic works, but
+  // doesn't mean it'll deliver the event to the target. Fix this to make this
+  // more reliable.
   ui::test::TestEventHandler handler;
   root_window->AddPreTargetHandler(&handler);
 
   ui::test::EventGenerator event_generator(root_window, contents_window);
   event_generator.ClickLeftButton();
-  int expected_mouse_presses = 1;
-  EXPECT_EQ(expected_mouse_presses, handler.num_mouse_events() / 2);
+  EXPECT_EQ(2, handler.num_mouse_events());
 
   for (int block_reason = FIRST_BLOCK_REASON;
        block_reason < NUMBER_OF_BLOCK_REASONS; ++block_reason) {
+    SCOPED_TRACE(base::StringPrintf("Reason: %d", block_reason));
     BlockUserSession(static_cast<UserSessionBlockReason>(block_reason));
+    handler.Reset();
     event_generator.ClickLeftButton();
-    expected_mouse_presses++;
-    EXPECT_EQ(expected_mouse_presses, handler.num_mouse_events() / 2);
+    // Click may generate CAPTURE_CHANGED event so make sure it's more than
+    // 2 (press,release);
+    EXPECT_LE(2, handler.num_mouse_events());
     UnblockUserSession();
   }
   root_window->RemovePreTargetHandler(&handler);

ash/system/palette/tools/screenshot_unittest.cc

@@ -60,7 +60,7 @@ TEST_F(ScreenshotToolTest, EnablingCaptureRegionCallsDelegateAndDisablesTool) {
   EXPECT_CALL(*palette_tool_delegate_.get(),
               DisableTool(PaletteToolId::CAPTURE_REGION));
 
-  const gfx::Rect selection(100, 200, 300, 400);
+  const gfx::Rect selection(100, 200, 300, 399);
   ui::test::EventGenerator* event_generator = GetEventGenerator();
   event_generator->EnterPenPointerMode();
   event_generator->MoveTouch(selection.origin());

ash/wm/immersive_fullscreen_controller_unittest.cc

@@ -370,6 +370,8 @@ TEST_F(ImmersiveFullscreenControllerTest, RevealedLock) {
 
 // Test mouse event processing for top-of-screen reveal triggering.
 TEST_F(ImmersiveFullscreenControllerTest, OnMouseEvent) {
+  // Create 2nd display for off screen test.
+  UpdateDisplay("800x600, 800x600");
   // Set up initial state.
   SetEnabled(true);
   ASSERT_TRUE(controller()->IsEnabled());