New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 717205 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug-Regression

Blocked on:
issue 708236



Sign in to add a comment

Fling scrolling doesn't work on Facebook on ChromeOS

Project Member Reported by esprehn@chromium.org, May 1 2017

Issue description

Google Chrome	57.0.2987.146 (Official Build) (64-bit)
Revision	0
Platform	9202.64.0 (Official Build) stable-channel samus

What steps will reproduce the problem?
(1) Go to facebook.com
(2) Login
(3) Fling on the trackpad.

What is the expected result?

Should fling scroll.

What happens instead?

Stops scrolling immediately when you lift your fingers from the track pad. In general scrolling feels very abrupt with no acceleration.
 
trace_scrolling.json.gz
3.0 MB Download
Note that fling does appear to work on some websites, ex. hacker news comments scroll fine, but many others don't.
esprehn@ Can you remove the mousewheel event listener in devtools for facebook?

I've done this locally and I see that it behaves nicely. I believe they are likely adjusting the scroll position themselves canceling any scroll. This might be fixed via CSS Smooth Scroll.
I've noticed this on lots of websites on my Chromebook recently. Did something change in Chrome?
Cc: ben.mau...@gmail.com
Summary: Fling scrolling doesn't work on lots of websites on ChromeOS (was: Fling scrolling doesn't work on lots of websites)
Adding this instrumentation on the console I can see that, although removing the mousewheel listener does fix the problem, Facebook isn't actually calling preventDefault on the wheel event:

document.addEventListener('wheel', function(e) { console.log(e, e.defaultPrevented); }); 

Is this perhaps something to do with touchpad latching and DOM node recycling done in infinite list implementations like Facebook?

Comment 5 Deleted

Seems to work fine on Mac, so perhaps this is triggered somehow by the different touchpad fling implementation in ChromeOS?  Using:

document.addEventListener('mousewheel', e => {console.log(e);}, {capture:true});

I don't see wheel events get delivered at all during the fling, so it definitely seems like a chromium bug somewhere causing the fling to be canceled.

Oh, I can reproduce this on any site by adding a passive wheel listener:

document.documentElement.addEventListener('mousewheel', e=>{}, {passive:true});

Perhaps our lack of wheel ACK in the passive wheel listener case is triggering the ChromeOS fling-cancelling logic or something?

Reproduced on 58.0.3029.78 beta
Owner: dtapu...@chromium.org
Summary: Fling scrolling doesn't work on ChromeOS when there is a passive wheel listener (was: Fling scrolling doesn't work on lots of websites on ChromeOS)
Cc: abodenha@chromium.org
Labels: -Type-Bug Type-Bug-Regression
Summary: Fling scrolling doesn't work on ChromeOS when there is a passive wheel listener (eg. Facebook) (was: Fling scrolling doesn't work on ChromeOS when there is a passive wheel listener)
/cc abodenha since this seems like a pretty serious ChromeOS-specific regression to me.  I don't think we have any easy way to do ChromeOS-specific per-commit build bisects.
Cc: adlr@chromium.org bokan@chromium.org
+adlr and bokan for their thoughts on touch and scroll.
Cc: dtapu...@chromium.org
Labels: Hotlist-Input-Dev
Owner: sahel@chromium.org
Status: Assigned (was: Untriaged)
sahel@ do you think you could have a look at this?
I could reproduce the issue on 60.0.3078.0 dev as well, but when threaded_scrolling is disabled fling happens. It looks like a compositor side problem. Still working on in to find the cause.
Reproduced on 54.0.2840.53 official build.
I couldn't go back any further since this was the oldest version that I could get from here: https://pantheon.corp.google.com/storage/browser/chromeos-image-archive/samus-release

rbyers@ could you please give an example website on which you could reproduce the bug by adding a passive mousewheel listener?
I checked the old devices that we have in the office, could reproduce it on M52.0.2743.116 (official build) too. However, on 60.0.3078.0 (developer build) I could reproduce it non-deterministically.

Maybe we could reduce the priority since it has been there at least since M52.
The issue happens when both passive and blocking wheel event listeners exist on a webpage. This link is a simplified case that reproduces the problem: http://output.jsbin.com/bomiwoy/quiet Deleting either of the listeners avoids the problem.

I couldn't reproduce the issue on 60.0.3078.0 (Developer Build). Occasional abrupt scroll terminations that I see on facebook.com is due to processing a fling cancel event.

esprehn@ could you please confirm if you can still see the issue on M60? 
Yes, that is the fix. I commented that line and could reproduce the issue.
Labels: -Pri-1 Pri-2
I've confirmed that issue is in M59. So I think we are good here that passive event listeners don't cause the fling to stop.

