Use screen coordinates in ExtendeDragSource

1) OnToplevelWindowDragStarted's location is in screen coordinates
2) It alsp simplifies the new window's bounds computation

Bug: 1306688
Test: covered by unittests
Change-Id: Ib206eacb540452a56a16f2e6a00bcb44b1511cc3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3659257
Reviewed-by: Antonio Gomes <tonikitoo@igalia.com>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1006495}

ash/display/mouse_cursor_event_filter_unittest.cc

@@ -150,11 +150,10 @@ TEST_F(MouseCursorEventFilterTest, CursorDeviceScaleFactor) {
       display::test::CreateDisplayLayout(display_manager(),
                                          display::DisplayPlacement::RIGHT, 0));
   auto* cursor_manager = Shell::Get()->cursor_manager();
-
   EXPECT_EQ(1.0f, cursor_manager->GetCursor().image_scale_factor());
-  TestIfMouseWarpsAt(gfx::Point(399, 200));
+  GetEventGenerator()->MoveMouseTo(401, 200);
   EXPECT_EQ(2.0f, cursor_manager->GetCursor().image_scale_factor());
-  TestIfMouseWarpsAt(gfx::Point(400, 200));
+  GetEventGenerator()->MoveMouseTo(399, 200);
   EXPECT_EQ(1.0f, cursor_manager->GetCursor().image_scale_factor());
 }
 

ash/test/ash_test_base.cc

@@ -85,8 +85,15 @@ class AshEventGeneratorDelegate
   ui::EventTarget* GetTargetAt(const gfx::Point& point_in_screen) override {
     display::Screen* screen = display::Screen::GetScreen();
     display::Display display = screen->GetDisplayNearestPoint(point_in_screen);
+    if (current_display_id_ != display.id()) {
+      Shell::Get()->cursor_manager()->SetDisplay(display);
+      current_display_id_ = display.id();
+    }
     return Shell::GetRootWindowForDisplayId(display.id())->GetHost()->window();
   }
+
+ private:
+  int64_t current_display_id_ = display::kInvalidDisplayId;
 };
 
 }  // namespace

ash/wm/drag_window_resizer_unittest.cc

@@ -732,11 +732,8 @@ TEST_F(DragWindowResizerTest, CursorDeviceScaleFactor) {
         CreateDragWindowResizer(window_.get(), gfx::Point(), HTCAPTION));
     EXPECT_EQ(1.0f, cursor_manager->GetCursor().image_scale_factor());
     ASSERT_TRUE(resizer.get());
-    // TODO(crbug.com/990589): Unit tests should be able to simulate mouse input
-    // without having to call |CursorManager::SetDisplay|.
-    cursor_manager->SetDisplay(display1);
     resizer->Drag(CalculateDragPoint(*resizer, 399, 200), 0);
-    TestIfMouseWarpsAt(gfx::Point(399, 200));
+    TestIfMouseWarpsAt(gfx::Point(699, 200));
     EXPECT_EQ(2.0f, cursor_manager->GetCursor().image_scale_factor());
     resizer->CompleteDrag();
     EXPECT_EQ(2.0f, cursor_manager->GetCursor().image_scale_factor());
@@ -752,9 +749,6 @@ TEST_F(DragWindowResizerTest, CursorDeviceScaleFactor) {
         CreateDragWindowResizer(window_.get(), gfx::Point(), HTCAPTION));
     EXPECT_EQ(2.0f, cursor_manager->GetCursor().image_scale_factor());
     ASSERT_TRUE(resizer.get());
-    // TODO(crbug.com/990589): Unit tests should be able to simulate mouse input
-    // without having to call |CursorManager::SetDisplay|.
-    cursor_manager->SetDisplay(display0);
     resizer->Drag(CalculateDragPoint(*resizer, -200, 200), 0);
     TestIfMouseWarpsAt(gfx::Point(400, 200));
     EXPECT_EQ(1.0f, cursor_manager->GetCursor().image_scale_factor());

components/exo/extended_drag_source.cc

@@ -20,6 +20,7 @@
 #include "components/exo/wm_helper.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/aura/client/aura_constants.h"
+#include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_delegate.h"
 #include "ui/aura/window_event_dispatcher.h"
