The site I used is: slashdot.org and attempted to scroll on the ads on the right-edge of the page.

+input-dev@ folks

sahel@ can you debug this?
sadrul@ is this ToT.

It may be related to latching & async events.

This is on ToT, when running with --site-per-process

mcnee@ - Could you take a look please?

Scroll latching was enabled by default on Friday. Presumably related? It also might be a combination of that and async event targeting.

Another crash I get doing the same thing (i.e. mouse-wheeling on the ads on the right-side of slashdot.org):

[48878:48878:0115/140632.812510:FATAL:render_widget_host_impl.cc(1183)] Check failed: !is_in_gesture_scroll_[gesture_event.source_device].
    #0 0x55cc4df10261 in __interceptor_backtrace /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:3867:13
    #1 0x7fa49faaa1ac in base::debug::StackTrace::StackTrace(unsigned long) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/debug/stack_trace_posix.cc:808:41
    #2 0x7fa49fb30269 in logging::LogMessage::~LogMessage() /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/logging.cc:581:29
    #3 0x7fa4961ce73d in content::RenderWidgetHostImpl::ForwardGestureEventWithLatencyInfo(blink::WebGestureEvent const&, ui::LatencyInfo const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/renderer_host/render_widget_host_impl.cc:1183:5
    #4 0x7fa4961f8f46 in content::RenderWidgetHostInputEventRouter::BubbleScrollEvent(content::RenderWidgetHostViewBase*, blink::WebGestureEvent const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/renderer_host/render_widget_host_input_event_router.cc:765:45
    #5 0x7fa49590b3f3 in content::CrossProcessFrameConnector::BubbleScrollEvent(blink::WebGestureEvent const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/frame_host/cross_process_frame_connector.cc:225:21
    #6 0x7fa49622d1ee in content::RenderWidgetHostViewChildFrame::GestureEventAck(blink::WebGestureEvent const&, content::InputEventAckState) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/renderer_host/render_widget_host_view_child_frame.cc:0:25
    #7 0x7fa4961ddcba in content::RenderWidgetHostImpl::OnGestureEventAck(content::EventWithLatencyInfo<blink::WebGestureEvent> const&, content::InputEventAckSource, content::InputEventAckState) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/renderer_host/render_widget_host_impl.cc:2421:12
    #8 0x7fa495f67b79 in AckGestureEventToClient /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/renderer_host/input/gesture_event_queue.cc:273:12
    #9 0x7fa495f67b79 in content::GestureEventQueue::AckCompletedEvents() /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/renderer_host/input/gesture_event_queue.cc:260:0
    #10 0x7fa495f65747 in content::GestureEventQueue::ProcessGestureAck(content::InputEventAckSource, content::InputEventAckState, blink::WebInputEvent::Type, ui::LatencyInfo const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/renderer_host/input/gesture_event_queue.cc:245:3
    #11 0x7fa495f756c3 in content::InputRouterImpl::GestureEventHandled(content::EventWithLatencyInfo<blink::WebGestureEvent> const&, content::InputEventAckSource, ui::LatencyInfo const&, content::InputEventAckState, base::Optional<ui::DidOverscrollParams> const&, base::Optional<cc::TouchAction> const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/renderer_host/input/input_router_impl.cc:519:24
    #12 0x7fa495f7a674 in void base::internal::FunctorTraits<void (content::InputRouterImpl::*)(content::EventWithLatencyInfo<blink::WebGestureEvent> const&, content::InputEventAckSource, ui::LatencyInfo const&, content::InputEventAckState, base::Optional<ui::DidOverscrollParams> const&, base::Optional<cc::TouchAction> const&), void>::Invoke<base::WeakPtr<content::InputRouterImpl>, content::EventWithLatencyInfo<blink::WebGestureEvent>, content::InputEventAckSource, ui::LatencyInfo const&, content::InputEventAckState, base::Optional<ui::DidOverscrollParams> const&, base::Optional<cc::TouchAction> const&>(void (content::InputRouterImpl::*)(content::EventWithLatencyInfo<blink::WebGestureEvent> const&, content::InputEventAckSource, ui::LatencyInfo const&, content::InputEventAckState, base::Optional<ui::DidOverscrollParams> const&, base::Optional<cc::TouchAction> const&), base::WeakPtr<content::InputRouterImpl>&&, content::EventWithLatencyInfo<blink::WebGestureEvent>&&, content::InputEventAckSource&&, ui::LatencyInfo const&, content::InputEventAckState&&, base::Optional<ui::DidOverscrollParams> const&, base::Optional<cc::TouchAction> const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/bind_internal.h:211:12
    #13 0x7fa495f7a383 in MakeItSo<void (content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebGestureEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &), base::WeakPtr<content::InputRouterImpl>, content::EventWithLatencyInfo<blink::WebGestureEvent>, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &> /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/bind_internal.h:314:5
    #14 0x7fa495f7a383 in RunImpl<void (content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebGestureEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &), std::__1::tuple<base::WeakPtr<content::InputRouterImpl>, content::EventWithLatencyInfo<blink::WebGestureEvent> >, 0, 1> /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/bind_internal.h:368:0
    #15 0x7fa495f7a383 in base::internal::Invoker<base::internal::BindState<void (content::InputRouterImpl::*)(content::EventWithLatencyInfo<blink::WebGestureEvent> const&, content::InputEventAckSource, ui::LatencyInfo const&, content::InputEventAckState, base::Optional<ui::DidOverscrollParams> const&, base::Optional<cc::TouchAction> const&), base::WeakPtr<content::InputRouterImpl>, content::EventWithLatencyInfo<blink::WebGestureEvent> >, void (content::InputEventAckSource, ui::LatencyInfo const&, content::InputEventAckState, base::Optional<ui::DidOverscrollParams> const&, base::Optional<cc::TouchAction> const&)>::RunOnce(base::internal::BindStateBase*, content::InputEventAckSource, ui::LatencyInfo const&, content::InputEventAckState, base::Optional<ui::DidOverscrollParams> const&, base::Optional<cc::TouchAction> const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/bind_internal.h:336:0
    #16 0x7fa49433162b in Run /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/callback.h:65:12
    #17 0x7fa49433162b in content::mojom::WidgetInputHandler_DispatchEvent_ForwardToCallback::Accept(mojo::Message*) /usr/local/google/home/sadrul/devel/c/src/out/cros/gen/content/common/input/input_handler.mojom.cc:1559:0
    #18 0x7fa49cd00638 in mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:414:23
    #19 0x7fa49ccfe838 in mojo::FilterChain::Accept(mojo::Message*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/filter_chain.cc:40:17
    #20 0x7fa49cd0460d in mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:306:19
    #21 0x7fa49cd2b172 in mojo::internal::MultiplexRouter::ProcessIncomingMessage(mojo::internal::MultiplexRouter::MessageWrapper*, mojo::internal::MultiplexRouter::ClientCallBehavior, base::SequencedTaskRunner*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/multiplex_router.cc:880:42
    #22 0x7fa49cd291dd in mojo::internal::MultiplexRouter::Accept(mojo::Message*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/multiplex_router.cc:604:38
    #23 0x7fa49ccfe838 in mojo::FilterChain::Accept(mojo::Message*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/filter_chain.cc:40:17
    #24 0x7fa49ccebe2d in mojo::Connector::ReadSingleMessage(unsigned int*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/connector.cc:445:51
    #25 0x7fa49ccee6b8 in mojo::Connector::ReadAllAvailableMessages() /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/connector.cc:474:10
    #26 0x7fa49ccee054 in mojo::Connector::OnHandleReadyInternal(unsigned int) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/bindings/lib/connector.cc:375:3
    #27 0x7fa49ccefb14 in Run /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/callback.h:94:12
    #28 0x7fa49ccefb14 in mojo::SimpleWatcher::DiscardReadyState(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/system/simple_watcher.h:194:0
    #29 0x7fa49cc5a36d in Run /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/callback.h:94:12
    #30 0x7fa49cc5a36d in mojo::SimpleWatcher::OnHandleReady(int, unsigned int, mojo::HandleSignalsState const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../mojo/public/cpp/system/simple_watcher.cc:276:0
    #31 0x7fa49cc5b594 in Invoke<const base::WeakPtr<mojo::SimpleWatcher> &, const int &, const unsigned int &, const mojo::HandleSignalsState &> /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/bind_internal.h:211:12
    #32 0x7fa49cc5b594 in MakeItSo<void (mojo::SimpleWatcher::*const &)(int, unsigned int, const mojo::HandleSignalsState &), const base::WeakPtr<mojo::SimpleWatcher> &, const int &, const unsigned int &, const mojo::HandleSignalsState &> /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/bind_internal.h:314:0
    #33 0x7fa49cc5b594 in void base::internal::Invoker<base::internal::BindState<void (mojo::SimpleWatcher::*)(int, unsigned int, mojo::HandleSignalsState const&), base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState>, void ()>::RunImpl<void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, 0ul, 1ul, 2ul, 3ul>(void (mojo::SimpleWatcher::* const&)(int, unsigned int, mojo::HandleSignalsState const&), std::__1::tuple<base::WeakPtr<mojo::SimpleWatcher>, int, unsigned int, mojo::HandleSignalsState> const&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/bind_internal.h:368:0
    #34 0x7fa49faac403 in Run /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/callback.h:65:12
    #35 0x7fa49faac403 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/debug/task_annotator.cc:55:0
    #36 0x7fa49fb55be1 in base::internal::IncomingTaskQueue::RunTask(base::PendingTask*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/message_loop/incoming_task_queue.cc:123:19
    #37 0x7fa49fb6425d in base::MessageLoop::RunTask(base::PendingTask*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/message_loop/message_loop.cc:399:25
    #38 0x7fa49fb65489 in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/message_loop/message_loop.cc:411:5
    #39 0x7fa49fb65de1 in base::MessageLoop::DoWork() /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/message_loop/message_loop.cc:455:16
    #40 0x7fa49fb70181 in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/message_loop/message_pump_libevent.cc:220:31
    #41 0x7fa49fb62c28 in base::MessageLoop::Run(bool) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/message_loop/message_loop.cc:350:12
    #42 0x7fa49fc298be in base::RunLoop::Run() /usr/local/google/home/sadrul/devel/c/src/out/cros/../../base/run_loop.cc:130:14
    #43 0x55cc50f93e98 in ChromeBrowserMainParts::MainMessageLoopRun(int*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../chrome/browser/chrome_browser_main.cc:1956:12
    #44 0x7fa495466ed5 in content::BrowserMainLoop::RunMainMessageLoopParts() /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/browser_main_loop.cc:1212:29
    #45 0x7fa49547191a in content::BrowserMainRunnerImpl::Run() /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/browser_main_runner.cc:138:17
    #46 0x7fa49545a6f6 in content::BrowserMain(content::MainFunctionParams const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/browser/browser_main.cc:46:28
    #47 0x7fa497b909bf in content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/app/content_main_runner.cc:426:14
    #48 0x7fa497b92c9b in content::ContentMainRunnerImpl::Run() /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/app/content_main_runner.cc:720:12
    #49 0x7fa4a027c616 in service_manager::Main(service_manager::MainParams const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../services/service_manager/embedder/main.cc:456:29
    #50 0x7fa497b8ed6c in content::ContentMain(content::ContentMainParams const&) /usr/local/google/home/sadrul/devel/c/src/out/cros/../../content/app/content_main.cc:19:10
    #51 0x55cc4df92fd1 in ChromeMain /usr/local/google/home/sadrul/devel/c/src/out/cros/../../chrome/app/chrome_main.cc:129:12

    #52 0x7fa46a4d3f45 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287:0
    #53 0x55cc4dec52fa in _start ??:0:0


