So it seems to me there are (at least) a couple of ways to attack this. I suspect Option#1 is simpler/faster to implement.

I'd appreciate any feedback about the relative merits of the approaches, and other ideas that people might have.

Option #1 (least invasive change):

- institute a touch-ack queue in RenderWidgetHostInputEventRouter
  - the queue keeps track of the target view for each unique_touch_event_id
- nothing is acked to the GestureProvider without checking it off, in order, in this queue (so all acks go through RWHIER)
  - in the case where one touch sequence goes to an unresponsive frame, and we start receiving acks from a subsequent touch sequence, we store these acks in the queue but don't immediately forward them to the GestureProvider, as they would confuse it
    + we will wait some maximum amount of time for the unresponsive renderer, but after that we'll send forced acks for everything in the unresponsive sequence (unconsumed? no_consumer?), and then we can process acks that have built up from subsequent sequences from other views
    + if we later receive acks from the unresponsive frame, we will discard them
  - if a renderer goes away without acking touch events, we can detect this and send the necessary acks to the gesture provider (no_consumer in this case)
  - they can be flagged while still in the queue if it's not their turn to be acked yet

This option then mostly involves a queue that keeps track of the various outbound touch events and their targets, and what the most recently seen touch_id is. The 'timeout'mechanism for unresponsive events is perhaps the hardest part of this.

Option#2 (more plumbing, less bookeeping):

- assign a GestureProvider to each cross-process frame
  - then RenderWidgetHostView{Aura|Mac|ChildFrame|etc} always sends acks to its own GestureProvider, and they're always in order for that GestureProvider
  - RenderWidgetHostViewChildFrame, in particular, would no longer blindly forward touch event acks to the root view

- We would need to be careful to correctly wire the call to GestureProvider::OnTouchEvent() to happen *after* the event has been routed so it goes to the correct GestureProvider
(https://cs.chromium.org/chromium/src/ui/events/gesture_detection/filtered_gesture_provider.cc?rcl=e75e328a026878f90373d4055f8a582da0f9e7e0&l=28)
  - at present, this happens under the assumption of a single gesture provider attached to the root view
 - this approach may also involve other event types that the GestureProvider is interested in

Note that this ordering issue will also apply to touchpad pinch. The concept behind option 1 can be easily extended to touchpad pinch, but not option 2.

Also, option 2 seems like it's deferring the reordering issue to the generated gesture events. If a GestureProvider in the root produces a pinch gesture from touches in the root and a GestureProvider in the child produces a pinch gesture from touches in the child, we'll have to combine these into a valid event sequence.

For option 1, is the timeout for acks from a child necessary? Unlike the case of the renderer going away, the timeout seems like an optimization. If the child renderer is slow to ack, it would have presumably been slow to ack in the single renderer case as well.

Also, what if the slow child does eventually preventDefault to implement some custom behaviour? Perhaps having a timeout where the slow acks are considered consumed would allow us to unblock the queue while still respecting preventDefault.

Pass through touch event queue already preserves touch events ack order. I guess the problem is that it per render widget. We could also be using callbacks to the gesture controller. Not sure if that would help. But old chrome ipc is gone so now we can bind what ever data we want with a call back.  

Thanks for the feedback.

I guess in option 2 I was assuming that things like touch-sequences would generate gestures that only go back to the same renderer, so overlap isn't an issue, *except* in the case of pinch, though I'm sure that can be managed. But as mentioned earlier, this is part of of what I suspect makes the Option #2 more complicated.

Regarding the timeout for acks, I partly included that because I know kenrb@ already has something like that in the targetting logic, and there are obvious advantages to not allowing un unresponsive child frame to impede performance on the main frame (or other frames for that matter). So yes, it may be an optimization, but I suspect it's a worthwhile one.

In your last example, I'm not entirely sure how that would play out, but at some point it's perhaps fair to 'give up' on an unresponsive page. Definitely though we want to unblock the queue without undue delay.

So far option #1 seems preferable, even if there are some details to figure out.

Re C#5, yes, the issue is that current ordering guarantees are per render widget.

Can you elaborate on the callbacks? Presumably they would have the same issues around asynchrony, so the Gesture controller would have to manage that.

Which leads me to an option #3 of sorts, in which the GestureProvider/Recognizer internally keeps separate state for each sequence. Then as touches are acked, the gesture provider can handle them so long as they're in-order per sequence. But I know less about the architecture of the GestureProvider than I do about RWHIER, so my ideas have been more focused on the latter.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/926835bcfa1e7a584475f3e2f4bff8cb089140cc

commit 926835bcfa1e7a584475f3e2f4bff8cb089140cc
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Wed Aug 22 18:33:14 2018

Refactor touch event acks to go through RenderWidgetHostInputEventRouter.

This is a refactoring CL to pave the way for a TouchEventAckQueue
implementation that will allow us to ensure the ordering of acks
when multiple renderers are present, i.e. OOPIF subframes exist.

Bug:  848050 
Change-Id: Ib446486ac6521c66cbfe3f552355e597107070db
Reviewed-on: https://chromium-review.googlesource.com/1181765
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585178}
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/frame_host/cross_process_frame_connector.h
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/renderer_host/frame_connector_delegate.h
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/926835bcfa1e7a584475f3e2f4bff8cb089140cc/content/browser/renderer_host/render_widget_host_view_child_frame.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/626dce8e9568cf276416d0fcd1216abb1ca5d5eb

