New issue
Advanced search Search tips

Issue 897901 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression
M70



Sign in to add a comment

High precision touchpad scrolling on an oopif stops once the user lifts their fingers.

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

Issue description

What steps will reproduce the problem?
(1)Enable site isolation
(2)navigate to https://jakearchibald.com/2015/tasks-microtasks-queues-and-schedules/ and scroll until you get to the comments section
(3)Use a high precision touchpad (e.i. touchpad on Microsoft Surface Pro.) to scroll on the comments section

What is the expected result?
The page should continue to scroll for a while after you lift your finger from the touchpad.

What happens instead?
The page stops scrolling abruptly the moment that you lift your finger from the touchpad.

Note: This does not happen with mouse wheels or normal touchpads, you need to scroll by high a precision touchpad for the repo.

This is a regression due to the cl landed in r599781


 
Labels: FoundIn-M71 FoundIn-M72
Since r599781 is the cause of this regression and we want to merge r599781 to M70, (see  crbug.com/884728 )
we should make sure that the fix for this issue is also merged to M70.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 22

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

commit e8259922112c02c7911960f5583d1b73a6da3167
Author: Sahel Sharify <sahel@chromium.org>
Date: Mon Oct 22 23:01:38 2018

GSU events with touchpad source and inertial phase should get bubbled.

This cl is a fix for the following cl:
https://chromium-review.googlesource.com/c/chromium/src/+/1281687

The original cl fixes the  crbug.com/884728  by making sure that
the GSU/GSE events with inertial phase are not bubbled to the parent.
In the original cl Mac is an exception since on this platform the GSU
events with inertial phase are received from the OS rather than being
generated by the fling controller.

What the original cl is missing is that Mac is not the only platform
on which we receive GSU events with inertial phase from OS. On Windows
devices with high precision touchpad the same thing happens.

This cl makes sure that we only stop bubbling the GSU/GSE events that are
generated from the fling controller: i.e. GSU/GSE events with inertial
phase that are either from touchscreen source or from touchpad source
but on ChromeOS only. (touchpad fling happens on chromeOS only, and on Mac
and on Windows(high precision touchpad) GSU events with inertial phase are
not generated by the fling controller.)

Bug:  884728 ,  897901 
Change-Id: I5d620cc147707f1727c938a868a4082639095568
Reviewed-on: https://chromium-review.googlesource.com/c/1294398
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601765}
[modify] https://crrev.com/e8259922112c02c7911960f5583d1b73a6da3167/content/browser/renderer_host/input/fling_browsertest.cc
[modify] https://crrev.com/e8259922112c02c7911960f5583d1b73a6da3167/content/browser/renderer_host/render_widget_host_view_child_frame.cc

I checked the fix in comment #2 on today's Canary( 72.0.3589.0) on a Microsoft Surface device and confirm that the landed cl resolves the issue while still preserving the fix for  crbug.com/884728  which is landed in r599781

Merge request for M70 since this cl fixes the issue that is caused by r599781 and we want to merge r599781 to M70 (see  crbug.com/884728 )
I also confirm that this cl does not break r599781 on Mac and ChromeOS and Canary/ToT with r599781 and r601765 have the correct behavior on all platforms.
Labels: Merge-Request-70
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 23

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-70 Merge-Approved-70
Labels: Merge-Request-71
Labels: -Merge-Request-71 Merge-Approved-71
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 23

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5e5bad16455f4ec8e56246e9bf9179092170fdc

commit e5e5bad16455f4ec8e56246e9bf9179092170fdc
Author: Sahel Sharify <sahel@chromium.org>
Date: Tue Oct 23 16:35:05 2018

cherry pick r601765 to M70

This cl merges the following cl to M70:
https://chromium-review.googlesource.com/c/1294398
The cherry pick is locally built and tested on M70. Gerrit cherry pick was not possible
since the fling browsertests landed before r601765 caused merge conflicts.
This cl only picks the fix that is added to render_widget_host_view_child_frame.cc
and leaves fling_browsertests.cc untouched to resolve the conflict.

TBR=creis@chromium.org,mcnee@chromium.org