Not sure if this gives additional insight to explain what's going wrong.

I can repro this consistently if I put my mouse wheel into its free-spin mode and move the mouse around in an OOPIF while the wheel is spinning. We enter the SendScrollBegin (kPhaseBegan case) twice.

Moving the mouse while the wheel is spinning is fine when done in the main frame, I'm not sure why this would only affect OOPIF.

Most likely it's a bug related to wheel scroll latching, I've started working on this.

The regression happens because of the following cl:
https://chromium-review.googlesource.com/c/chromium/src/+/854239

particularly this part of the code:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?l=424

To reproduce the crash, do a mouse wheel scroll on one of the ads in slashdot.org with free spinning wheel and move the mouse. The crash will happen on the first wheel tick after mouse move since the mouseMove breaks scroll latching. i.e. On the first wheel event after mouse move the phase handler will send the pending wheel event with phaseEnded and assign PhaseBegan to the new mouse wheel. The newly added part of the code checks if (!root_view->TransformPointToCoordSpaceForView(
          mouse_wheel_event.PositionInWidget(), target, &point_in_target))  and ignores the mouse wheel if the transformation is not successful, in this particular case the mouse wheel with phaseEnded gets ignored, and the crash happens while processing the wheel with phaseBegan.

I don't know why this transformation check is needed, since with wheel scroll latching enabled we will keep sending the wheel events to the target at the beginning of the scroll even when the target is not under the cursor anymore. 