Moving to a P2, I think the only thing we need to figure out is why we are generating a Fling Cancel on facebook but I imagine this is due to their infinite scroller.
Blockedon: 708236
Summary: Fling scrolling doesn't work on Facebook on ChromeOS (was: Fling scrolling doesn't work on ChromeOS when there is a passive wheel listener (eg. Facebook))
Ok so the issue I reproduced is really  issue 708236  and not connected to the facebook behavior at all?
Yes Rick.. It is fixed in M59 and sahel@ is still investigating facebook.
Abrupt scrollEnds occasionally happen because of a race condition. On ChromeOS wheel events are generated from ui:ET_SCROLL events, and GSU events are generated from wheel events if their ack is not consumed; But, GFS events are generated from ui::ET_SCROLL_FLING_START events directly. Different paths and delays for generating GSU and GFS events sometimes causes a GSU from a older ET_SCROLL event to reach the GestureEventQueue after a GFS from a more recent ET_SCROLL_FLING_START. This reordering causes dropping GFS from the debouncing deferral queue before adding it to GestureEventQueue.

Since the race condition is not common (and it will be even less common when async wheel events with immediate acks are implemented) I leave it as it is for now. After discussing with tdresser@ and dtapuska@, one potential solution is to move the fling curve generation higher and generate wheel events with phase info similar to fling on Mac. This will also help with cleaning up parts of the fling handling that is different between trackpad and Touchscreen.
To confirm you're saying that M60 should have this fixed? The current behavior on Facebook is 100% of the time, so it doesn't seem to be an "uncommon race".
The change in #15 reduces this to a 100% occurrence to a not frequent occurrence.
There is a bug in the browser where using the touchpad scroll gesture to navigate back/forward will get "stuck" between two pages.  I found this bug has the same root cause as explained in #21.

It is not that common, but it is obvious and disruptive when it happens.  I plan on adding a special case to the GestureEventQueue bounce filter for GFS, since they shouldn't be ignored.
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 20 2017

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

commit 29786c71a84fc576bc6a861a1b91d830e2f686c9
Author: Sean O'Brien <seobrien@google.com>
Date: Thu Jul 20 19:35:14 2017

Don't filter GestureFlingStart in |GestureEventQueue|

Scroll events from the touchpad may arrive in the |GestureEventQueue|
out of order, such that a GestureFlingStart arrives before a
GestureScrollUpdate.  Currently, the debounce filter removes the
GestureFlingStart.

This CL will forward the GestureFlingStart after emptying the
debounce queue.

BUG= 717205 

Change-Id: I42bd147195b9a2be8e34391c534c625cc785e2e0
Reviewed-on: https://chromium-review.googlesource.com/553424
Commit-Queue: Sean O'Brien <seobrien@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488353}
[modify] https://crrev.com/29786c71a84fc576bc6a861a1b91d830e2f686c9/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/29786c71a84fc576bc6a861a1b91d830e2f686c9/content/browser/renderer_host/input/gesture_event_queue_unittest.cc

Cc: rbyers@chromium.org sahel@chromium.org
 Issue 749723  has been merged into this issue.
Status: Fixed (was: Assigned)
Project Member

Comment 28 by bugdroid1@chromium.org, Nov 30 2017

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

commit 2dc65f0d49b20c16af3d00eb1585d0ebf8b14d96
Author: Sean O'Brien <seobrien@google.com>
Date: Thu Nov 30 15:56:47 2017

Filter GestureScrollUpdate during scroll fling

In |GestureEventQueue|, filter GestureScrollUpdate events if a scroll
fling is ongoing.  Scroll events from the touchpad sometimes arrive in
the |GestureEventQueue| out of order, and a GestureScrollUpdate may
arrive after a GestureFlingStart. These should be filtered so we don't
have a GestureScrollUpdate without a subsequent GestureFlingStart.

BUG= 717205 

Change-Id: I14bf134736269b323d441c5f670601f0d50ed03f
Reviewed-on: https://chromium-review.googlesource.com/727464
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sean O'Brien <seobrien@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520556}
[modify] https://crrev.com/2dc65f0d49b20c16af3d00eb1585d0ebf8b14d96/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/2dc65f0d49b20c16af3d00eb1585d0ebf8b14d96/content/browser/renderer_host/input/gesture_event_queue_unittest.cc

Comment 29 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 30 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Project Member

Comment 31 by bugdroid1@chromium.org, Apr 17 2018

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

commit fe4b9ea5050c23b5eb03258412f02ef42798084e
Author: Sahel Sharify <sahel@chromium.org>
Date: Tue Apr 17 14:48:12 2018

Revert "Filter GestureScrollUpdate during scroll fling."

Original cl reviewed on https://chromium-review.googlesource.com/727464

Reason for revert: The original cl causes the following regression and
with browser side fling and async wheel events it's not needed anymore.

Regression: Touchpad scrolling disables wheel scrolling since GSU events
generated from wheel scrolling get filtered here:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/gesture_event_queue.cc?q=Gesture_Event_queue&dr=C&l=130