Bug:  897901 
Change-Id: Iacdaa89cfd30454689e6fec181c1da1b5bf4c7c5
Reviewed-on: https://chromium-review.googlesource.com/c/1296825
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#1037}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/e5e5bad16455f4ec8e56246e9bf9179092170fdc/content/browser/renderer_host/render_widget_host_view_child_frame.cc

Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/e5e5bad16455f4ec8e56246e9bf9179092170fdc

Commit: e5e5bad16455f4ec8e56246e9bf9179092170fdc
Author: sahel@chromium.org
Commiter: sahel@chromium.org
Date: 2018-10-23 16:35:05 +0000 UTC

cherry pick r601765 to M70

This cl merges the following cl to M70:
https://chromium-review.googlesource.com/c/1294398
The cherry pick is locally built and tested on M70. Gerrit cherry pick was not possible
since the fling browsertests landed before r601765 caused merge conflicts.
This cl only picks the fix that is added to render_widget_host_view_child_frame.cc
and leaves fling_browsertests.cc untouched to resolve the conflict.

TBR=creis@chromium.org,mcnee@chromium.org

Bug:  897901 
Change-Id: Iacdaa89cfd30454689e6fec181c1da1b5bf4c7c5
Reviewed-on: https://chromium-review.googlesource.com/c/1296825
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#1037}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Please dont forget to merge to M71 :)
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 23

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b63c2547e462a21002d1339ee848bd2b31e25665

commit b63c2547e462a21002d1339ee848bd2b31e25665
Author: Sahel Sharify <sahel@chromium.org>
Date: Tue Oct 23 17:40:57 2018

cherry pick r601765 to M71

This cl merges the following cl to M71:
https://chromium-review.googlesource.com/c/1294398
The cherry pick is locally built and tested on M71. Gerrit cherry pick was not possible
since the fling browsertests landed before r601765 caused merge conflicts.
This cl only picks the fix that is added to render_widget_host_view_child_frame.cc
and leaves fling_browsertests.cc untouched to resolve the conflict.

TBR=creis@chromium.org,mcnee@chromium.org

Bug:  897901 
Change-Id: I7f3f8b581ddbd78c3de538b87c40c1e535f2dfd3
Reviewed-on: https://chromium-review.googlesource.com/c/1297077
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#268}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/b63c2547e462a21002d1339ee848bd2b31e25665/content/browser/renderer_host/render_widget_host_view_child_frame.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/b63c2547e462a21002d1339ee848bd2b31e25665

Commit: b63c2547e462a21002d1339ee848bd2b31e25665
Author: sahel@chromium.org
Commiter: sahel@chromium.org
Date: 2018-10-23 17:40:57 +0000 UTC

cherry pick r601765 to M71

This cl merges the following cl to M71:
https://chromium-review.googlesource.com/c/1294398
The cherry pick is locally built and tested on M71. Gerrit cherry pick was not possible
since the fling browsertests landed before r601765 caused merge conflicts.
This cl only picks the fix that is added to render_widget_host_view_child_frame.cc
and leaves fling_browsertests.cc untouched to resolve the conflict.

TBR=creis@chromium.org,mcnee@chromium.org

Bug:  897901 
Change-Id: I7f3f8b581ddbd78c3de538b87c40c1e535f2dfd3
Reviewed-on: https://chromium-review.googlesource.com/c/1297077
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#268}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Started)
Labels: TE-Verified-M70 TE-Verified-70.0.3538.77
Able to reproduce the issue on the build without fix #72.0.3587.0 using Surface Pro Windows 10 by following steps as per comment#0.

Verified the fix on Surface Pro Windows 10, as per comment#0 on latest chrome version #70.0.3538.77.
Attaching screencast for reference.
Observed that page continues to scroll for a while after we lift the finger from the touchpad.
Hence, the fix is working as expected.
Adding the verified labels.

Thanks...!!
897901.mp4
5.7 MB View Download
Labels: TE-Verified-71 TE-Verified-71.0.3578.30
Able to reproduce the issue on chrome version# 72.0.3587.0 using Surface Pro Windows 10 by following steps as per comment#0.

Verified the fix on Windows 10 Surface Pro, as per comment#0 on latest chrome version #71.0.3538.30.
Attaching screencast for reference.
Observed that page continues to scroll for a while after we lift the finger from the touchpad.
Hence, the fix is working as expected.
Adding the verified labels.

Thanks...!!
897901_M71.mp4
8.7 MB View Download

Sign in to add a comment