New issue
Advanced search Search tips

Issue 781431 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 711468



Sign in to add a comment

[root layer scrolls] failures in fast/events/...

Project Member Reported by skobes@chromium.org, Nov 3 2017

Issue description

The following 14 layout tests fail or timeout with RLS enabled:

  fast/events/autoscroll.html
  fast/events/clientXY-in-zoom-and-scroll.html
  fast/events/drag-and-drop-autoscroll-frameset.html
  fast/events/drag-and-drop-autoscroll-inner-frame.html
  fast/events/drag-and-drop-autoscroll-mainframe.html
  fast/events/pointerevents/pointer-event-mouse-coords-in-zoom-and-scroll.html
  fast/events/pointerevents/pointer-event-pen-coords-in-zoom-and-scroll.html
  fast/events/scale-and-scroll-iframe-window.html
  fast/events/touch/compositor-touch-hit-rects-iframes.html
  fast/events/touch/gesture/gesture-tap-frame-scrolled.html
  fast/events/touch/gesture/gesture-tap-scrolled.html
  fast/events/touch/touch-coords-in-zoom-and-scroll.html
  fast/events/touch/touch-fractional-coordinates.html
  fast/events/touch/touch-inside-iframe-scrolled.html
 

Comment 1 by bokan@chromium.org, Nov 30 2017

Owner: bokan@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 1 2017

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

commit 014a0817025835ff131546c81dc7a4d81510d07d
Author: David Bokan <bokan@chromium.org>
Date: Fri Dec 01 02:16:09 2017

Modernize autoscroll.html test

This test was "failing" in root-layer-scrolls mode but it wasn't related
to what this test is checking. Since this test just dumps the layer tree
it was reflecting changes to where scroll offsets are reported in the
tree.

I've rewritten the test to use testharness.js removing all expectation
files and replacing them with a simple assertion that the scroll offset
isn't changed by drag selecting text.

Bug:  781431 
Change-Id: I8391caeea794cbdaebfb2b7b86e5c3d9da6268fa
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_root_layer_scrolls
Reviewed-on: https://chromium-review.googlesource.com/802178
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520803}
[modify] https://crrev.com/014a0817025835ff131546c81dc7a4d81510d07d/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/014a0817025835ff131546c81dc7a4d81510d07d/third_party/WebKit/LayoutTests/fast/events/autoscroll.html
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/linux/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/linux/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/linux/virtual/pointerevent/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/linux/virtual/pointerevent/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/linux/virtual/trustedeventsdefaultaction/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/linux/virtual/trustedeventsdefaultaction/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/mouseevent_fractional/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/mouseevent_fractional/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/pointerevent/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/pointerevent/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/trustedeventsdefaultaction/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/trustedeventsdefaultaction/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.11/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.11/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/mouseevent_fractional/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/mouseevent_fractional/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac/virtual/mouseevent_fractional/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac/virtual/mouseevent_fractional/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac/virtual/pointerevent/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac/virtual/pointerevent/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac/virtual/trustedeventsdefaultaction/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/mac/virtual/trustedeventsdefaultaction/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/win/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/win/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/win/virtual/pointerevent/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/win/virtual/pointerevent/fast/events/autoscroll-expected.txt
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/win/virtual/trustedeventsdefaultaction/fast/events/autoscroll-expected.png
[delete] https://crrev.com/52054ad2f6ad8fc7257962285c80a29271864e4a/third_party/WebKit/LayoutTests/platform/win/virtual/trustedeventsdefaultaction/fast/events/autoscroll-expected.txt

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 2 2017

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

commit d7d2d8579bb3599140f83bcb45e5d8acc82ad37e
Author: David Bokan <bokan@chromium.org>
Date: Sat Dec 02 00:59:30 2017

Fix DnD autoscroll under root-layer-scrolls

The drag and drop autoscroll tests were timing out under
root-layer-scrolls but autoscroll itself worked. The issue was that the
LayoutView now owns the scrollbars when RLS is turned on so its
bounding box is larger. This meant that the autoscroll activation region
was further out so the test never activated autoscroll.

This test fixes the issue by excluding the scrollbars when calculating
the autoscroll regions. This means that the activation area starts from
the scrollbar-content edge, rather than the window edge.

Bug:  781431 
Change-Id: Ifc0e03d8e5016055586f6242feddc7af50f90069
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_root_layer_scrolls
Reviewed-on: https://chromium-review.googlesource.com/802519
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521162}
[modify] https://crrev.com/d7d2d8579bb3599140f83bcb45e5d8acc82ad37e/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/d7d2d8579bb3599140f83bcb45e5d8acc82ad37e/third_party/WebKit/Source/core/layout/LayoutBox.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 4 2017

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