riajiang@chromium.org@ could you please take a look?

The wheel event routing logic has changed significantly and I don't really understand all the details.

What I had in mind while implementing the latching was: on wheel with phaseBegan we find the target and latch to it till wheel with phaseEnded, regardless of the location of the wheel events or the cursor. With recent changes about how target views are found/reused it seems it seems like this assumption is violated.
Is there any documentations about the last changes?

So it's curious that the transformation would fail, given that the view we're transforming from (root_view) should be guaranteed to be an ancestor of the original.

But probably doing a no-consumer ack is the wrong thing here ... instead it should just send the event anyways, and log an error.

Re c#13: The idea is the same, but we have the complication that finding the original target to latch to may take a while (it may require a round-trip to the renderer). But once it's found, the target is locked until phaseEnded, and all wheels are sent to the target.

In the past we always transformed the wheel event coords (as best we could) to the target's coordinate, using the delta value. In the new scheme we're just trying to do something a little better than using a delta to achieve the same end result.

@sahel: Did it always hit https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?l=424 when it crashes for you? All the times that I was able to repro, it didn't enter that block. I was testing http://springfield.wat.corp.google.com/stuff/oopif.html on TOT with --site-per-process --disable-features=EnablePreventLayerSquashing (it's the same w/o the --disable-features=... flag)

