Make event-targeting asynchronous in window server.

services/ui/ws/event_dispatcher.h

Index: services/ui/ws/event_dispatcher.h
diff --git a/services/ui/ws/event_dispatcher.h b/services/ui/ws/event_dispatcher.h
index 8b084d7994f697a555d078ac1023b4dc5336f825..e921503a41f516d7394fb171491a7e7f1ade2079 100644
--- a/services/ui/ws/event_dispatcher.h
+++ b/services/ui/ws/event_dispatcher.h
@@ -12,6 +12,7 @@
 #include <utility>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "services/ui/common/types.h"
 #include "services/ui/public/interfaces/cursor/cursor.mojom.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
@@ -144,6 +145,8 @@ class EventDispatcher : public ServerWindowObserver, public DragCursorUpdater {
   // handled this is again called with an AcceleratorMatchPhase of POST_ONLY.
   void ProcessEvent(const ui::Event& event, AcceleratorMatchPhase match_phase);

[sky, 2017/05/22 16:06:56]

Update the description.

[riajiang, 2017/05/29 23:38:07]

Added that it may be asynchronous. 

 
+  bool IsHitTestInFlight() const;
+
  private:
   friend class test::EventDispatcherTestApi;
 
@@ -168,6 +171,18 @@ class EventDispatcher : public ServerWindowObserver, public DragCursorUpdater {
     bool is_pointer_down;
   };
 
+  using PointerTargetForEventCallback = base::OnceCallback<void(PointerTarget)>;
+  using HitTestCallback = base::OnceCallback<void(DeepestWindow)>;
+
+  struct HitTestRequest {
+    HitTestRequest(const gfx::Point& location,
+                   HitTestCallback hittest_callback);
+    ~HitTestRequest();
+
+    gfx::Point hittest_location;
+    HitTestCallback hittest_callback;
+  };
+
   void SetMouseCursorSourceWindow(ServerWindow* window);
 
   void ProcessKeyEvent(const ui::KeyEvent& event,
@@ -187,6 +202,8 @@ class EventDispatcher : public ServerWindowObserver, public DragCursorUpdater {
   // This also generates exit events as appropriate. For example, if the mouse
   // moves between one window to another an exit is generated on the first.
   void ProcessPointerEvent(const ui::PointerEvent& event);
+  void ProcessPointerEventOnFoundTarget(const ui::PointerEvent& event,
+                                        PointerTarget pointer_target_found);
 
   // Adds |pointer_target| to |pointer_targets_|.
   void StartTrackingPointer(int32_t pointer_id,
@@ -199,12 +216,18 @@ class EventDispatcher : public ServerWindowObserver, public DragCursorUpdater {
   // pointer sends the appropriate event to the delegate and updates the
   // currently tracked PointerTarget appropriately.
   void UpdateTargetForPointer(int32_t pointer_id,
-                              const ui::LocatedEvent& event);
-
-  // Returns a PointerTarget for the supplied event. If there is no valid
-  // event target for the specified location |window| in the returned value is
-  // null.
-  PointerTarget PointerTargetForEvent(const ui::LocatedEvent& event);
+                              const ui::PointerEvent& event,
+                              const PointerTarget& pointer_target_found);
+
+  // |callback| is called with a PointerTarget for the supplied event. If there
+  // is no valid event target for the specified location |window| in the
+  // returned value is null.
+  void PointerTargetForEvent(const ui::PointerEvent& event,
+                             PointerTargetForEventCallback callback);
+  void PointerTargetForEventOnFoundWindow(
+      const ui::PointerEvent& event,
+      PointerTargetForEventCallback callback,
+      DeepestWindow deepest_window);
 
   // Returns true if any pointers are in the pressed/down state.
   bool AreAnyPointersDown() const;
@@ -234,7 +257,21 @@ class EventDispatcher : public ServerWindowObserver, public DragCursorUpdater {
   Accelerator* FindAccelerator(const ui::KeyEvent& event,
                                const ui::mojom::AcceleratorPhase phase);
 
-  DeepestWindow FindDeepestVisibleWindowForEvents(const gfx::Point& location);
+  // Call WindowFinder to find the deepest visible window for |location|.
+  // |callback| is called with the window found.
+  void FindDeepestVisibleWindowForEvents(const gfx::Point& location,
+                                         HitTestCallback callback);

[sky, 2017/05/22 16:06:56]

I understand you're doing this as an initial step in the direction of adding
async processing, but the heavy use of callbacks here makes this hard to follow.
Won't we end up with a real class that likely has a delegate? Would it better to
structure the code like that now? For example, I'm assuming the real hit test
class is likely to coalesce multiple requests in some cases, or possibly end up
synchronously processing. It's hard to see that now.

I recommend adding a real class now for the hit-testing that has a delegate. You
can make this class pure virtual if you want with two implementations. One that
processes everything async, then other sync. Does that make sense?

[riajiang, 2017/05/29 23:38:07]

As discussed, changed to use the new EventTargeter class (introduced in
https://codereview.chromium.org/2905333002) to make the code a bit cleaner.

+  void FindDeepestVisibleWindowForEventsImpl(const gfx::Point& location,
+                                             HitTestCallback callback);
+  void FindDeepestVisibleWindowForEventsAsync(const gfx::Point& location,
+                                              HitTestCallback callback);
+
+  void UpdateNonClientAreaForCurrentWindowOnFoundWindow(
+      DeepestWindow deepest_window);
+  void UpdateCursorProviderByLastKnownLocationOnFoundWindow(
+      DeepestWindow deepest_window);
+
+  void ProcessNextHittesetRequestFromQueue();
 
   // Clears the implicit captures in |pointer_targets_|, with the exception of
   // |window|. |window| may be null. |client_id| is the target client of
@@ -271,6 +308,16 @@ class EventDispatcher : public ServerWindowObserver, public DragCursorUpdater {
 
   std::map<uint32_t, std::unique_ptr<Accelerator>> accelerators_;
 
+  // True if we are waiting for the result of a hit-test. False otherwise.
+  bool hittest_in_flight_;
+
+  // Request would go into this queue if it's coming from EventDispatcher::
+  // UpdateNonClientAreaForCurrentWindow() or EventDispatcher::
+  // UpdateCursorProviderByLastKnownLocation(). If it's an event that needs
+  // targeting, it would only go to WindowManagerState::event_queue_ when it's
+  // waiting for an active hit-test.
+  std::queue<std::unique_ptr<HitTestRequest>> hittest_request_queue_;
+
   using PointerIdToTargetMap = std::map<int32_t, PointerTarget>;
   // |pointer_targets_| contains the active pointers. For a mouse based pointer
   // a PointerTarget is always active (and present in |pointer_targets_|). For
@@ -287,6 +334,8 @@ class EventDispatcher : public ServerWindowObserver, public DragCursorUpdater {
       AcceleratorMatchPhase::ANY;
 #endif
 
+  base::WeakPtrFactory<EventDispatcher> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(EventDispatcher);
 };