fling_in_progress_ gets reset on GFC which is received when the user
puts their fingers on touchpad, so if the user scrolls with touchpad
first and then starts a subsequent wheel scrolling no GFC will be
received after the GFS and fling_in_progress_ will be still true while
GSU events from wheel scrolling are arriving.

Bug:  817479 ,  717205 
Change-Id: I3997f756430df432aab37e5af77e1409dc8e0225
Reviewed-on: https://chromium-review.googlesource.com/1014415
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sahel Sharifymoghaddam <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551330}
[modify] https://crrev.com/fe4b9ea5050c23b5eb03258412f02ef42798084e/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/fe4b9ea5050c23b5eb03258412f02ef42798084e/content/browser/renderer_host/input/gesture_event_queue_unittest.cc

Comment 32 by sahel@chromium.org, Apr 17 2018

I keep the bug as fixed since with browser side fling and async wheel events the fix in comment #28 is not needed anymore.

Comment 33 by adlr@chromium.org, Apr 18 2018

This should be picked back to 67 right?

Comment 34 by sahel@chromium.org, Apr 18 2018

Added M67 merge request to 817479 since that bug had a clearer reason for the merge request.
Project Member

Comment 35 by bugdroid1@chromium.org, Apr 26 2018

Labels: merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cf859e1cadf1ece236aa114715a9be9de3d364a0

commit cf859e1cadf1ece236aa114715a9be9de3d364a0
Author: Sahel Sharify <sahel@chromium.org>
Date: Thu Apr 26 17:05:44 2018

Revert "Filter GestureScrollUpdate during scroll fling."

Original cl reviewed on https://chromium-review.googlesource.com/727464

Reason for revert: The original cl causes the following regression and
with browser side fling and async wheel events it's not needed anymore.

Regression: Touchpad scrolling disables wheel scrolling since GSU events
generated from wheel scrolling get filtered here:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/gesture_event_queue.cc?q=Gesture_Event_queue&dr=C&l=130

fling_in_progress_ gets reset on GFC which is received when the user
puts their fingers on touchpad, so if the user scrolls with touchpad
first and then starts a subsequent wheel scrolling no GFC will be
received after the GFS and fling_in_progress_ will be still true while
GSU events from wheel scrolling are arriving.

Bug:  817479 ,  717205 
Change-Id: I3997f756430df432aab37e5af77e1409dc8e0225
Reviewed-on: https://chromium-review.googlesource.com/1014415
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sahel Sharifymoghaddam <sahel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551330}(cherry picked from commit fe4b9ea5050c23b5eb03258412f02ef42798084e)
Reviewed-on: https://chromium-review.googlesource.com/1030575
Reviewed-by: Sahel Sharifymoghaddam <sahel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#327}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/cf859e1cadf1ece236aa114715a9be9de3d364a0/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/cf859e1cadf1ece236aa114715a9be9de3d364a0/content/browser/renderer_host/input/gesture_event_queue_unittest.cc

Project Member

Comment 36 by bugdroid1@chromium.org, May 2 2018

Labels: merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/144d151e97463940eff5d36ee76c95925d4b8fa7

commit 144d151e97463940eff5d36ee76c95925d4b8fa7
Author: Sahel Sharify <sahel@chromium.org>
Date: Wed May 02 19:43:37 2018

Merge the fix for  crbug.com/817479  to M66.

To make sure that the merge doesn't break anything, I cherry picked the
fix manually, did a clean build and ran content_unittests locally.

Revert "Filter GestureScrollUpdate during scroll fling."

Original cl reviewed on https://chromium-review.googlesource.com/727464

Reason for revert: The original cl causes the following regression and
with browser side fling and async wheel events it's not needed anymore.

Regression: Touchpad scrolling disables wheel scrolling since GSU events
generated from wheel scrolling get filtered here:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/gesture_event_queue.cc?q=Gesture_Event_queue&dr=C&l=130

fling_in_progress_ gets reset on GFC which is received when the user
puts their fingers on touchpad, so if the user scrolls with touchpad
first and then starts a subsequent wheel scrolling no GFC will be
received after the GFS and fling_in_progress_ will be still true while
GSU events from wheel scrolling are arriving.

(cherry picked from commit fe4b9ea5050c23b5eb03258412f02ef42798084e)
TBR=dtapuska@chromium.org

Bug:  817479 ,  717205 
Change-Id: I405969389ed51bea5c64dfe82825d3e6c5840624
Reviewed-on: https://chromium-review.googlesource.com/1014415
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sahel Sharifymoghaddam <sahel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551330}
Reviewed-on: https://chromium-review.googlesource.com/1040646
Reviewed-by: Sahel Sharifymoghaddam <sahel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#787}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/144d151e97463940eff5d36ee76c95925d4b8fa7/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/144d151e97463940eff5d36ee76c95925d4b8fa7/content/browser/renderer_host/input/gesture_event_queue_unittest.cc

Sign in to add a comment