update: 
There seems to be two causes for the crash:
1. We are coalescing events wrong. Instead of properly coalescing mouse-wheel events, we are replacing the last event in the queue with the new one [1]. Hence the crash if we have replaced a phaseEnd mouse-wheel with a new mouse-wheel and didn't update scroll_in_progress_ state.
2. We are not checking fallback surfaces in SurfaceHittest (surface synchronization has already been turned on by default), so if we try to do TransformPointToCoordSpaceForView [2] when the primary surface is not available yet, we couldn't do the transform successfully and ignore the event. 

[1]https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_targeter.cc?l=19
[2]https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?l=424

update:
For the point 2 above in comment 17, it turns out that primary surfaces are always available when TransformPointToCoordSpaceForView fails. When TransformPointToCoordSpaceForView fails, it fails to find the surface that matches the target_surface_id even tho we do have the wanted surface in the surface-tree (printed with [1]). It looks like the wanted surface_id is not in the DrawQuad in the RenderPass [2]. I'm not sure what's the relationship between the SurfaceReferences we can print out and the DrawQuad that we were testing against.

Since the branch cut is this Thursday, I think we should change back to use the delta scheme first so that the only significant change is the async targeting for site isolation, make sure we don't get more crashes before the branch cut, and then debug why we are not finding the correct surface when transforming on the side/ after that. What do you guys think?

[1] https://cs.chromium.org/chromium/src/components/viz/service/surfaces/surface_manager.cc?type=cs&l=96
[2] https://cs.chromium.org/chromium/src/components/viz/service/surfaces/surface_hittest.cc?l=245

fsamuel@ - Could you please take a look here?

I'd really rather not have to go back to deltas if we can avoid it.

If, by chance, we do temporarily go back to using the delta, and since that will require plumbing new parameters, may I suggest we give DispatchXEvent() both (1) the original event, and (2) the transformed point? Then the touch/gesture/mousehweel dispatch handlers can calculate the delta whenever they set the target (phaseBegin/TouchStart).

This gives the delegate control over how the deltas are handled, and RenderWidgetTargeter doesn't have to know the details of what a delta even is.

It sounds like switching to passing deltas is a good idea, we should really have a fix to the coalescing issue landed by tomorrow. We can leave this bug open to resolve it in a better way.

