I'll take a look at this today

Issue 814669 has been merged into this issue.

Strange, other content on the page doesn't have this issue i.e. text selections not in the input box work correctly.

amaralp@, any advice that might speed me up here on where to look or what the difference might be between regular text selection and those inside an input field?

Narrowed it down a bit: it seems like the touch handles are now susceptible to main thread jank. Here's a boiled down repro with alternating 1s janks:

https://bokand.github.io/bugs/812048.html

Before RLS the handles presumably get their position from CompositorFrames but it looks like with RLS we have some dependency on the main thread.

Re c5: There is also some lag in textarea's and contenteditable's (although it seems to be less pronounced). I'm also surprised that this bug is dependent on editability. I'm not familiar with "Root Layer Scrolling" but here is where the coordinates are transformed from layer coordinates to absolute coordinates:

https://cs.chromium.org/chromium/src/cc/trees/layer_tree_impl.cc?type=cs&l=2129

Thanks, that looks like a good place to start.

Ok, I think I understand what's going on. The current selection is computed in Blink on the container layer of a scroller and sent to CC.

Since LayoutView didn't used to be a scroller and had no container layer, it was effectively computed on the scrolling contents layer (since the layout view layer was basically a scrolling contents layer for the PLC's container layers). In CC, when a new CompositorFrame is generated, we apply the ToScreen transform to it which includes the frame scroll offset so the handles would stick to the content as it scrolled.

Now that LayoutView is a scroller the selection is computed relative to the container layer. This means the ToScreen transform doesn't include the scroll offset so we only update its position when we commit from Blink. This is how it works even pre-RLS for other scrollers on the page (and you can see with RLS turned off that the handles lag a bit inside a scrolling <div>).

Chris, I see you made a related change here: https://codereview.chromium.org/2620383004. There's a comment that says we explicitly wanted the container layer, do you know why that is? The immediate fix here for RLS would be to treat the LayoutView specially but it begs the question, why not just store the selection relative to the scrolling contents layer and fix this for all scrollers? The ToScreen transform should include scroll offsets for all composited scrollers, right?

I think storing relative to the scrolling contents layer makes sense. Just need
to add in the scroll offset at the end. I guess I just didn't think of this
situation.

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

commit 93d6eba0317a43e986fe1f2ba89b7557aa01949b
Author: David Bokan <bokan@chromium.org>
Date: Thu Mar 15 17:51:45 2018

Make touch handles relative to scrolling contents

Selection bounds are sent from Blink to CC as part of a commit cycle so
that we can draw touch handles for it. Currently, the selection bounds
are relative to the main graphics layer of a CompositedLayerMapping. In
the case of a scroller, this will be its clip rect - rather than its
scrolling contents layer. Unfortunately, this means that scrolling on
the compositor isn't applied as part of the ToScreen transformation on
the selection bounds so scrolling wont update the selection bounds
location until another Blink commit. This went unnoticed until now
because the root layer was not considered a scroller. The page would
paint into a document-sized layer and the compositor would provide extra
scrolling layers to handle frame scrolling. In this configuration, the
ToScreen transformation from the root layer does include the scroll
offset.

Now that root layer scrolling has been turned on, frame scrolling works
much the same as other scrollers. Thus, this shortcoming is seen on
frame scrolling also.

The solution in this CL is to move the selection bounds rect to be
relative to the scrolling contents layer - if one exists. The ToScreen
transformation done in CC will correctly compensate for any scroll
offset applied in the compositor and touch selection handles stick to
the selection as its scrolled. For the CC side, see
ComputeViewportSelectionBound in layer_tree_impl.cc

Bug:  812048 
Change-Id: I219ac3209d334ac91e9c13ffaaa51cbc9db57886
Reviewed-on: https://chromium-review.googlesource.com/963006
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543436}
[modify] https://crrev.com/93d6eba0317a43e986fe1f2ba89b7557aa01949b/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/93d6eba0317a43e986fe1f2ba89b7557aa01949b/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp
[modify] https://crrev.com/93d6eba0317a43e986fe1f2ba89b7557aa01949b/third_party/WebKit/Source/core/exported/WebFrameTest.cpp

I don't think the issue is serious enough to risk a merge at this point so marking this as fixed.

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

commit 0c5f2838e1fc903257e8c90db12bbe4f14f69474
Author: Greg Levin <glevin@chromium.org>
Date: Thu Mar 15 22:45:39 2018

