Curious.

The fix we landed marks the OOPIFs *root* layer as having a wheel handler ... it assumes that the root layer rect contains all the sub-layer rects ... is that possibly a bad assumption? I'm just now reading this for the first time, so I'll try testing with the layer borders displayed to see what happens ...

I'm not quite sure what to make of this so far.

Some comments on the initial report:

*** Also reproduces with --site-per-process

*** I've generally observed that the number of non-scrolling ads are in the minority, and some pages all the ads scroll

*** I tested over a ChromeRemoteDesktop link to a fast workstation ... could it be something is racy?

It *doesn't* seem to be related to mouse wheel though ... has anyone tried with a touch screen to see if there are any ads that resist touch scrolling?

What I've discovered so far is this:

0) I have observed the symptoms on slashdot pages where non of the OOPIF frames seem to be installing any wheel handlers.

1) The mouse wheels are getting acked, unconsumed, to MouseWheelEventQueue:

https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/mouse_wheel_event_queue.cc?rcl=8b03b3715b4f951d87890c6ef9f6cbe027f777f0&l=90

2) The Gesture scroll updates are getting dispatched, and coming back to RenderWidgetHostViewChildFrame::GestureEventAck().

3) For the frames that don't scroll, the value of |ack_result| is INPUT_EVENT_ACK_STATE_CONSUMED

4) On most pages it is not obvious that anything *ought* to scroll in the frame, e.g. I've seen this happen where the sole element in the frame is a canvas tag properly sized to the frame, and no scroll handlers or wheel handlers.

This is why they don't bubble the scroll to the main frame. The mystery at this point is, who's consuming the GSUs? I'm pretty sure they're being consumed in the renderer, but I don't have anything more specific than that.

Since I'm ooo for a bit, if anyone else wants to continue digging into this feel free. If not, I'll pick it up when I get back.

> has anyone tried with a touch screen to see if there are any ads that resist touch scrolling?

Yes, on a Pixel 1, there are OOPIF ads that prevent me from scrolling the page if I touch the screen there and drag to scroll.  (For example, I'm seeing that for IBM ads on Slashdot.)

That said, I'm surprised to find that the bug is kind of non-deterministic (whether I'm using mousewheel, trackpad, or touch scrolling).  Sometimes scrolling starts working again over a given ad, and then stops again, and I haven't figured out what the transition is.

I'm back working on this now.

I've noticed that the bug seems to correlate somewhat with ads that have (css?) animations in them. As an experiment I tried running with --disable-smooth-scrolling, and when I do this the issue seems to disappear (caveat: I just discovered this, so need to test with it some more).

Theory: (with thanks to kenrb@ for suggesting a ScrollingCoordinator link)
Somehow oopifs when they scroll do not interact properly with any in-flight animations, since ScrollingCoordinator only understands main-frames and not tabs.

I'll continue to investigate this, but it looks like there is some light at the end of the tunnel ...

"tabs" -> "sub-frames"

I have an initial CL up at https://codereview.chromium.org/2785533003 and am running it on the trybots.

It solves the wheel-scroll issue, but there is still something going on with touch-screen scrolling (the initial scroll fails, but if you scroll the mainframe then go back to the sub-frame of interest, it starts working).

James, do you think  issue 704410  could be related to this one?

lfg@ - I looked at  issue 704410 , and while they're in the same part of the codebase, I don't think it's related. It's entirely based on (1) navigation, and (2) works without any OOPIFs present.

I think I understand this issue pretty well now: the fact OOPIFs don't get scroll layers assigned in the VisualViewport means they don't properly handle scrolls when the scrolls should bubble. I'm talking to bokan@ about how to best resolve that.

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

commit a755e30003e89da8ed1ee6f9e4e039acb395e3af
Author: wjmaclean <wjmaclean@chromium.org>
Date: Thu Apr 20 00:43:16 2017

Animated scroll shouldn't require main-frame for OOPIFs.

