Keep root_location to be in pixels and display coordinates in WS.

services/ui/ws/window_manager_state.cc

Index: services/ui/ws/window_manager_state.cc
diff --git a/services/ui/ws/window_manager_state.cc b/services/ui/ws/window_manager_state.cc
index 6657f1500c52b9d070a88b1ef07e96d2023a9ba1..e9a40a277ede18e7b06283b2864ad5ee2cea382b 100644
--- a/services/ui/ws/window_manager_state.cc
+++ b/services/ui/ws/window_manager_state.cc
@@ -22,6 +22,7 @@
 #include "services/ui/ws/window_server.h"
 #include "services/ui/ws/window_tree.h"
 #include "ui/events/event.h"
+#include "ui/gfx/geometry/dip_util.h"
 
 namespace ui {
 namespace ws {
@@ -273,10 +274,12 @@ bool WindowManagerState::IsActive() const {
   return window_server()->user_id_tracker()->active_id() == user_id();
 }
 
-void WindowManagerState::Activate(const gfx::Point& mouse_location_on_screen) {
+void WindowManagerState::Activate(const gfx::Point& mouse_location_on_display,
+                                  const int64_t display_id) {
   SetAllRootWindowsVisible(true);
   event_dispatcher_.Reset();
-  event_dispatcher_.SetMousePointerScreenLocation(mouse_location_on_screen);
+  event_dispatcher_.SetMousePointerDisplayLocation(mouse_location_on_display,
+                                                   display_id);
 }
 
 void WindowManagerState::Deactivate() {
@@ -321,9 +324,9 @@ void WindowManagerState::OnAcceleratorAck(
     DCHECK(details->event->IsKeyEvent());
     if (!properties.empty())
       details->event->AsKeyEvent()->SetProperties(properties);
-    event_processing_display_id_ = details->display_id;
     event_dispatcher_.ProcessEvent(
-        *details->event, EventDispatcher::AcceleratorMatchPhase::POST_ONLY);
+        *details->event, details->display_id,
+        EventDispatcher::AcceleratorMatchPhase::POST_ONLY);
   } else {
     // We're not going to process the event any further, notify event observers.
     // We don't do this first to ensure we don't send an event twice to clients.
@@ -395,8 +398,8 @@ void WindowManagerState::OnEventAck(mojom::WindowTree* tree,
 
   if (result == mojom::EventResult::UNHANDLED &&
       details->post_target_accelerator) {
-    OnAccelerator(details->post_target_accelerator->id(), *details->event,
-                  AcceleratorPhase::POST);
+    OnAccelerator(details->post_target_accelerator->id(), details->display_id,
+                  *details->event, AcceleratorPhase::POST);
   }
 
   ProcessNextEventFromQueue();
@@ -417,9 +420,8 @@ void WindowManagerState::OnEventAckTimeout(ClientSpecificId client_id) {
 void WindowManagerState::ProcessEventImpl(const ui::Event& event,
                                           int64_t display_id) {
   // Debug accelerators are always checked and don't interfere with processing.
-  ProcessDebugAccelerator(event);
-  event_processing_display_id_ = display_id;
-  event_dispatcher_.ProcessEvent(event,
+  ProcessDebugAccelerator(event, display_id);
+  event_dispatcher_.ProcessEvent(event, display_id,
                                  EventDispatcher::AcceleratorMatchPhase::ANY);
 }
 
@@ -447,8 +449,8 @@ void WindowManagerState::ProcessNextEventFromQueue() {
     if (queued_event->processed_target->IsValid()) {
       DispatchInputEventToWindowImpl(
           queued_event->processed_target->window(),
-          queued_event->processed_target->client_id(), *queued_event->event,
-          queued_event->processed_target->accelerator());
+          queued_event->processed_target->client_id(), queued_event->display_id,
+          *queued_event->event, queued_event->processed_target->accelerator());
       return;
     }
   }
@@ -457,6 +459,7 @@ void WindowManagerState::ProcessNextEventFromQueue() {
 void WindowManagerState::DispatchInputEventToWindowImpl(
     ServerWindow* target,
     ClientSpecificId client_id,
+    const int64_t display_id,
     const ui::Event& event,
     base::WeakPtr<Accelerator> accelerator) {
   DCHECK(target);
@@ -470,7 +473,8 @@ void WindowManagerState::DispatchInputEventToWindowImpl(
 
   WindowTree* tree = window_server()->GetTreeWithId(client_id);
   DCHECK(tree);
-  ScheduleInputEventTimeout(tree, target, event, EventDispatchPhase::TARGET);
+  ScheduleInputEventTimeout(tree, target, display_id, event,
+                            EventDispatchPhase::TARGET);
   in_flight_event_details_->post_target_accelerator = accelerator;
 
   // Ignore |tree| because it will receive the event via normal dispatch.
@@ -491,20 +495,22 @@ void WindowManagerState::AddDebugAccelerators() {
   debug_accelerators_.push_back(accelerator);
 }
 
-void WindowManagerState::ProcessDebugAccelerator(const ui::Event& event) {
+void WindowManagerState::ProcessDebugAccelerator(const ui::Event& event,
+                                                 const int64_t display_id) {
   if (event.type() != ui::ET_KEY_PRESSED)
     return;
 
   const ui::KeyEvent& key_event = *event.AsKeyEvent();
   for (const DebugAccelerator& accelerator : debug_accelerators_) {
     if (accelerator.Matches(key_event)) {
-      HandleDebugAccelerator(accelerator.type);
+      HandleDebugAccelerator(accelerator.type, display_id);
       break;
     }
   }
 }
 
-void WindowManagerState::HandleDebugAccelerator(DebugAcceleratorType type) {
+void WindowManagerState::HandleDebugAccelerator(DebugAcceleratorType type,
+                                                const int64_t display_id) {
 #if DCHECK_IS_ON()
   // Error so it will be collected in system logs.
   for (Display* display : display_manager()->displays()) {
@@ -515,7 +521,7 @@ void WindowManagerState::HandleDebugAccelerator(DebugAcceleratorType type) {
                  << display_root->root()->GetDebugWindowHierarchy();
     }
   }
-  ServerWindow* focused_window = GetFocusedWindowForEventDispatcher();
+  ServerWindow* focused_window = GetFocusedWindowForEventDispatcher(display_id);
   LOG(ERROR) << "Focused window: "
              << (focused_window ? focused_window->id().ToString() : "(null)");
 #endif
@@ -523,11 +529,12 @@ void WindowManagerState::HandleDebugAccelerator(DebugAcceleratorType type) {
 
 void WindowManagerState::ScheduleInputEventTimeout(WindowTree* tree,
                                                    ServerWindow* target,
+                                                   const int64_t display_id,
                                                    const Event& event,
                                                    EventDispatchPhase phase) {
   std::unique_ptr<InFlightEventDetails> details =
-      base::MakeUnique<InFlightEventDetails>(
-          this, tree, event_processing_display_id_, event, phase);
+      base::MakeUnique<InFlightEventDetails>(this, tree, display_id, event,
+                                             phase);
 
   // TOOD(sad): Adjust this delay, possibly make this dynamic.
   const base::TimeDelta max_delay = base::debug::BeingDebugged()
@@ -540,10 +547,24 @@ void WindowManagerState::ScheduleInputEventTimeout(WindowTree* tree,
   in_flight_event_details_ = std::move(details);
 }
 
+bool WindowManagerState::ConvertPointToScreen(gfx::Point* point,
+                                              const int64_t display_id) {
+  Display* display = display_manager()->GetDisplayById(display_id);
+  if (display) {

[sky, 2017/05/25 22:10:19]

When would there not be a display?

[riajiang, 2017/05/25 22:59:23]

This is called by WindowManagerState::GetRootWindowContaining which is part of
event-targeting. Since we are putting events in a queue if there's an event
currently being dispatched, by the time this event is being processed its
display might have already gone? In that case it would go to the if
(!target_display_root) handling block.

[sky, 2017/05/25 23:55:24]

If the display configuration changes in some way, then it's likely the events
applied to the new configuration don't make sense. I think if the display
configuration changes in anyway we should dump any existing events.

[riajiang, 2017/05/26 01:40:08]

I changed to return nullptr in GetRootWindowContaining if display config has
changed, then no target window is found and OnEventTargetNotFound
(https://cs.chromium.org/chromium/src/services/ui/ws/event_dispatcher_delegate...)
would be called to handle this later.

+    const display::Display& originated_display = display->GetDisplay();
+    *point = gfx::ConvertPointToDIP(originated_display.device_scale_factor(),
+                                    *point);
+    *point += originated_display.bounds().origin().OffsetFromOrigin();
+    return true;
+  }
+  return false;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // EventDispatcherDelegate:
 
 void WindowManagerState::OnAccelerator(uint32_t accelerator_id,
+                                       const int64_t display_id,
                                        const ui::Event& event,
                                        AcceleratorPhase phase) {
   DCHECK(IsActive());
@@ -551,7 +572,7 @@ void WindowManagerState::OnAccelerator(uint32_t accelerator_id,
   WindowTree::AcceleratorCallback ack_callback;
   if (needs_ack) {
     DCHECK(!in_flight_event_details_);
-    ScheduleInputEventTimeout(window_tree_, nullptr, event,
+    ScheduleInputEventTimeout(window_tree_, nullptr, display_id, event,
                               EventDispatchPhase::PRE_TARGET_ACCELERATOR);
     ack_callback =
         base::BindOnce(&WindowManagerState::OnAcceleratorAck,
@@ -566,14 +587,15 @@ void WindowManagerState::SetFocusedWindowFromEventDispatcher(
   window_server()->SetFocusedWindow(new_focused_window);
 }
 
-ServerWindow* WindowManagerState::GetFocusedWindowForEventDispatcher() {
+ServerWindow* WindowManagerState::GetFocusedWindowForEventDispatcher(
+    const int64_t display_id) {
   ServerWindow* focused_window = window_server()->GetFocusedWindow();
   if (focused_window)
     return focused_window;
 
   // When none of the windows have focus return the window manager's root.
   for (auto& display_root_ptr : window_manager_display_roots_) {
-    if (display_root_ptr->display()->GetId() == event_processing_display_id_)
+    if (display_root_ptr->display()->GetId() == display_id)
       return display_root_ptr->GetClientVisibleRoot();
   }
   if (!window_manager_display_roots_.empty())
@@ -611,15 +633,20 @@ void WindowManagerState::OnCaptureChanged(ServerWindow* new_capture,
   window_server()->ProcessCaptureChanged(new_capture, old_capture);
 }
 
-void WindowManagerState::OnMouseCursorLocationChanged(const gfx::Point& point) {
+void WindowManagerState::OnMouseCursorLocationChanged(
+    const gfx::Point& point_in_display,
+    const int64_t display_id) {
+  gfx::Point point_in_screen(point_in_display);
+  ConvertPointToScreen(&point_in_screen, display_id);
   window_server()
       ->display_manager()
       ->GetCursorLocationManager(user_id())
-      ->OnMouseCursorLocationChanged(point);
+      ->OnMouseCursorLocationChanged(point_in_screen);
 }
 
 void WindowManagerState::DispatchInputEventToWindow(ServerWindow* target,
                                                     ClientSpecificId client_id,
+                                                    const int64_t display_id,
                                                     const ui::Event& event,
                                                     Accelerator* accelerator) {
   DCHECK(IsActive());
@@ -628,15 +655,15 @@ void WindowManagerState::DispatchInputEventToWindow(ServerWindow* target,
   if (in_flight_event_details_) {
     std::unique_ptr<ProcessedEventTarget> processed_event_target(
         new ProcessedEventTarget(target, client_id, accelerator));
-    QueueEvent(event, std::move(processed_event_target),
-               event_processing_display_id_);
+    QueueEvent(event, std::move(processed_event_target), display_id);
     return;
   }
 
   base::WeakPtr<Accelerator> weak_accelerator;
   if (accelerator)
     weak_accelerator = accelerator->GetWeakPtr();
-  DispatchInputEventToWindowImpl(target, client_id, event, weak_accelerator);
+  DispatchInputEventToWindowImpl(target, client_id, display_id, event,
+                                 weak_accelerator);
 }
 
 ClientSpecificId WindowManagerState::GetEventTargetClientId(
@@ -668,40 +695,50 @@ ClientSpecificId WindowManagerState::GetEventTargetClientId(
 }
 
 ServerWindow* WindowManagerState::GetRootWindowContaining(
-    gfx::Point* location) {
+    gfx::Point* location_in_display,
+    int64_t* display_id) {
   if (window_manager_display_roots_.empty())
     return nullptr;
 
-  // TODO(riajiang): This is broken for HDPI because it mixes PPs and DIPs. See
-  // http://crbug.com/701036 for details.
   WindowManagerDisplayRoot* target_display_root = nullptr;
-  for (auto& display_root_ptr : window_manager_display_roots_) {
-    if (display_root_ptr->display()->GetDisplay().bounds().Contains(
-            *location)) {
-      target_display_root = display_root_ptr.get();
-      break;
+  gfx::Point location_in_screen(*location_in_display);
+  if (ConvertPointToScreen(&location_in_screen, *display_id)) {
+    for (auto& display_root_ptr : window_manager_display_roots_) {
+      if (display_root_ptr->display()->GetDisplay().bounds().Contains(
+              location_in_screen)) {
+        target_display_root = display_root_ptr.get();
+        break;
+      }
     }
   }
 
   // TODO(kylechar): Better handle locations outside the window. Overlapping X11
   // windows, dragging and touch sensors need to be handled properly.
   if (!target_display_root) {
-    DVLOG(1) << "Invalid event location " << location->ToString();
+    DVLOG(1) << "Invalid event location " << location_in_display->ToString()
+             << " / display id " << *display_id;
     target_display_root = window_manager_display_roots_.begin()->get();
   }
 
-  // Translate the location to be relative to the display instead of relative
-  // to the screen space.
-  gfx::Point origin =
-      target_display_root->display()->GetDisplay().bounds().origin();
-  *location -= origin.OffsetFromOrigin();
+  // Update |location_in_display| and |display_id| if the target display is
+  // different from the originated display, e.g. drag-and-drop.
+  if (*display_id != target_display_root->display()->GetId()) {
+    gfx::Point origin =
+        target_display_root->display()->GetDisplay().bounds().origin();
+    *location_in_display = location_in_screen - origin.OffsetFromOrigin();
+    *location_in_display = gfx::ConvertPointToPixel(
+        target_display_root->display()->GetDisplay().device_scale_factor(),
+        *location_in_display);
+    *display_id = target_display_root->display()->GetId();
+  }
+
   return target_display_root->GetClientVisibleRoot();
 }
 
-void WindowManagerState::OnEventTargetNotFound(const ui::Event& event) {
+void WindowManagerState::OnEventTargetNotFound(const ui::Event& event,
+                                               const int64_t display_id) {
   window_server()->SendToPointerWatchers(event, user_id(), nullptr, /* window */
-                                         nullptr /* ignore_tree */,
-                                         event_processing_display_id_);
+                                         nullptr /* ignore_tree */, display_id);
   if (event.IsMousePointerEvent())
     UpdateNativeCursorFromDispatcher();
 }