Revert "Make touch handles relative to scrolling contents"

This reverts commit 93d6eba0317a43e986fe1f2ba89b7557aa01949b.

Reason for revert: Failing webkit_unit_tests:
  RenderedPositionTest.PositionInScrollableRoot
  RenderedPositionTest.PositionInScroller/
on https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/4730

Original change's description:
> Make touch handles relative to scrolling contents
> 
> Selection bounds are sent from Blink to CC as part of a commit cycle so
> that we can draw touch handles for it. Currently, the selection bounds
> are relative to the main graphics layer of a CompositedLayerMapping. In
> the case of a scroller, this will be its clip rect - rather than its
> scrolling contents layer. Unfortunately, this means that scrolling on
> the compositor isn't applied as part of the ToScreen transformation on
> the selection bounds so scrolling wont update the selection bounds
> location until another Blink commit. This went unnoticed until now
> because the root layer was not considered a scroller. The page would
> paint into a document-sized layer and the compositor would provide extra
> scrolling layers to handle frame scrolling. In this configuration, the
> ToScreen transformation from the root layer does include the scroll
> offset.
> 
> Now that root layer scrolling has been turned on, frame scrolling works
> much the same as other scrollers. Thus, this shortcoming is seen on
> frame scrolling also.
> 
> The solution in this CL is to move the selection bounds rect to be
> relative to the scrolling contents layer - if one exists. The ToScreen
> transformation done in CC will correctly compensate for any scroll
> offset applied in the compositor and touch selection handles stick to
> the selection as its scrolled. For the CC side, see
> ComputeViewportSelectionBound in layer_tree_impl.cc
> 
> Bug:  812048 
> Change-Id: I219ac3209d334ac91e9c13ffaaa51cbc9db57886
> Reviewed-on: https://chromium-review.googlesource.com/963006
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Commit-Queue: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#543436}

TBR=bokan@chromium.org,chrishtr@chromium.org

Change-Id: I7ed3e4a8494caf31772af6eae2b25e3d4d13a330
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  812048 
Reviewed-on: https://chromium-review.googlesource.com/965281
Reviewed-by: Greg Levin <glevin@chromium.org>
Commit-Queue: Greg Levin <glevin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543526}
[modify] https://crrev.com/0c5f2838e1fc903257e8c90db12bbe4f14f69474/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/0c5f2838e1fc903257e8c90db12bbe4f14f69474/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp
[modify] https://crrev.com/0c5f2838e1fc903257e8c90db12bbe4f14f69474/third_party/WebKit/Source/core/exported/WebFrameTest.cpp

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

commit 6fb0fe3f2c3d3c703b800694623b4f51ae10f717
Author: David Bokan <bokan@chromium.org>
Date: Fri Mar 16 20:20:41 2018

[Reland] Make touch handles relative to scrolling contents

Selection bounds are sent from Blink to CC as part of a commit cycle so
that we can draw touch handles for it. Currently, the selection bounds
are relative to the main graphics layer of a CompositedLayerMapping. In
the case of a scroller, this will be its clip rect - rather than its
scrolling contents layer. Unfortunately, this means that scrolling on
the compositor isn't applied as part of the ToScreen transformation on
the selection bounds so scrolling wont update the selection bounds
location until another Blink commit. This went unnoticed until now
because the root layer was not considered a scroller. The page would
paint into a document-sized layer and the compositor would provide extra
scrolling layers to handle frame scrolling. In this configuration, the
ToScreen transformation from the root layer does include the scroll
offset.

Now that root layer scrolling has been turned on, frame scrolling works
much the same as other scrollers. Thus, this shortcoming is seen on
frame scrolling also.

The solution in this CL is to move the selection bounds rect to be
relative to the scrolling contents layer - if one exists. The ToScreen
transformation done in CC will correctly compensate for any scroll
offset applied in the compositor and touch selection handles stick to
the selection as its scrolled. For the CC side, see
ComputeViewportSelectionBound in layer_tree_impl.cc

Reland Note: This patch makes the RenderedPositionTests run with mock
scrollbars.

TBR=chrishtr@chromium.org

Bug:  812048 
Change-Id: Ib9eacd14b75b71b4e0d4f0a9b57416800f9dfb91
Reviewed-on: https://chromium-review.googlesource.com/967001
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543803}
[modify] https://crrev.com/6fb0fe3f2c3d3c703b800694623b4f51ae10f717/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/6fb0fe3f2c3d3c703b800694623b4f51ae10f717/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp
[modify] https://crrev.com/6fb0fe3f2c3d3c703b800694623b4f51ae10f717/third_party/WebKit/Source/core/exported/WebFrameTest.cpp

