New issue
Advanced search Search tips

Issue 900357 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Task



Sign in to add a comment

Do not bubble GFS from oopif

Project Member Reported by sahel@chromium.org, Oct 30

Issue description

With scroll bubbling we decide to bubble the rest of the events in a sequence if the GSB of the sequence is bubbled. This works because the GSB event is handled blocking.

With browser side fling since the GFS is processed by the fling controller instead of getting queued in gesture event queue, it is possible that the GFS gets processed before arrival of the GSB's ack.
In this case we won't bubble the GFS.

The fix for this is not to bubble the GFS and let the fling controller of the oopif generate GSU events which get bubbled. But then the parent must be able to tell the oopif's fling controller to stop flinging (GSU event generation) when the parent's renderer cannot consume the generated GSUs anymore.
 
Cc: mcnee@chromium.org creis@chromium.org wjmaclean@chromium.org
Components: Internals>Compositing>Scroll Internals>Sandbox>SiteIsolation
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 13

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

commit dc59b447f027f67515929b508afe948b5ad3ef41
Author: Sahel Sharify <sahel@chromium.org>
Date: Tue Nov 13 20:41:03 2018

Do not bubble GFS events from oopif.

With browser side fling instead of queuing a GFS in gesture event queue
the fling controller processes the event immediately. It means that it is
possible to process the GFS before arrival of the GSB's ack. This behavior
does not cause any issues since after processing the GFS event the fling
controller starts to generate GSU events at every begin frame. Since the
generated GSU events are queued in gesture event queue they will wait for
the ack of the GSB before getting processed.

The only issue with the logic above is when we are bubbling GFS:
It is possible that the fling controller processes the GFS before the
ack of the GSB is arrived. In this case we won't bubble the GFS since
we only bubble the rest of the scroll events in a sequence only when
the GSB of the sequence is bubbled.

This cl changes the scroll bubbling logic to avoid GFS bubbling. With
this change the GFS won't get bubbled and the fling controller of the
oopif will generate GSU events which will get bubbled. When the bubbling
target did not consume a generated GSU event it will tell the oopif's fling
controller (via RWH_input_event_router) to stop flinging.

The first three tests are not new. They just show that the flinging on
the oopif still causes the parent to scroll when we are bubbling scroll.
The last test shows that the child's fling controller stops flinging when
the parent's renderer does not consumed the generated GSUs that are bubbled.

I also changed the following tests to be browser tests since instead of the
fling controller cancelling the fling when generated GSUs are not consumed
the RWHV_* calls stop fling. This change is necessary since the RWHV_* knows
when the scroll is getting bubbled but the fling controller does not:
-Early[Touchpad|Touchscreen]FlingCancelationOnInertialGSUAckNotConsumed

BrowserSideFlingBrowserTest.TouchscreenInertialGSUsBubblesFromOOPIF,
BrowserSideFlingBrowserTest.InertialGSEGetsBubbledFromOOPIF,
BrowserSideFlingBrowserTest.InertialGSUBubblingStopsWhenParentCannotScroll

Bug:  900357 
Test: BrowserSideFlingBrowserTest.TouchpadInertialGSUsBubblesFromOOPIF,
Change-Id: I784aa03b561d0482a95341544133761e40a1917f
Reviewed-on: https://chromium-review.googlesource.com/c/1308598
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607726}
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/frame_host/cross_process_frame_connector.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/input/fling_browsertest.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/input/fling_controller.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/input/fling_controller.h
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/input/fling_controller_unittest.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_input_event_router.h
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/dc59b447f027f67515929b508afe948b5ad3ef41/content/browser/renderer_host/render_widget_host_view_mac.mm

Labels: -Type-Bug Type-Task
Status: Fixed (was: Assigned)

Sign in to add a comment