update: TransformPointToCoordSpaceForView fails when the previous targeted OOPIF (from phaseBegan) becomes invisible (so it's not in the draw-quad-list anymore) when we try to find it for following phaseUpdate/phaseEnd events. In which case, wheel scroll latching prob shouldn't target the hidden OOPIF anymore and early out because of TransformPointToCoordSpaceForView failure is prob the right thing to do. @sahel, could you confirm what the expected latching beheviour should be in this case? If latching shouldn't target hidden OOPIF, then existing code should in theory work correctly if we clean all the states properly [1] when we ack NO_CONSUMER?

[1] https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?q=dispatchmousewheelevent&sq=package:chromium&l=426

Re c#22 ... It sort of seems wrong that, if the OOPIF disappears, we're finding out about it from the transform function, and not from the hit-testing code itself. Why aren't we getting back a nullptr for the target in this case?

What can happen is that the target we find may no longer be visible on screen. An example page: http://output.jsbin.com/rotulidugi

If you start the timer, and start mouse-wheeling on the iframe, then at some point, the iframe will be pushed down in the page enough that it becomes invisible. Since you are still mouse-wheeling, it will attempt to do the transform, but fail (because the oopif's surface is no longer on screen, and so there's no draw-quad for it).

Re c#23: for phase-update mouse-wheels, I believe we don't actually do any location-based hit-testing. We use whatever target we found for phase-begin mouse-wheel. So if the iframe disappears in the middle of scrolling, we don't have a good way of knowing it.

Re c#24 - thanks sadrul@ ... that makes sense ... I guess we're still trying to use the transform function long after we last did a hit-test, aren't we?

That being said, it still seems fragile that our ability to transform between two views is dependent on the quads, since they may disappear when both views are both still very much in existence.

Yes, that's just another symptom of graphics use cases and input event use cases not lining up, which is the reason we are moving toward Viz in the first place.

Re c#22:
I am not sure about what you mean by hidden, Do you mean that the target gets out of the screen because of scrolling? As long as the target exists we should keep sending the wheel events to the target even when the user cannot see the target.

This crash happens since a wheel event with phaseBegan gets dispatched and its corresponding wheel event with phaseEnded doesn't get dispatched because of [1].
Acking the wheel event with phaseEnded instead of dispatching it won't solve the issue since if we dispatch a wheel event with phaseBegan and don't dispatch its corresponding wheel event with phaseEnded, GSB/GSE pairing gets violated.

[1]https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?q=dispatchmousewheelevent&sq=package:chromium&l=424

With wheel scroll latching enabled we want to dispatch all wheel events in a sequence to the same target. The only exception is when the latched target gets deleted, then we don't care about the rest of the wheel events in the sequence and we can ignore them. 

c28: "Acking the wheel event with phaseEnded instead of dispatching it won't solve the issue since if we dispatch a wheel event with phaseBegan and don't dispatch its corresponding wheel event with phaseEnded, GSB/GSE pairing gets violated."

-> can we send a fake GSE in that case, like when the wheel scroll latching timeout has hit?

the wheel latching timeout generates and routes a Wheel event with phaseEnded which later on gets processed to generate a GSE.

From kenrb@:
> It sounds like switching to passing deltas is a good idea, 

I agree. I think we are too close to the branch point to do anything else. The problem with having a fixed delta still exists without async targeting. Async targeting may make it more noticeable, but we should work on that as a separate problem, especially since it's not trivial.

From wjmaclean@:
> it still seems fragile that our ability to transform between two
> views is dependent on the quads, since they may disappear when
> both views are both still very much in existence.

Indeed. With viz, the browser will not even have the quads. But with full-viz hit-testing, it should get better.

#28:
> I am not sure about what you mean by hidden, Do you mean that the target gets out of the screen because of scrolling?

It is still able to receive input events, but we use the graphics system to convert the event's (x,y) from the main frame's coordinate space to the OOPIF's coordinate space, and Sadrul is saying that we lose the ability to do that when the frame leaves the screen -- because it is no longer drawn.

To fix this, we could replace the call to TransformPointToCoordSpaceForView() and instead just use the relative GetViewBounds() offsets of the root and the OOPIF RWHVs. It is slightly less accurate because it doesn't account for transformations, but it wouldn't have the problem with scrolling off screen.

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

commit 6eeea9f06126e377e4e30db7e66bf323923d9e5a
Author: Ria Jiang <riajiang@chromium.org>
Date: Thu Jan 18 01:09:59 2018

Coalesce events properly in RenderWidgetTargeter.

We were not colalescing events properly in RenderWidgetTargeter::
MergeEventIfPossible. As a result, when we have incorrecly replaced a
mouse-wheel event of phaseEnd in the queue, we get crashes complaining
that there's already scrolling in progress when we process a new
phaseBegan event.

Bug:  802063 
Test: SitePerProcessBrowserTest.InputEventRouterWheelCoalesceTest
Change-Id: Idb42b310a7542697df601e5646fcc11bd16d9701
Reviewed-on: https://chromium-review.googlesource.com/871514
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529993}
[modify] https://crrev.com/6eeea9f06126e377e4e30db7e66bf323923d9e5a/content/browser/renderer_host/render_widget_targeter.cc
[modify] https://crrev.com/6eeea9f06126e377e4e30db7e66bf323923d9e5a/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/6eeea9f06126e377e4e30db7e66bf323923d9e5a/testing/buildbot/filters/viz.content_browsertests.filter

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