Issue 820957 has been merged into this issue.

Thank you for the fix.

Can you also request merge to M66 since this is a regression?  The fix was verified in M67 already as per duped issue 820957.

I'm not sure this is serious enough to warrant a merge but I'll let the TPM decide whether the fix fits in our merge risk tolerance. 

The patch is fairly small and self contained but it could possibly change how touch handles get drawn in some scenarios. 

This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

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

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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/+/2940d405c166a3700bd21ea77055f339f4bd403a

commit 2940d405c166a3700bd21ea77055f339f4bd403a
Author: David Bokan <bokan@chromium.org>
Date: Mon Mar 26 19:38:00 2018

[Reland] Make touch handles relative to scrolling contents

Selection bounds are sent from Blink to CC as part of a commit cycle so
that we can draw touch handles for it. Currently, the selection bounds
are relative to the main graphics layer of a CompositedLayerMapping. In
the case of a scroller, this will be its clip rect - rather than its
scrolling contents layer. Unfortunately, this means that scrolling on
the compositor isn't applied as part of the ToScreen transformation on
the selection bounds so scrolling wont update the selection bounds
location until another Blink commit. This went unnoticed until now
because the root layer was not considered a scroller. The page would
paint into a document-sized layer and the compositor would provide extra
scrolling layers to handle frame scrolling. In this configuration, the
ToScreen transformation from the root layer does include the scroll
offset.

Now that root layer scrolling has been turned on, frame scrolling works
much the same as other scrollers. Thus, this shortcoming is seen on
frame scrolling also.

The solution in this CL is to move the selection bounds rect to be
relative to the scrolling contents layer - if one exists. The ToScreen
transformation done in CC will correctly compensate for any scroll
offset applied in the compositor and touch selection handles stick to
the selection as its scrolled. For the CC side, see
ComputeViewportSelectionBound in layer_tree_impl.cc

Reland Note: This patch makes the RenderedPositionTests run with mock
scrollbars.

TBR=chrishtr@chromium.org