At present the animated scroll code for mouse wheels will mark as
SCROLL_ON_IMPL any attempt to scroll if there is no scrollable
layer, and in an OOPIF this causes the GestureScrollUpdate to be
marked as consumed when in fact it isn't. In turn, this prevents
the GestureScrollUpdate from bubbling to the embedder.

This CL fixes LayerTreeHostImpl::ScrollAnimated() to corrently report
such GestureScrollUpdate events as ignored, so they can bubble to the
embedder.

This CL has no behavioral changes for Mac, as it does not support
smooth scrolling.

BUG= 701178 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2785533003
Cr-Commit-Position: refs/heads/master@{#465828}

[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/cc/test/layer_tree_test.cc
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/cc/test/stub_layer_tree_host_client.cc
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/cc/test/stub_layer_tree_host_client.h
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/cc/trees/layer_tree_host_client.h
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/cc/trees/layer_tree_settings.h
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/content/renderer/gpu/render_widget_compositor.h
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/content/renderer/render_widget.cc
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/content/test/layouttest_support.cc
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/third_party/WebKit/Source/platform/testing/WebLayerTreeViewImplForTesting.cpp
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/third_party/WebKit/Source/platform/testing/WebLayerTreeViewImplForTesting.h
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/ui/compositor/compositor.cc
[modify] https://crrev.com/a755e30003e89da8ed1ee6f9e4e039acb395e3af/ui/compositor/compositor.h

I can confirm in 60.0.3076.0.  Thanks!

James, do you think this is worth merging to M59, or is it too large of a change?  I would imagine it might impact the OOPIF-based <webview> trial, for example.

I would suspect it's too large, though I'm willing to try if the TPM is ok with trying ... I'll add the request merge label here to get that discussion going ...

Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit bd145783f35871897d7bcf24b194cae2631186b3
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Mon Apr 24 17:06:39 2017

Animated scroll shouldn't require main-frame for OOPIFs.

At present the animated scroll code for mouse wheels will mark as
SCROLL_ON_IMPL any attempt to scroll if there is no scrollable
layer, and in an OOPIF this causes the GestureScrollUpdate to be
marked as consumed when in fact it isn't. In turn, this prevents
the GestureScrollUpdate from bubbling to the embedder.

This CL fixes LayerTreeHostImpl::ScrollAnimated() to corrently report
such GestureScrollUpdate events as ignored, so they can bubble to the
embedder.

This CL has no behavioral changes for Mac, as it does not support
smooth scrolling.

BUG= 701178 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2785533003
Cr-Commit-Position: refs/heads/master@{#465828}
(cherry picked from commit a755e30003e89da8ed1ee6f9e4e039acb395e3af)

Review-Url: https://codereview.chromium.org/2833423003 .
Cr-Commit-Position: refs/branch-heads/3071@{#166}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/cc/test/layer_tree_test.cc
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/cc/test/stub_layer_tree_host_client.cc
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/cc/test/stub_layer_tree_host_client.h
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/cc/trees/layer_tree_host_client.h
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/cc/trees/layer_tree_settings.h
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/content/renderer/gpu/render_widget_compositor.h
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/content/renderer/render_widget.cc
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/content/test/layouttest_support.cc
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/third_party/WebKit/Source/platform/testing/WebLayerTreeViewImplForTesting.cpp
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/third_party/WebKit/Source/platform/testing/WebLayerTreeViewImplForTesting.h
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/ui/compositor/compositor.cc
[modify] https://crrev.com/bd145783f35871897d7bcf24b194cae2631186b3/ui/compositor/compositor.h

Verified this issue on Windows-10, Ubuntu 14.04 and Mac OS 10.12.4 using chrome latest Dev #59.0.3071.25 by following steps mentioned in the original comment. Observed able to scroll right side of the ad page as expected. Hence adding TE-Verified label.

Deleted: 701178.mp4 9.5 MB

	701178.mp4 9.5 MB View Download