commit 626dce8e9568cf276416d0fcd1216abb1ca5d5eb
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Wed Sep 12 16:01:22 2018

Implement TouchEventAckQueue in RenderWidgetHostInputEventRouter.

This CL introduces a queue to track outgoing TouchEvents, and
track when they are acked. At present that's all it does, but in a
subsequent CL this queue will be used by RWHIER for invoking
ProcessAckedTouchEvent() on the root RenderWidgetHostView, and
making sure the TouchEvents are acked in-order with respect to their
unique_touch_event_id values.

This CL also introduces UMA metrics to track renderer performance
by noting when the queue length grows beyond a preset size. In future
metrics will be added to detect (i) acking touch events due to a
renderer going away, and (ii) when acks are forced due to
non-responsiveness of a nominally 'alive' renderer.

Bug:  848050 
Change-Id: I976e41082f3859d77184ce7371ff7c7dba265782
Reviewed-on: https://chromium-review.googlesource.com/1188436
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590705}
[modify] https://crrev.com/626dce8e9568cf276416d0fcd1216abb1ca5d5eb/content/browser/renderer_host/input/compositor_event_ack_browsertest.cc
[modify] https://crrev.com/626dce8e9568cf276416d0fcd1216abb1ca5d5eb/content/browser/renderer_host/input/main_thread_event_queue_browsertest.cc
[modify] https://crrev.com/626dce8e9568cf276416d0fcd1216abb1ca5d5eb/content/browser/renderer_host/input/touch_input_browsertest.cc
[modify] https://crrev.com/626dce8e9568cf276416d0fcd1216abb1ca5d5eb/content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc
[modify] https://crrev.com/626dce8e9568cf276416d0fcd1216abb1ca5d5eb/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/626dce8e9568cf276416d0fcd1216abb1ca5d5eb/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/626dce8e9568cf276416d0fcd1216abb1ca5d5eb/tools/metrics/histograms/histograms.xml

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad0bd35f93a41cc30cd7741ea842cf84c7e39821

commit ad0bd35f93a41cc30cd7741ea842cf84c7e39821
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Wed Dec 12 22:10:05 2018

Shift TouchEventAcking to TEAQ

This CL makes TouchEventAckQueue responsible for calling
ProcessAckedTouchEvents() on the root view. Several main changes:

1) The TEAQ now stores the full TouchEvent, not just the event's
touch ID. This is required by ProcessAckedTouchEvents().

2) With the assumed invariant that now all touch events are
acked, and in-order, we don't need to explicitly mark emulated
touch events as such, since we can use the TouchEmulator to keep
track for us.

3) The TEAQ now holds a pointer to its owning RWHIER, since it
needs access to both (i) the TouchEmulator, and (ii) the function
IsViewInMap(). The latter means having to make IsViewInMap() a
public function on RWHIER.

Bug:  848050 
Change-Id: I822424ff1d04b8ba0c67a88956b1517c38a877ec
Reviewed-on: https://chromium-review.googlesource.com/c/1355243
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616071}
[modify] https://crrev.com/ad0bd35f93a41cc30cd7741ea842cf84c7e39821/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/ad0bd35f93a41cc30cd7741ea842cf84c7e39821/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/ad0bd35f93a41cc30cd7741ea842cf84c7e39821/content/browser/site_per_process_browsertest.cc