Bug:  812048 
Change-Id: Ib9eacd14b75b71b4e0d4f0a9b57416800f9dfb91
Reviewed-on: https://chromium-review.googlesource.com/967001
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543803}(cherry picked from commit 6fb0fe3f2c3d3c703b800694623b4f51ae10f717)
Reviewed-on: https://chromium-review.googlesource.com/981014
Cr-Commit-Position: refs/branch-heads/3359@{#449}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/2940d405c166a3700bd21ea77055f339f4bd403a/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/2940d405c166a3700bd21ea77055f339f4bd403a/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp
[modify] https://crrev.com/2940d405c166a3700bd21ea77055f339f4bd403a/third_party/WebKit/Source/core/exported/WebFrameTest.cpp

Please revert the cl in #23. It seems to be the culprit for  https://bugs.chromium.org/p/chromium/issues/detail?id=826065

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

commit 6c261d0bfdd9d15d254f56e855a3bb30ba4b1524
Author: Estelle Yomba <cmasso@chromium.org>
Date: Tue Mar 27 00:13:35 2018

Revert "[Reland] Make touch handles relative to scrolling contents"

This reverts commit 2940d405c166a3700bd21ea77055f339f4bd403a.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> [Reland] Make touch handles relative to scrolling contents
> 
> Selection bounds are sent from Blink to CC as part of a commit cycle so
> that we can draw touch handles for it. Currently, the selection bounds
> are relative to the main graphics layer of a CompositedLayerMapping. In
> the case of a scroller, this will be its clip rect - rather than its
> scrolling contents layer. Unfortunately, this means that scrolling on
> the compositor isn't applied as part of the ToScreen transformation on
> the selection bounds so scrolling wont update the selection bounds
> location until another Blink commit. This went unnoticed until now
> because the root layer was not considered a scroller. The page would
> paint into a document-sized layer and the compositor would provide extra
> scrolling layers to handle frame scrolling. In this configuration, the
> ToScreen transformation from the root layer does include the scroll
> offset.
> 
> Now that root layer scrolling has been turned on, frame scrolling works
> much the same as other scrollers. Thus, this shortcoming is seen on
> frame scrolling also.
> 
> The solution in this CL is to move the selection bounds rect to be
> relative to the scrolling contents layer - if one exists. The ToScreen
> transformation done in CC will correctly compensate for any scroll
> offset applied in the compositor and touch selection handles stick to
> the selection as its scrolled. For the CC side, see
> ComputeViewportSelectionBound in layer_tree_impl.cc
> 
> Reland Note: This patch makes the RenderedPositionTests run with mock
> scrollbars.
> 
> TBR=chrishtr@chromium.org
> 
> Bug:  812048 
> Change-Id: Ib9eacd14b75b71b4e0d4f0a9b57416800f9dfb91
> Reviewed-on: https://chromium-review.googlesource.com/967001
> Reviewed-by: David Bokan <bokan@chromium.org>
> Commit-Queue: David Bokan <bokan@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#543803}(cherry picked from commit 6fb0fe3f2c3d3c703b800694623b4f51ae10f717)
> Reviewed-on: https://chromium-review.googlesource.com/981014
> Cr-Commit-Position: refs/branch-heads/3359@{#449}
> Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}

TBR=bokan@chromium.org

Change-Id: I2a9cf3c6a3749475b5f47d93bdbe2472ad1faa52
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  812048 
Reviewed-on: https://chromium-review.googlesource.com/981155
Reviewed-by: Estelle Yomba <cmasso@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#456}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/6c261d0bfdd9d15d254f56e855a3bb30ba4b1524/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/6c261d0bfdd9d15d254f56e855a3bb30ba4b1524/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp
[modify] https://crrev.com/6c261d0bfdd9d15d254f56e855a3bb30ba4b1524/third_party/WebKit/Source/core/exported/WebFrameTest.cpp

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

commit 41ea010be90fd283bfef02d5aef53b26b64fb6f2
Author: David Bokan <bokan@chromium.org>
Date: Tue Mar 27 13:45:00 2018

[Merge] Make touch handles relative to scrolling contents

Selection bounds are sent from Blink to CC as part of a commit cycle so
that we can draw touch handles for it. Currently, the selection bounds
are relative to the main graphics layer of a CompositedLayerMapping. In
the case of a scroller, this will be its clip rect - rather than its
scrolling contents layer. Unfortunately, this means that scrolling on
the compositor isn't applied as part of the ToScreen transformation on
the selection bounds so scrolling wont update the selection bounds
location until another Blink commit. This went unnoticed until now
because the root layer was not considered a scroller. The page would
paint into a document-sized layer and the compositor would provide extra
scrolling layers to handle frame scrolling. In this configuration, the
ToScreen transformation from the root layer does include the scroll
offset.

Now that root layer scrolling has been turned on, frame scrolling works
much the same as other scrollers. Thus, this shortcoming is seen on
frame scrolling also.

The solution in this CL is to move the selection bounds rect to be
relative to the scrolling contents layer - if one exists. The ToScreen
transformation done in CC will correctly compensate for any scroll
offset applied in the compositor and touch selection handles stick to
the selection as its scrolled. For the CC side, see
ComputeViewportSelectionBound in layer_tree_impl.cc

Reland Note: This patch makes the RenderedPositionTests run with mock
scrollbars.

Merge Note: RuntimeEnabledFeaturesTestHelpers.h was renamed after
branch.

TBR=chrishtr@chromium.org

(cherry picked from commit 2940d405c166a3700bd21ea77055f339f4bd403a)

Bug:  812048 
Change-Id: If404af9e42197f82bcbc9899c37d8fa4f78d6143
Reviewed-on: https://chromium-review.googlesource.com/967001
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#543803}(cherry picked from commit 6fb0fe3f2c3d3c703b800694623b4f51ae10f717)
Reviewed-on: https://chromium-review.googlesource.com/981014
Cr-Original-Commit-Position: refs/branch-heads/3359@{#449}
Cr-Original-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
Reviewed-on: https://chromium-review.googlesource.com/982008
Cr-Commit-Position: refs/branch-heads/3359@{#462}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/41ea010be90fd283bfef02d5aef53b26b64fb6f2/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/41ea010be90fd283bfef02d5aef53b26b64fb6f2/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp
[modify] https://crrev.com/41ea010be90fd283bfef02d5aef53b26b64fb6f2/third_party/WebKit/Source/core/exported/WebFrameTest.cpp

Works as per expected behavior , Issue verified on 66.0.3359.67 and 67.0.3381.2