commit 4ca1c620a075f74d526126a0cb411217b0ba417a
Author: David Bokan <bokan@chromium.org>
Date: Mon Dec 04 01:12:25 2017

Make mouse events aware of root layer scrolls

MouseEvents are initialized with client coordinates and translated into
offset and page coordinates based on the main frame's scroll offset.
This patch fixes the scroll offset calculation for when
root-layer-scrolls is turned on.

Bug:  781431 
Change-Id: Ibdbb24600ff6f0ded589b9e4a31be8bf9e753cb5
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_root_layer_scrolls
Reviewed-on: https://chromium-review.googlesource.com/798537
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521246}
[modify] https://crrev.com/4ca1c620a075f74d526126a0cb411217b0ba417a/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/4ca1c620a075f74d526126a0cb411217b0ba417a/third_party/WebKit/LayoutTests/fast/events/clientXY-in-zoom-and-scroll-expected.txt
[modify] https://crrev.com/4ca1c620a075f74d526126a0cb411217b0ba417a/third_party/WebKit/LayoutTests/fast/events/clientXY-in-zoom-and-scroll.html
[modify] https://crrev.com/4ca1c620a075f74d526126a0cb411217b0ba417a/third_party/WebKit/Source/core/events/MouseEvent.cpp
[modify] https://crrev.com/4ca1c620a075f74d526126a0cb411217b0ba417a/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/4ca1c620a075f74d526126a0cb411217b0ba417a/third_party/WebKit/Source/core/frame/LocalFrameView.h

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 4 2017

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

commit de6781cecdd5c31629bff92dd2874dae67025481
Author: David Bokan <bokan@chromium.org>
Date: Mon Dec 04 06:22:00 2017

Fix for layer coordinates in root layer scrolls

This patch fixes MouseEvent.layerX and layerY when root-layer-scrolls is
enabled. The layer coordinates are calculated by subtracting from the
document location the layer offsets of each parent layer starting from
the current layer's parent. This yields the offset within the layer.

The repeated subtraction gets us back to the absolute coordinate space.
Pre-RLS, this was the same as document/content coordinates. With RLS
turned on, the LayoutView is the same size as the frame and clips and
scrolls its content. Thus, transforming through it yields frame
coordinates. Therefore, the change is to start from the event's absolute
location. With RLS off, the conversion yields a no-op so there's no
change. When RLS is on this accounts for the LayoutView's scroll offset.

Bug:  781431 
Change-Id: Ib25859effbfb71516e5edb5173154c807ea98314
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_root_layer_scrolls
Reviewed-on: https://chromium-review.googlesource.com/798832
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521279}
[modify] https://crrev.com/de6781cecdd5c31629bff92dd2874dae67025481/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/de6781cecdd5c31629bff92dd2874dae67025481/third_party/WebKit/Source/core/events/MouseEvent.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 4 2017

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

commit 6b79dadb31f4ad4e6a03a8338f5dab1164b3ac94
Author: David Bokan <bokan@chromium.org>
Date: Mon Dec 04 11:22:13 2017

Fix touch event coodinates for root-layer-scrolls

This patch fixes client and page coordinates on touch events when root
layer scrolling is turned on. This is the touch equivalent patch of
https://crrev.com/c/798537.

Coordinates that are transformed by the viewport's scroll offset need
to account for whether that offset is represented in the FrameView
(today) or root PaintLayer's PaintLayerScrollableArea
(root-layer-scrolls enabled). Using
LocalFrameView::LayoutViewportScrollableArea uses the correct area.

Additionally, conversions based on absolute coordinates also need
adjustment since "absolute" means the root layout coodinate system.
Since RLS makes the LayoutView (the root) clip and scroll its contents,
absolute coordinates with RLS become equivalent to frame coordinates.
Prior to RLS, absolut coordinates were equivalent to "document"
coordinates since the LayoutView was the size of the entire document
and scrolling happened outside the layout hierarchy. The new conversion
methods in LayoutFrameView abstract the differences correctly so we
make use of them in this patch.

