chromeos: converts observed pointer events to DIPs

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 d1725e8c549ddf59b5486f060418a3cc00949f7b..76c5684e6110498d51a7509a25e4321dfa7fd6cb 100644
--- a/services/ui/ws/window_manager_state.cc
+++ b/services/ui/ws/window_manager_state.cc
@@ -67,6 +67,18 @@ const ServerWindow* GetEmbedRoot(const ServerWindow* window) {
 
 }  // namespace
 
+WindowManagerState::InFlightEventDetails::InFlightEventDetails(
+    WindowTree* tree,
+    int64_t display_id,
+    const Event& event,
+    EventDispatchPhase phase)
+    : tree(tree),
+      display_id(display_id),
+      event(Event::Clone(event)),
+      phase(phase) {}
+
+WindowManagerState::InFlightEventDetails::~InFlightEventDetails() {}
+
 class WindowManagerState::ProcessedEventTarget {
  public:
   ProcessedEventTarget(ServerWindow* window,
@@ -169,9 +181,10 @@ void WindowManagerState::SetDragDropSourceWindow(
     const std::unordered_map<std::string, std::vector<uint8_t>>& drag_data,
     uint32_t drag_operation) {
   int32_t drag_pointer = PointerEvent::kMousePointerId;
-  if (event_awaiting_input_ack_ &&
-      event_awaiting_input_ack_->IsPointerEvent()) {
-    drag_pointer = event_awaiting_input_ack_->AsPointerEvent()->pointer_id();
+  if (in_flight_event_details_ &&
+      in_flight_event_details_->event->IsPointerEvent()) {
+    drag_pointer =
+        in_flight_event_details_->event->AsPointerEvent()->pointer_id();
   } else {
     NOTIMPLEMENTED() << "Set drag drop set up during something other than a "
                      << "pointer event; rejecting drag.";
@@ -205,15 +218,15 @@ const UserId& WindowManagerState::user_id() const {
 void WindowManagerState::OnWillDestroyTree(WindowTree* tree) {
   event_dispatcher_.OnWillDestroyDragTargetConnection(tree);
 
-  if (tree_awaiting_input_ack_ != tree)
+  if (!in_flight_event_details_ || in_flight_event_details_->tree != tree)
     return;
 
   // The WindowTree is dying. So it's not going to ack the event.
   // If the dying tree matches the root |tree_| marked as handled so we don't
   // notify it of accelerators.
-  OnEventAck(tree_awaiting_input_ack_, tree == window_tree_
-                                           ? mojom::EventResult::HANDLED
-                                           : mojom::EventResult::UNHANDLED);
+  OnEventAck(in_flight_event_details_->tree,
+             tree == window_tree_ ? mojom::EventResult::HANDLED
+                                  : mojom::EventResult::UNHANDLED);
 }
 
 ServerWindow* WindowManagerState::GetOrphanedRootWithId(const WindowId& id) {
@@ -243,66 +256,68 @@ void WindowManagerState::Deactivate() {
   event_queue.swap(event_queue_);
 }
 
-void WindowManagerState::ProcessEvent(const ui::Event& event) {
+void WindowManagerState::ProcessEvent(const ui::Event& event,
+                                      int64_t display_id) {
   // If this is still waiting for an ack from a previously sent event, then
   // queue up the event to be dispatched once the ack is received.
-  if (event_ack_timer_.IsRunning()) {
+  if (in_flight_event_details_) {
     if (!event_queue_.empty() && !event_queue_.back()->processed_target &&
         EventsCanBeCoalesced(*event_queue_.back()->event, event)) {
       event_queue_.back()->event = CoalesceEvents(
           std::move(event_queue_.back()->event), ui::Event::Clone(event));
+      event_queue_.back()->display_id = display_id;
       return;
     }
-    QueueEvent(event, nullptr);
+    QueueEvent(event, nullptr, display_id);
     return;
   }
 
-  ProcessEventImpl(event);
+  ProcessEventImpl(event, display_id);
 }
 
 void WindowManagerState::OnEventAck(mojom::WindowTree* tree,
                                     mojom::EventResult result) {
-  if (tree_awaiting_input_ack_ != tree ||
-      event_dispatch_phase_ != EventDispatchPhase::TARGET) {
+  if (!in_flight_event_details_ || in_flight_event_details_->tree != tree ||
+      in_flight_event_details_->phase != EventDispatchPhase::TARGET) {
     // TODO(sad): The ack must have arrived after the timeout. We should do
     // something here, and in OnEventAckTimeout().
     return;
   }
-  tree_awaiting_input_ack_ = nullptr;
-  event_ack_timer_.Stop();
+  std::unique_ptr<InFlightEventDetails> details =
+      std::move(in_flight_event_details_);
 
   base::WeakPtr<Accelerator> post_target_accelerator = post_target_accelerator_;
   post_target_accelerator_.reset();
-  std::unique_ptr<ui::Event> event = std::move(event_awaiting_input_ack_);
 
   if (result == mojom::EventResult::UNHANDLED && post_target_accelerator) {
-    OnAccelerator(post_target_accelerator->id(), *event,
+    OnAccelerator(post_target_accelerator->id(), *details->event,
                   AcceleratorPhase::POST);
   }
 
-  event_dispatch_phase_ = EventDispatchPhase::NONE;
   ProcessNextEventFromQueue();
 }
 
 void WindowManagerState::OnAcceleratorAck(mojom::EventResult result) {
-  if (event_dispatch_phase_ != EventDispatchPhase::PRE_TARGET_ACCELERATOR) {
+  if (!in_flight_event_details_ ||
+      in_flight_event_details_->phase !=
+          EventDispatchPhase::PRE_TARGET_ACCELERATOR) {
     // TODO(sad): The ack must have arrived after the timeout. We should do
     // something here, and in OnEventAckTimeout().
     return;
   }
 
-  tree_awaiting_input_ack_ = nullptr;
-  event_ack_timer_.Stop();
-  event_dispatch_phase_ = EventDispatchPhase::NONE;
-  std::unique_ptr<ui::Event> event = std::move(event_awaiting_input_ack_);
+  std::unique_ptr<InFlightEventDetails> details =
+      std::move(in_flight_event_details_);
 
   if (result == mojom::EventResult::UNHANDLED) {
+    event_processing_display_id_ = details->display_id;
     event_dispatcher_.ProcessEvent(
-        *event, EventDispatcher::AcceleratorMatchPhase::POST_ONLY);
+        *details->event, 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.
-    window_server()->SendToPointerWatchers(*event, user_id(), nullptr, nullptr);
+    window_server()->SendToPointerWatchers(*details->event, user_id(), nullptr,
+                                           nullptr, details->display_id);
     ProcessNextEventFromQueue();
   }
 }
@@ -363,25 +378,33 @@ void WindowManagerState::OnEventAckTimeout(ClientSpecificId client_id) {
   WindowTree* hung_tree = window_server()->GetTreeWithId(client_id);
   if (hung_tree && !hung_tree->janky())
     window_tree_->ClientJankinessChanged(hung_tree);
-  if (event_dispatch_phase_ == EventDispatchPhase::PRE_TARGET_ACCELERATOR)
+  if (in_flight_event_details_->phase ==
+      EventDispatchPhase::PRE_TARGET_ACCELERATOR) {
     OnAcceleratorAck(mojom::EventResult::UNHANDLED);
-  else
-    OnEventAck(tree_awaiting_input_ack_, mojom::EventResult::UNHANDLED);
+  } else {
+    OnEventAck(
+        in_flight_event_details_ ? in_flight_event_details_->tree : nullptr,
+        mojom::EventResult::UNHANDLED);
+  }
 }
 
-void WindowManagerState::ProcessEventImpl(const ui::Event& event) {
+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,
                                  EventDispatcher::AcceleratorMatchPhase::ANY);

[sadrul, 2017/02/09 02:27:45]

Should we carry |display_id| through into EventDispatcher?

[sky, 2017/02/09 17:06:20]

I was tempted to, but then EventDispatcher doesn't need it for anything and
EventDispatcher is entirely driven by WindowManagerState, so I didn't see a
reason to.

 }
 
 void WindowManagerState::QueueEvent(
     const ui::Event& event,
-    std::unique_ptr<ProcessedEventTarget> processed_event_target) {
+    std::unique_ptr<ProcessedEventTarget> processed_event_target,
+    int64_t display_id) {
   std::unique_ptr<QueuedEvent> queued_event(new QueuedEvent);
   queued_event->event = ui::Event::Clone(event);
   queued_event->processed_target = std::move(processed_event_target);
+  queued_event->display_id = display_id;
   event_queue_.push(std::move(queued_event));
 }
 
@@ -392,7 +415,7 @@ void WindowManagerState::ProcessNextEventFromQueue() {
     std::unique_ptr<QueuedEvent> queued_event = std::move(event_queue_.front());
     event_queue_.pop();
     if (!queued_event->processed_target) {
-      ProcessEventImpl(*queued_event->event);
+      ProcessEventImpl(*queued_event->event, queued_event->display_id);
       return;
     }
     if (queued_event->processed_target->IsValid()) {
@@ -410,7 +433,8 @@ void WindowManagerState::DispatchInputEventToWindowImpl(
     ClientSpecificId client_id,
     const ui::Event& event,
     base::WeakPtr<Accelerator> accelerator) {
-  if (target && target->parent() == nullptr)
+  DCHECK(target);
+  if (target->parent() == nullptr)
     target = GetWindowManagerRoot(target);
 
   if (event.IsMousePointerEvent()) {
@@ -418,19 +442,16 @@ void WindowManagerState::DispatchInputEventToWindowImpl(
     UpdateNativeCursorFromDispatcher();
   }
 
-  event_dispatch_phase_ = EventDispatchPhase::TARGET;
-
   WindowTree* tree = window_server()->GetTreeWithId(client_id);
   DCHECK(tree);
-  ScheduleInputEventTimeout(tree);
-
-  event_awaiting_input_ack_ = ui::Event::Clone(event);
+  ScheduleInputEventTimeout(tree, target, event, EventDispatchPhase::TARGET);
 
   if (accelerator)
     post_target_accelerator_ = accelerator;
 
   // Ignore |tree| because it will receive the event via normal dispatch.
-  window_server()->SendToPointerWatchers(event, user_id(), target, tree);
+  window_server()->SendToPointerWatchers(event, user_id(), target, tree,
+                                         in_flight_event_details_->display_id);
 
   tree->DispatchInputEvent(target, event);
 }
@@ -469,16 +490,22 @@ void WindowManagerState::HandleDebugAccelerator(DebugAcceleratorType type) {
 #endif
 }
 
-void WindowManagerState::ScheduleInputEventTimeout(WindowTree* tree) {
+void WindowManagerState::ScheduleInputEventTimeout(WindowTree* tree,
+                                                   ServerWindow* target,
+                                                   const Event& event,
+                                                   EventDispatchPhase phase) {
+  std::unique_ptr<InFlightEventDetails> details =
+      base::MakeUnique<InFlightEventDetails>(tree, event_processing_display_id_,
+                                             event, phase);
+
   // TOOD(sad): Adjust this delay, possibly make this dynamic.
   const base::TimeDelta max_delay = base::debug::BeingDebugged()
                                         ? base::TimeDelta::FromDays(1)
                                         : GetDefaultAckTimerDelay();
-  event_ack_timer_.Start(FROM_HERE, max_delay,
-                         base::Bind(&WindowManagerState::OnEventAckTimeout,
-                                    weak_factory_.GetWeakPtr(), tree->id()));
-
-  tree_awaiting_input_ack_ = tree;
+  details->timer.Start(FROM_HERE, max_delay,
+                       base::Bind(&WindowManagerState::OnEventAckTimeout,
+                                  weak_factory_.GetWeakPtr(), tree->id()));
+  in_flight_event_details_ = std::move(details);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -490,10 +517,9 @@ void WindowManagerState::OnAccelerator(uint32_t accelerator_id,
   DCHECK(IsActive());
   const bool needs_ack = phase == AcceleratorPhase::PRE;
   if (needs_ack) {
-    DCHECK_EQ(EventDispatchPhase::NONE, event_dispatch_phase_);
-    event_dispatch_phase_ = EventDispatchPhase::PRE_TARGET_ACCELERATOR;
-    event_awaiting_input_ack_ = ui::Event::Clone(event);
-    ScheduleInputEventTimeout(window_tree_);
+    DCHECK(!in_flight_event_details_);
+    ScheduleInputEventTimeout(window_tree_, nullptr, event,
+                              EventDispatchPhase::PRE_TARGET_ACCELERATOR);
   }
   window_tree_->OnAccelerator(accelerator_id, event, needs_ack);
 }
@@ -553,10 +579,11 @@ void WindowManagerState::DispatchInputEventToWindow(ServerWindow* target,
   DCHECK(IsActive());
   // TODO(sky): this needs to see if another wms has capture and if so forward
   // to it.
-  if (event_ack_timer_.IsRunning()) {
+  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));
+    QueueEvent(event, std::move(processed_event_target),
+               event_processing_display_id_);

[sadrul, 2017/02/09 02:27:45]

Is |event_processing_display_id_| always going to be the correct display-id? For
example, if EventDispatcher is generating a pointer-exit event, that could be
going to a window that lives in a different display. Should we get the
display-id from |target| instead?

[sky, 2017/02/09 17:06:20]

With this patch all non-pointer-watcher events get the display id of the display
the window is on (see line 1153 in services/ui/ws/window_tree.cc). That leaves
pointer-watcher events. With this patch pointer-watcher events get the display
id of the display the event is generated on. This seems right given the client
uses that display to convert pixels to dips. That said, I don't know the state
of how we position multiple displays with differing densities. I believe
currently the positioning is done at the DIP level and I don't know if care is
taken to make sure the pixel boundaries line up nicely.

     return;
   }
 
@@ -625,7 +652,8 @@ ServerWindow* WindowManagerState::GetRootWindowContaining(
 
 void WindowManagerState::OnEventTargetNotFound(const ui::Event& event) {
   window_server()->SendToPointerWatchers(event, user_id(), nullptr, /* window */
-                                         nullptr /* ignore_tree */);
+                                         nullptr /* ignore_tree */,
+                                         event_processing_display_id_);

[sadrul, 2017/02/09 02:27:45]

Is this going to be correct? I assume an |event| that didn't find a target will
be in the coord-space of the screen. In that case, specifying a display-id would
probably be confusing? Perhaps just send -1 as the invalid/unknown display-id?

[sky, 2017/02/09 17:06:20]

See comment above. I believe this is right.

   if (event.IsMousePointerEvent())
     UpdateNativeCursorFromDispatcher();
 }