commit 1549ec4c016c25b31936fb229519f9fba74a553e
Author: Ria Jiang <riajiang@chromium.org>
Date: Thu Jan 18 16:27:01 2018

Change back to use wheel-target delta for mouse-wheel events.

For mouse-wheel events with phaseChanged and phaseEnd, we used to use
the wheel_target_.delta (stored from phaseBegan event) to transform the
event location. In [1], when we changed mouse-wheel to do async
targeting, we changed mouse-wheel to always transform events using
TransformPointToCoordSpaceForView, which caused some crashes when the
target has already became invisible since we found it for the phaseBegan
event. Before we fix it properly (details in the bug), changing back to
use the delta scheme to make sure async targeting works correctly before
the branch cut.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/854239

Bug:  802063 
Change-Id: I898cfb25d9a8b5f96a3f7320d3a83f14667727f3
Reviewed-on: https://chromium-review.googlesource.com/871750
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530160}
[modify] https://crrev.com/1549ec4c016c25b31936fb229519f9fba74a553e/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/1549ec4c016c25b31936fb229519f9fba74a553e/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/1549ec4c016c25b31936fb229519f9fba74a553e/content/browser/renderer_host/render_widget_targeter.cc
[modify] https://crrev.com/1549ec4c016c25b31936fb229519f9fba74a553e/content/browser/renderer_host/render_widget_targeter.h

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

commit 25d4203af7f03a2651146d5ffd436ca4b99c4ea0
Author: Ria Jiang <riajiang@chromium.org>
Date: Thu Jan 18 16:53:19 2018

Change to use touch-target delta for non-start touch events.

For touch events after kTouchStart, we used to use the
touch_target_.delta (stored from touchStart event) to transform the
event location. In [1], when we changed touch events to do async
targeting, we changed it to always transform events using
TransformPointToCoordSpaceForView. Before we fix transform properly
(details in the bug), changing back to use the delta scheme to make sure
async targeting works correctly before the branch cut.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/860338

Bug:  802063 
Change-Id: Iefeaa5f1d891ac35914a5d419cfb8553f1b0c4d5
Reviewed-on: https://chromium-review.googlesource.com/872392
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530167}
[modify] https://crrev.com/25d4203af7f03a2651146d5ffd436ca4b99c4ea0/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/25d4203af7f03a2651146d5ffd436ca4b99c4ea0/content/browser/renderer_host/render_widget_host_input_event_router.h

I just got a DCHECK(needs_update) in MouseWheelEventQueue::ProcessMouseWheelAck. Is it related to this or should I file a new bug?

Callstack:
 	base.dll!base::debug::BreakDebugger() Line 21	C++	Symbols loaded.
 	base.dll!logging::LogMessage::~LogMessage() Line 844	C++	Symbols loaded.