@@ -219,7 +220,7 @@ void ExtendedDragSource::OnToplevelWindowDragStarted(
   MaybeLockCursor();
 
   if (dragged_window_holder_ && dragged_window_holder_->toplevel_window())
-    StartDrag(dragged_window_holder_->toplevel_window(), start_location);
+    StartDrag(dragged_window_holder_->toplevel_window());
 }
 
 DragOperation ExtendedDragSource::OnToplevelWindowDragDropped() {
@@ -239,7 +240,9 @@ void ExtendedDragSource::OnToplevelWindowDragCancelled() {
 
 void ExtendedDragSource::OnToplevelWindowDragEvent(ui::LocatedEvent* event) {
   DCHECK(event);
+  aura::Window* target = static_cast<aura::Window*>(event->target());
   pointer_location_ = event->root_location_f();
+  wm::ConvertPointToScreen(target->GetRootWindow(), &pointer_location_);
 
   if (!dragged_window_holder_)
     return;
@@ -278,13 +281,12 @@ void ExtendedDragSource::UnlockCursor() {
   }
 }
 
-void ExtendedDragSource::StartDrag(aura::Window* toplevel,
-                                   const gfx::PointF& pointer_location) {
+void ExtendedDragSource::StartDrag(aura::Window* toplevel) {
   // Ensure |toplevel| window does skip events while it's being dragged.
   event_blocker_ =
       std::make_unique<aura::ScopedWindowEventTargetingBlocker>(toplevel);
 
-  DVLOG(1) << "Starting drag. pointer_loc=" << pointer_location.ToString();
+  DVLOG(1) << "Starting drag. pointer_loc=" << pointer_location_.ToString();
   auto* toplevel_handler = ash::Shell::Get()->toplevel_window_event_handler();
   auto move_source = drag_event_source_ == ui::mojom::DragEventSource::kTouch
                          ? ::wm::WINDOW_MOVE_SOURCE_TOUCH
@@ -305,8 +307,12 @@ void ExtendedDragSource::StartDrag(aura::Window* toplevel,
 
   // TODO(crbug.com/1167581): Experiment setting |update_gesture_target| back
   // to true when capture is removed from drag and drop.
+
+  gfx::PointF pointer_location_in_parent(pointer_location_);
+  wm::ConvertPointFromScreen(toplevel->parent(), &pointer_location_in_parent);
+
   toplevel_handler->AttemptToStartDrag(
-      toplevel, pointer_location, HTCAPTION, move_source,
+      toplevel, pointer_location_in_parent, HTCAPTION, move_source,
       std::move(end_closure),
       /*update_gesture_target=*/false,
       /*grab_capture =*/
@@ -324,7 +330,6 @@ void ExtendedDragSource::OnDraggedWindowVisibilityChanging(bool visible) {
 
   aura::Window* toplevel = dragged_window_holder_->toplevel_window();
   DCHECK(toplevel);
-
   DCHECK(drag_source_window_);
   toplevel->SetProperty(ash::kIsDraggingTabsKey, true);
   if (drag_source_window_ != toplevel) {
@@ -349,11 +354,14 @@ void ExtendedDragSource::OnDraggedWindowVisibilityChanged(bool visible) {
   // The |toplevel| window for the dragged surface has just been created and
   // it's about to be mapped. Calculate and set its position based on
   // |drag_offset_| and |pointer_location_| before starting the actual drag.
-  auto screen_location = CalculateOrigin(toplevel);
+  auto screen_location =
+      gfx::ToFlooredPoint(pointer_location_ - dragged_window_holder_->offset());
 
   auto toplevel_bounds =
       gfx::Rect({screen_location, toplevel->bounds().size()});
-  toplevel->SetBounds(toplevel_bounds);
+  auto display = display::Screen::GetScreen()->GetDisplayNearestWindow(
+      drag_source_window_);
+  toplevel->SetBoundsInScreen(toplevel_bounds, display);
 
   if (WMHelper::GetInstance()->InTabletMode()) {
     // The bounds that is stored in ash::kRestoreBoundsOverrideKey will be used
@@ -366,16 +374,7 @@ void ExtendedDragSource::OnDraggedWindowVisibilityChanged(bool visible) {
   DVLOG(1) << "Dragged window mapped. toplevel=" << toplevel
            << " origin=" << screen_location.ToString();
 
-  gfx::PointF pointer_location(screen_location +
-                               dragged_window_holder_->offset());
-  StartDrag(toplevel, pointer_location);
-}
-
-gfx::Point ExtendedDragSource::CalculateOrigin(aura::Window* target) const {
-  DCHECK(dragged_window_holder_);
-  gfx::Point screen_location = gfx::ToRoundedPoint(pointer_location_);
-  wm::ConvertPointToScreen(target->GetRootWindow(), &screen_location);
-  return screen_location - dragged_window_holder_->offset();
+  StartDrag(toplevel);
 }
 
 void ExtendedDragSource::Cleanup() {

components/exo/extended_drag_source.h

@@ -92,11 +92,9 @@ class ExtendedDragSource : public DataSourceObserver,
 
   void MaybeLockCursor();
   void UnlockCursor();
-  void StartDrag(aura::Window* toplevel,
-                 const gfx::PointF& pointer_location_in_screen);
+  void StartDrag(aura::Window* toplevel);
   void OnDraggedWindowVisibilityChanging(bool visible);
   void OnDraggedWindowVisibilityChanged(bool visible);
-  gfx::Point CalculateOrigin(aura::Window* target) const;
   void Cleanup();
 
   static ExtendedDragSource* instance_;
@@ -107,6 +105,7 @@ class ExtendedDragSource : public DataSourceObserver,
   // tied to the zcr_extended_drag_source_v1 object it's attached to.
   Delegate* const delegate_;
 
+  // The pointer location in screen coordinates.
   gfx::PointF pointer_location_;
   ui::mojom::DragEventSource drag_event_source_;
   bool cursor_locked_ = false;

components/exo/extended_drag_source_unittest.cc

@@ -14,6 +14,7 @@
 #include "ash/public/cpp/window_properties.h"
 #include "ash/shell.h"
 #include "ash/wm/toplevel_window_event_handler.h"
+#include "ash/wm/window_state.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind.h"
 #include "base/test/gmock_callback_support.h"
@@ -596,9 +597,9 @@ TEST_F(ExtendedDragSourceTest, DragWithScreenCoordinates) {
   auto operation = DragDropOperation::Create(
       &data_exchange_delegate, data_source.get(), shell_surface->root_surface(),
       nullptr, location, ui::mojom::DragEventSource::kMouse);
-
   auto* drag_drop_controller = static_cast<ash::DragDropController*>(
       aura::client::GetDragDropClient(ash::Shell::GetPrimaryRootWindow()));
+
   EXPECT_FALSE(shell_surface->IsDragged());
   base::RunLoop loop;
   drag_drop_controller->SetLoopClosureForTesting(
@@ -615,4 +616,68 @@ TEST_F(ExtendedDragSourceTest, DragWithScreenCoordinates) {
   EXPECT_FALSE(shell_surface->IsDragged());
 }
 
+TEST_F(ExtendedDragSourceTest, DragToAnotherDisplay) {
+  UpdateDisplay("800x600,800x600");
+
+  // Create and map a toplevel shell surface.
+  auto shell_surface =
+      exo::test::ShellSurfaceBuilder({32, 32}).BuildShellSurface();
+  auto* surface = shell_surface->root_surface();
+
+  shell_surface->GetWidget()->SetBounds({810, 10, 32, 32});
+
+  auto display = display::Screen::GetScreen()->GetDisplayNearestWindow(
+      shell_surface->GetWidget()->GetNativeWindow());
+  EXPECT_EQ(gfx::Rect(800, 0, 800, 600), display.bounds());
+
+  gfx::Rect expected_bounds =
+      shell_surface->GetWidget()->GetWindowBoundsInScreen();
+
+  constexpr int kDragOffset = 10;
+  extended_drag_source_->Drag(surface, gfx::Vector2d(kDragOffset, 0));
+
+  // Start the DND + extended-drag session.
+  // Creates a mouse-pressed event before starting the drag session.
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  generator->MoveMouseTo({810 + kDragOffset, 10});
+  generator->PressLeftButton();
+
+  // Start a DragDropOperation.
+  drag_drop_controller_->set_should_block_during_drag_drop(true);
+  seat_->StartDrag(data_source_.get(), surface, /*icon=*/nullptr,
+                   ui::mojom::DragEventSource::kMouse);
+  // Just move to the middle to avoid snapping.
+  int x_movement = 500;
+  expected_bounds.set_x(310);
+
+  constexpr int kXDragDelta = 20;
+  auto* toplevel_handler = ash::Shell::Get()->toplevel_window_event_handler();
+
+  base::RunLoop loop;
+  drag_drop_controller_->SetLoopClosureForTesting(
+      base::BindLambdaForTesting([&]() {
+        if (x_movement == 500) {
+          auto* window_state = ash::WindowState::Get(
+              shell_surface->GetWidget()->GetNativeWindow());
+          EXPECT_EQ(gfx::PointF(20, 10),
+                    window_state->drag_details()->initial_location_in_parent);
+        }
+        if (x_movement > 0) {
+          x_movement -= kXDragDelta;
+          generator->MoveMouseBy(-kXDragDelta, 0);
+          EXPECT_TRUE(toplevel_handler->is_drag_in_progress());
+        } else {
+          generator->ReleaseLeftButton();
+        }
+      }),
+      loop.QuitClosure());
+  loop.Run();
+  EXPECT_FALSE(toplevel_handler->is_drag_in_progress());
+  EXPECT_EQ(expected_bounds,
+            shell_surface->GetWidget()->GetWindowBoundsInScreen());
+  display = display::Screen::GetScreen()->GetDisplayNearestWindow(
+      shell_surface->GetWidget()->GetNativeWindow());
+  EXPECT_EQ(gfx::Rect(0, 0, 800, 600), display.bounds());
+}
+
 }  // namespace exo

components/exo/pointer.cc

@@ -486,8 +486,6 @@ void Pointer::OnMouseEvent(ui::MouseEvent* event) {
   if (event->type() == ui::ET_MOUSE_CAPTURE_CHANGED)
     return;
 
-  seat_->SetLastPointerLocation(event->root_location_f());
-
   Surface* target = GetEffectiveTargetForEvent(event);
   gfx::PointF location_in_target = event->location_f();
   if (target) {

components/exo/seat.cc

@@ -31,6 +31,7 @@
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
 #include "ui/base/data_transfer_policy/data_transfer_endpoint_serializer.h"
+#include "ui/display/screen.h"
 #include "ui/events/event_utils.h"
 #include "ui/events/platform/platform_event_source.h"
 #include "ui/gfx/geometry/point_f.h"
@@ -130,13 +131,11 @@ void Seat::StartDrag(DataSource* source,
                      Surface* icon,
                      ui::mojom::DragEventSource event_source) {
   // DragDropOperation manages its own lifetime.
+  auto cursor_location =
+      gfx::PointF(display::Screen::GetScreen()->GetCursorScreenPoint());
   drag_drop_operation_ =
       DragDropOperation::Create(data_exchange_delegate_.get(), source, origin,
-                                icon, last_pointer_location_, event_source);
-}
-
-void Seat::SetLastPointerLocation(const gfx::PointF& last_pointer_location) {
-  last_pointer_location_ = last_pointer_location;
+                                icon, cursor_location, event_source);
 }
 
 void Seat::AbortPendingDragOperation() {

components/exo/seat.h

@@ -118,10 +118,6 @@ class Seat : public aura::client::FocusChangeObserver,
                  Surface* icon,
                  ui::mojom::DragEventSource event_source);
 
-  // Sets the last location in screen coordinates, irrespective of mouse or
-  // touch.
-  void SetLastPointerLocation(const gfx::PointF& last_pointer_location);
-
   // Abort any drag operations that haven't been started yet.
   void AbortPendingDragOperation();
 
@@ -237,8 +233,6 @@ class Seat : public aura::client::FocusChangeObserver,
   // True while Seat is updating clipboard data to selection source.
   bool changing_clipboard_data_to_selection_source_;
 
-  gfx::PointF last_pointer_location_;
-
   bool was_shutdown_ = false;
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)

components/exo/touch.cc

@@ -65,8 +65,6 @@ void Touch::OnTouchEvent(ui::TouchEvent* event) {
   if (seat_->was_shutdown())
     return;
 
-  seat_->SetLastPointerLocation(event->root_location_f());
-
   bool send_details = false;
 
   const int touch_pointer_id = event->pointer_details().id;