Bug:  781431 
Change-Id: Ie84d81b4bc30b468d800e0e5a2867012f7760114
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_root_layer_scrolls
Reviewed-on: https://chromium-review.googlesource.com/802142
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521311}
[modify] https://crrev.com/6b79dadb31f4ad4e6a03a8338f5dab1164b3ac94/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/6b79dadb31f4ad4e6a03a8338f5dab1164b3ac94/third_party/WebKit/Source/core/events/WebInputEventConversion.cpp
[modify] https://crrev.com/6b79dadb31f4ad4e6a03a8338f5dab1164b3ac94/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/6b79dadb31f4ad4e6a03a8338f5dab1164b3ac94/third_party/WebKit/Source/core/frame/LocalFrameView.h
[modify] https://crrev.com/6b79dadb31f4ad4e6a03a8338f5dab1164b3ac94/third_party/WebKit/Source/core/input/Touch.cpp
[modify] https://crrev.com/6b79dadb31f4ad4e6a03a8338f5dab1164b3ac94/third_party/WebKit/Source/core/input/TouchEventManager.cpp

Comment 7 by bokan@chromium.org, Jan 30 2018

Status: Started (was: Assigned)
The last remaining test here is fast/events/scale-and-scroll-iframe-window.html

I took a look and it's actually the old expectation that's technically incorrect, the root-layer-scrolls one is more correct. The issue here is that the test sets the minimum-scale to 0.5. This is an unsupported configuration outside of Android where VisualViewportSuppliesScrollbars() returns true and the main frame doesn't create scrollbars. 

What's happening without RLS is that PositionScrollbarLayer in FrameView uses the FrameRect location to position the layer. However, in PLC::UpdateOverflowControlLayers we set the parent layer to the visual viewport container (so that they don’t zoom and stay onscreen). However, if we make the minimum-scale < 1 the FrameView is expanded to the minimum-scale and the scrollbar FrameRects along with it. So the visual viewport container (which is a fixed size, 800x600 in layout tests) has scrollbars attached at an offset that takes the minimum-scale into account (1600x1200 in the layout test case).

When RLS is turned on the scrollbars are positioned correctly but produce a pixel diff since they're newly visible.

I'm going to remove the pixel expectation and fix the test to only make assertions (since the test is only checking that setting scroll offset while zoomed sets the correct offset). I'll wait until the dust settles on the Flag Flip CL.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 31 2018

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

commit 3437aff4700827da880db2c11abe0a4a088360a3
Author: David Bokan <bokan@chromium.org>
Date: Wed Jan 31 02:35:24 2018

Modernize scale-and-scroll-iframe-window.html test

This CL converts this test to use testharness.js assertions rather than
js-test.js and a pixel test. The impetus for this is the pixel diff when
root layer scrolling is turned on. The difference it not related to what
this test is checking (and is actually correct when RLS is turned on).
See the bug (comment #7) for more details.

Bug:  781431 
Change-Id: Ida7c287b616b4fd166b436e9957f28bf794098d4
Reviewed-on: https://chromium-review.googlesource.com/894365
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533170}
[delete] https://crrev.com/f8c029b4e431dac489aa6113f93dfe6abfc080a9/third_party/WebKit/LayoutTests/fast/events/scale-and-scroll-iframe-window-expected.png
[delete] https://crrev.com/f8c029b4e431dac489aa6113f93dfe6abfc080a9/third_party/WebKit/LayoutTests/fast/events/scale-and-scroll-iframe-window-expected.txt
[modify] https://crrev.com/3437aff4700827da880db2c11abe0a4a088360a3/third_party/WebKit/LayoutTests/fast/events/scale-and-scroll-iframe-window.html
[delete] https://crrev.com/f8c029b4e431dac489aa6113f93dfe6abfc080a9/third_party/WebKit/LayoutTests/platform/mac/virtual/pointerevent/fast/events/scale-and-scroll-iframe-window-expected.png
[delete] https://crrev.com/f8c029b4e431dac489aa6113f93dfe6abfc080a9/third_party/WebKit/LayoutTests/platform/win/virtual/pointerevent/fast/events/scale-and-scroll-iframe-window-expected.png

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 6 2018

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

commit fda37c408843cd094b0eedbad64686f56a2ecaf2
Author: David Bokan <bokan@chromium.org>
Date: Tue Feb 06 18:42:22 2018

Remove passing root-layer-scrolls TestExpectation

fast/events/scale-and-scroll-iframe-window.html was fixed in r533170 but
the TestExpectation line wasn't removed to avoid interfering with
landing r533694. Dashboard says it's been passing reliably.

Bug:  781431 
Change-Id: I92ff5c63246fd68556598a42b09e86477e983cdd
Reviewed-on: https://chromium-review.googlesource.com/899644
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534735}
[modify] https://crrev.com/fda37c408843cd094b0eedbad64686f56a2ecaf2/third_party/WebKit/LayoutTests/TestExpectations

Status: Fixed (was: Started)

Sign in to add a comment