>	content.dll!content::MouseWheelEventQueue::ProcessMouseWheelAck(content::InputEventAckSource ack_source, content::InputEventAckState ack_result, const ui::LatencyInfo & latency_info) Line 195	C++	Symbols loaded.
 	content.dll!content::InputRouterImpl::MouseWheelEventHandled(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> & event, content::InputEventAckSource source, const ui::LatencyInfo & latency, content::InputEventAckState state, const base::Optional<ui::DidOverscrollParams> & overscroll, const base::Optional<cc::TouchAction> & touch_action) Line 552	C++	Symbols loaded.
 	content.dll!base::internal::FunctorTraits<void (content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &) __attribute__((thiscall)),void>::Invoke<base::WeakPtr<content::InputRouterImpl>,content::EventWithLatencyInfo<blink::WebMouseWheelEvent>,content::InputEventAckSource,const ui::LatencyInfo &,content::InputEventAckState,const base::Optional<ui::DidOverscrollParams> &,const base::Optional<cc::TouchAction> &>(void(content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &) method, base::WeakPtr<content::InputRouterImpl> && receiver_ptr, content::EventWithLatencyInfo<blink::WebMouseWheelEvent> && args, content::InputEventAckSource && args, const ui::LatencyInfo & args, content::InputEventAckState && args, const base::Optional<ui::DidOverscrollParams> & args, const base::Optional<cc::TouchAction> & args) Line 211	C++	Symbols loaded.
 	content.dll!base::internal::InvokeHelper<1,void>::MakeItSo<void (content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &) __attribute__((thiscall)),base::WeakPtr<content::InputRouterImpl>,content::EventWithLatencyInfo<blink::WebMouseWheelEvent>,content::InputEventAckSource,const ui::LatencyInfo &,content::InputEventAckState,const base::Optional<ui::DidOverscrollParams> &,const base::Optional<cc::TouchAction> &>(void(content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &) && functor, base::WeakPtr<content::InputRouterImpl> && weak_ptr, content::EventWithLatencyInfo<blink::WebMouseWheelEvent> && args, content::InputEventAckSource && args, const ui::LatencyInfo & args, content::InputEventAckState && args, const base::Optional<ui::DidOverscrollParams> & args, const base::Optional<cc::TouchAction> & args) Line 317	C++	Symbols loaded.
 	content.dll!base::internal::Invoker<base::internal::BindState<void (content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &) __attribute__((thiscall)),base::WeakPtr<content::InputRouterImpl>,content::EventWithLatencyInfo<blink::WebMouseWheelEvent> >,void (content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &)>::RunImpl<void (content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &) __attribute__((thiscall)),std::tuple<base::WeakPtr<content::InputRouterImpl>,content::EventWithLatencyInfo<blink::WebMouseWheelEvent> >,0,1>(void(content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &) && functor, std::tuple<base::WeakPtr<content::InputRouterImpl>,content::EventWithLatencyInfo<blink::WebMouseWheelEvent> > && bound, std::integer_sequence<unsigned int,0,1>, content::InputEventAckSource && unbound_args, const ui::LatencyInfo & unbound_args, content::InputEventAckState && unbound_args, const base::Optional<ui::DidOverscrollParams> & unbound_args, const base::Optional<cc::TouchAction> & unbound_args) Line 368	C++	Symbols loaded.
 	content.dll!base::internal::Invoker<base::internal::BindState<void (content::InputRouterImpl::*)(const content::EventWithLatencyInfo<blink::WebMouseWheelEvent> &, content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &) __attribute__((thiscall)),base::WeakPtr<content::InputRouterImpl>,content::EventWithLatencyInfo<blink::WebMouseWheelEvent> >,void (content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &)>::RunOnce(base::internal::BindStateBase * base, content::InputEventAckSource unbound_args, const ui::LatencyInfo & unbound_args, content::InputEventAckState unbound_args, const base::Optional<ui::DidOverscrollParams> & unbound_args, const base::Optional<cc::TouchAction> & unbound_args) Line 336	C++	Symbols loaded.
 	content.dll!base::OnceCallback<void (content::InputEventAckSource, const ui::LatencyInfo &, content::InputEventAckState, const base::Optional<ui::DidOverscrollParams> &, const base::Optional<cc::TouchAction> &)>::Run(content::InputEventAckSource args, const ui::LatencyInfo & args, content::InputEventAckState args, const base::Optional<ui::DidOverscrollParams> & args, const base::Optional<cc::TouchAction> & args) Line 65	C++	Symbols loaded.
 	content.dll!content::mojom::WidgetInputHandler_DispatchEvent_ForwardToCallback::Accept(mojo::Message * message) Line 1559	C++	Symbols loaded.

It doesn't look like it's related. Previous crashes were caused by coalescing and TransformPointToCoordSpaceForView and should not happen anymore.