New issue
Advanced search Search tips

Issue 878904 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 836890



Sign in to add a comment

[blink-gen-property-trees] fast/scroll-snap/snap-scrolls-visual-viewport.html fails

Project Member Reported by chaopeng@chromium.org, Aug 29

Issue description

Run the test with BGPT flag:

third_party/blink/tools/run_web_tests.py -t Default --additional-driver-flag=--enable-blink-gen-property-trees --skipped=ignore --no-retry-failures fast/scroll-snap/snap-scrolls-visual-viewport.html

Log:

PASS Snap scrolls visual viewport when it's scrollable.
PASS Snap scrolls layout viewport when visual viewport has reached its scroll extent.
FAIL Snap scrolls both layout and visual viewports when visual viewport is scrollable but does not has enough room to reach the snap position. assert_approx_equals: expected 300 +/- 1 but got 293
 
I have do some investigation. It seems this test runs with bgpt or spv2 flag some how consider the scrollbar width/height.

I can make it pass by changing the css: body { overflow: overlay; }

with overflow: overlay, scrollbar does not taking the space of content.


Status: Assigned (was: Untriaged)
Cc: -bokan@chromium.org sunyunjia@chromium.org
Labels: -Pri-3 Pri-2
Owner: bokan@chromium.org
Status: Fixed (was: Assigned)
Not sure why commit bot didn't catch it but the fix landed:

commit	6b1bad395f50417ca241143e8bebf7cf8b1d47d6
author	David Bokan <bokan@chromium.org>	Tue Sep 04 17:00:17 2018

[blink-gen-property-trees] Fix viewport scroll extents

The viewport's scroll node's container and contents rects must match the
layout viewport's size. However, we were incorrectly excluding scrollbars
from these rects. This doesn't make a difference when we're not zoomed in
(since the sizes are the same), but when we do zoom in - the max scroll
offset calculation on the compositor side would then incorrectly include
a scaled scrollbar in the max offset calculation, making the max scroll
offset too small since the scrollbars don't actually scale when we zoom.

Bug:  878904 
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I2b37838874af61b10c0628c8fbd54409140dc962
Reviewed-on: https://chromium-review.googlesource.com/1199609
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588553}
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 5

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

commit 6b1bad395f50417ca241143e8bebf7cf8b1d47d6
Author: David Bokan <bokan@chromium.org>
Date: Tue Sep 04 17:00:17 2018

[blink-gen-property-trees] Fix viewport scroll extents

The viewport's scroll node's container and contents rects must match the
layout viewport's size. However, we were incorrectly excluding scrollbars
from these rects. This doesn't make a difference when we're not zoomed in
(since the sizes are the same), but when we do zoom in - the max scroll
offset calculation on the compositor side would then incorrectly include
a scaled scrollbar in the max offset calculation, making the max scroll
offset too small since the scrollbars don't actually scale when we zoom.

Bug:  878904 
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I2b37838874af61b10c0628c8fbd54409140dc962
Reviewed-on: https://chromium-review.googlesource.com/1199609
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588553}
[modify] https://crrev.com/6b1bad395f50417ca241143e8bebf7cf8b1d47d6/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
[modify] https://crrev.com/6b1bad395f50417ca241143e8bebf7cf8b1d47d6/third_party/WebKit/LayoutTests/fast/scroll-snap/snap-scrolls-visual-viewport.html
[modify] https://crrev.com/6b1bad395f50417ca241143e8bebf7cf8b1d47d6/third_party/blink/renderer/core/frame/visual_viewport.cc
[modify] https://crrev.com/6b1bad395f50417ca241143e8bebf7cf8b1d47d6/third_party/blink/renderer/core/paint/paint_property_tree_update_tests.cc

Sign in to add a comment