Make event-targeting asynchronous in window server.

services/ui/ws/event_dispatcher.cc

Index: services/ui/ws/event_dispatcher.cc
diff --git a/services/ui/ws/event_dispatcher.cc b/services/ui/ws/event_dispatcher.cc
index c6e6897508347096389172210d6d3e9b91594479..bb2395e180c023b19406f1d781835c771870007b 100644
--- a/services/ui/ws/event_dispatcher.cc
+++ b/services/ui/ws/event_dispatcher.cc
@@ -224,30 +224,21 @@ const ServerWindow* EventDispatcher::GetWindowForMouseCursor() const {
 
 void EventDispatcher::UpdateNonClientAreaForCurrentWindow() {
   if (mouse_cursor_source_window_) {
-    DeepestWindow deepest_window =
-        event_targeter_->FindDeepestVisibleWindowForEvents(
-            &mouse_pointer_last_location_, &mouse_pointer_display_id_);
-    if (deepest_window.window == mouse_cursor_source_window_) {
-      mouse_cursor_in_non_client_area_ = mouse_cursor_source_window_
-                                             ? deepest_window.in_non_client_area
-                                             : false;
-    }
+    event_targeter_->FindDeepestVisibleWindowForEvents(
+        &mouse_pointer_last_location_, &mouse_pointer_display_id_,
+        base::BindOnce(
+            &EventDispatcher::UpdateNonClientAreaForCurrentWindowOnFoundWindow,
+            base::Unretained(this)));
   }
 }
 
 void EventDispatcher::UpdateCursorProviderByLastKnownLocation() {

[sky, 2017/05/30 17:26:02]

This class is mostly written assuming synchronous processing. If we're going to
introduce async processing I think we need to better restructure it. For
example, this function may be called after you've gotten the result from
ProcessPointerEventOnFoundTarget. We should ensure there is at most one async
lookup when processing an event. Additionally for some event processing we do
not need to lookup the target at all, the way you have it now assumes we do.

I think the code should be structured to only do lookup if necessary, and if
lookup is needed, we ensure it is done only once.

[riajiang, 2017/05/31 22:44:03]

True! I didn't notice that UpdateCursorProviderByLastKnownLocation is called
right after we found the new target in ProcessPointerEventOnFoundTarget. For
now, I changed it to be still always look up the target when processing event
but use the target we found for
UpdateCursorProviderByLastKnownLocationWithWindow so we only do async targeting
once for an event. 
As Sadrul mentioned in a previous comment, after the HittestComponent is done we
are going to look into caching to see if targeting is necessary for both event
processing and window hierarchy/bounds changes (when
UpdateCursorProviderByLastKnownLocation would be called). WDYT?

   if (!mouse_button_down_) {
-    DeepestWindow deepest_window =
-        event_targeter_->FindDeepestVisibleWindowForEvents(
-            &mouse_pointer_last_location_, &mouse_pointer_display_id_);
-    SetMouseCursorSourceWindow(deepest_window.window);
-    if (mouse_cursor_source_window_) {
-      mouse_cursor_in_non_client_area_ = deepest_window.in_non_client_area;
-    } else {
-      SetMouseCursorSourceWindow(delegate_->GetRootWindowContaining(
-          &mouse_pointer_last_location_, &mouse_pointer_display_id_));
-      mouse_cursor_in_non_client_area_ = true;
-    }
+    event_targeter_->FindDeepestVisibleWindowForEvents(
+        &mouse_pointer_last_location_, &mouse_pointer_display_id_,
+        base::BindOnce(&EventDispatcher::
+                           UpdateCursorProviderByLastKnownLocationOnFoundWindow,
+                       base::Unretained(this)));
   }
 }
 
@@ -356,6 +347,17 @@ void EventDispatcher::ProcessKeyEvent(const ui::KeyEvent& event,
 
 void EventDispatcher::ProcessPointerEvent(const ui::PointerEvent& event) {
   DCHECK(event.IsPointerEvent());
+  event_targeter_->PointerTargetForEvent(
+      event, &event_display_id_,
+      base::BindOnce(&EventDispatcher::ProcessPointerEventOnFoundTarget,
+                     base::Unretained(this), event));
+}
+
+void EventDispatcher::ProcessPointerEventOnFoundTarget(
+    const ui::PointerEvent& event,
+    PointerTarget pointer_target_found) {
+  event_targeter_->ProcessNextHittesetRequestFromQueue();
+
   const bool is_mouse_event = event.IsMousePointerEvent();
 
   if (is_mouse_event) {
@@ -381,9 +383,8 @@ void EventDispatcher::ProcessPointerEvent(const ui::PointerEvent& event) {
   }
 
   if (drag_controller_) {
-    const PointerTarget target =
-        event_targeter_->PointerTargetForEvent(event, &event_display_id_);
-    if (drag_controller_->DispatchPointerEvent(event, target.window))
+    if (drag_controller_->DispatchPointerEvent(event,
+                                               pointer_target_found.window))
       return;
   }
 
@@ -397,7 +398,7 @@ void EventDispatcher::ProcessPointerEvent(const ui::PointerEvent& event) {
   if (!IsTrackingPointer(pointer_id) ||
       !pointer_targets_[pointer_id].is_pointer_down) {
     const bool any_pointers_down = AreAnyPointersDown();
-    UpdateTargetForPointer(pointer_id, event);
+    UpdateTargetForPointer(pointer_id, event, pointer_target_found);
     if (is_mouse_event)
       SetMouseCursorSourceWindow(pointer_targets_[pointer_id].window);
 
@@ -457,22 +458,21 @@ void EventDispatcher::StopTrackingPointer(int32_t pointer_id) {
     UnobserveWindow(window);
 }
 
-void EventDispatcher::UpdateTargetForPointer(int32_t pointer_id,
-                                             const ui::LocatedEvent& event) {
+void EventDispatcher::UpdateTargetForPointer(
+    int32_t pointer_id,
+    const ui::PointerEvent& event,
+    const PointerTarget& pointer_target_found) {
   if (!IsTrackingPointer(pointer_id)) {
-    StartTrackingPointer(pointer_id, event_targeter_->PointerTargetForEvent(
-                                         event, &event_display_id_));
+    StartTrackingPointer(pointer_id, pointer_target_found);
     return;
   }
 
-  const PointerTarget pointer_target =
-      event_targeter_->PointerTargetForEvent(event, &event_display_id_);
-  if (pointer_target.window == pointer_targets_[pointer_id].window &&
-      pointer_target.in_nonclient_area ==
+  if (pointer_target_found.window == pointer_targets_[pointer_id].window &&
+      pointer_target_found.in_nonclient_area ==
           pointer_targets_[pointer_id].in_nonclient_area) {
     // The targets are the same, only set the down state to true if necessary.
     // Down going to up is handled by ProcessLocatedEvent().
-    if (pointer_target.is_pointer_down)
+    if (pointer_target_found.is_pointer_down)
       pointer_targets_[pointer_id].is_pointer_down = true;
     return;
   }
@@ -491,7 +491,33 @@ void EventDispatcher::UpdateTargetForPointer(int32_t pointer_id,
   // Technically we're updating in place, but calling start then stop makes for
   // simpler code.
   StopTrackingPointer(pointer_id);
-  StartTrackingPointer(pointer_id, pointer_target);
+  StartTrackingPointer(pointer_id, pointer_target_found);
+}
+
+void EventDispatcher::UpdateNonClientAreaForCurrentWindowOnFoundWindow(
+    DeepestWindow deepest_window) {
+  event_targeter_->ProcessNextHittesetRequestFromQueue();
+
+  if (deepest_window.window == mouse_cursor_source_window_) {
+    mouse_cursor_in_non_client_area_ =
+        mouse_cursor_source_window_ ? deepest_window.in_non_client_area : false;
+  }
+  delegate_->UpdateNativeCursorFromDispatcher();
+}
+
+void EventDispatcher::UpdateCursorProviderByLastKnownLocationOnFoundWindow(
+    DeepestWindow deepest_window) {
+  event_targeter_->ProcessNextHittesetRequestFromQueue();
+
+  SetMouseCursorSourceWindow(deepest_window.window);
+  if (mouse_cursor_source_window_) {
+    mouse_cursor_in_non_client_area_ = deepest_window.in_non_client_area;
+  } else {
+    SetMouseCursorSourceWindow(delegate_->GetRootWindowContaining(
+        &mouse_pointer_last_location_, &mouse_pointer_display_id_));
+    mouse_cursor_in_non_client_area_ = true;
+  }
+  delegate_->UpdateNativeCursorFromDispatcher();
 }
 
 bool EventDispatcher::AreAnyPointersDown() const {