New issue
Advanced search Search tips

Issue 701575 link

Starred by 1 user
Status: Verified
Owner:
szager@chromium.org
Closed: May 2017
Cc:
szager@chromium.org
cbiesin...@chromium.org
bokan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 417782
issue 695257



Sign in to add a comment

[root layer scrolls] PaintLayer::size() should match the layout viewport

Project Member Reported by skobes@chromium.org, Mar 14 2017 
In root layer scrolling mode, the root PaintLayer should be the size of the layout viewport, which may differ from the initial containing block (see  issue 492871 ).

We are currently not setting the PaintLayer size correctly in pinch zoom scenarios.  The test case in http://crrev.com/2751843002 demonstrates the issue.  An incorrect result from PaintLayer::size() affects PLSA::visibleContentRect(), and anything that depends on that.

(We had a meeting last month to discuss this; notes from that are attached.)
meeting-notes.txt
1014 bytes View Download

Comment 1 by skobes@chromium.org, Mar 14 2017

Blocking: 695257

Comment 2 by szager@chromium.org, Mar 15 2017

Owner: szager@chromium.org
I have a patch in progress to fix this.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 28 2017 

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

commit 61472254f109375173180671106927aa6867699a
Author: szager <szager@chromium.org>
Date: Fri Apr 28 14:58:28 2017

Refactor scroll updates after layout.

Every LayoutBox with a PaintLayer will do some layer updating at the end of its layout; this consists of:

- updating the layer's transform
- updating the layer's scrollable area, if necessary
- in the near future, updating the layer's size

This change consolidates all of those into LayoutBox::UpdateAfterLayout.

BUG= 701575 
R=skobes@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2839183002
Cr-Commit-Position: refs/heads/master@{#467995}

[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutBlock.h
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutBox.h
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutDeprecatedFlexibleBox.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutEmbeddedObject.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutFrameSet.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutIFrame.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutPart.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutReplaced.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutTable.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/ng/layout_ng_block_flow.cc
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/layout/svg/LayoutSVGRoot.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/paint/PaintLayer.cpp
[modify] https://crrev.com/61472254f109375173180671106927aa6867699a/third_party/WebKit/Source/core/paint/PaintLayer.h
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 28 2017 

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

commit 9309126a1a26b15343a2db4e2a5358dcaa9bd9f8
Author: szager <szager@chromium.org>
Date: Fri Apr 28 18:16:24 2017

[RootLayerScrolls] Add convenience methods for PLSA visible client size.

This anticipates when the size of the main frame's root layer is based on
the main frame's layout size.

BUG= 701575 
R=skobes@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2841113002
Cr-Commit-Position: refs/heads/master@{#468062}

[modify] https://crrev.com/9309126a1a26b15343a2db4e2a5358dcaa9bd9f8/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/9309126a1a26b15343a2db4e2a5358dcaa9bd9f8/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 28 2017 

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

commit 90f1043d53ecd259599da1af4a17ca1a1b0a3164
Author: szager <szager@chromium.org>
Date: Fri Apr 28 18:28:01 2017

Add FrameView::MarkViewportConstrainedObjectsForLayout

A minor refactoring; the method will get a couple of other callers
in subsequent patches (which I'm breaking up for digestibility).

The new condition is something that currently will never be false;
subsequent patches will cause it be false sometimes.

BUG= 701575 

Review-Url: https://codereview.chromium.org/2846263002
Cr-Commit-Position: refs/heads/master@{#468066}

[modify] https://crrev.com/90f1043d53ecd259599da1af4a17ca1a1b0a3164/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/90f1043d53ecd259599da1af4a17ca1a1b0a3164/third_party/WebKit/Source/core/frame/FrameView.h
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 29 2017 

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

commit b6c07cb297c05e54ce3865a63b9325fdd8222017
Author: szager <szager@chromium.org>
Date: Sat Apr 29 11:04:39 2017

Refactor WebView resize and update-after-layout methods

This changes some method names to make them more accurate, and splits
out ResizeAfterLayout from LayoutUpdated.  In a subsequent patch, these
two methods will be called independently through ChromeClient.

BUG= 701575 

Review-Url: https://codereview.chromium.org/2848043002
Cr-Commit-Position: refs/heads/master@{#468236}

[modify] https://crrev.com/b6c07cb297c05e54ce3865a63b9325fdd8222017/third_party/WebKit/Source/core/page/ChromeClient.h
[modify] https://crrev.com/b6c07cb297c05e54ce3865a63b9325fdd8222017/third_party/WebKit/Source/web/ChromeClientImpl.cpp
[modify] https://crrev.com/b6c07cb297c05e54ce3865a63b9325fdd8222017/third_party/WebKit/Source/web/ChromeClientImpl.h
[modify] https://crrev.com/b6c07cb297c05e54ce3865a63b9325fdd8222017/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/b6c07cb297c05e54ce3865a63b9325fdd8222017/third_party/WebKit/Source/web/WebViewImpl.h
Project Member

Comment 7 by bugdroid1@chromium.org, May 4 2017 

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

commit f240cb6e78c2d189ec6baabde91165b19b27a190
Author: szager <szager@chromium.org>
Date: Thu May 04 17:33:27 2017

Don't paint empty scroll corner

The existing logic for checking whether the compositor has a scroll
corner layer is insufficient.  When the compositor runs
UpdateOverflowControlsLayers, it will delete its scroll corner layer
if the scroll corner isn't visible.  In this case, the paint code
should not try to paint the scroll corner.

The existing code causes a number of crashes in compositing tests
when this patch is applied:

https://codereview.chromium.org/2860433002

BUG= 701575 
R=pdr@chromium.org,chrishtr@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2857333004
Cr-Commit-Position: refs/heads/master@{#469370}

[modify] https://crrev.com/f240cb6e78c2d189ec6baabde91165b19b27a190/third_party/WebKit/Source/core/paint/FramePainter.cpp
Project Member

Comment 8 by bugdroid1@chromium.org, May 12 2017 

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

commit 54a03d79877c3cacd79e37e33ec715abb3460bd7
Author: szager <szager@chromium.org>
Date: Fri May 12 11:45:00 2017

Update WebView/FrameView size from LayoutView::UpdateAfterLayout

Previously, this was set *after* layout had completed, via
FrameView::AdjustViewSize and WebViewImpl::LayoutUpdated.

The layout size of the full document must be known before the
WebView and FrameView can be resized, because the document size may
affect the minimum page scale factor, which will affect the
FrameView's size.  The earliest that the full document's layout size
is known is after the LayoutView completes its block layout.

This patch updates the FrameView size in
LayoutView::UpdateAfterLayout, which is after the LayoutView has
finished block layout, but still in time for the LayoutView to use
the new sizes when updating its scrollbars (when root layer
scrolling is enabled).

The new test expectations are just paint invalidation reasons, which
are expected; and one case (resize-iframe-text.html) where the existing
invalidation geometry was too big, and this patch results in a smaller,
correct set of invalidations.

BUG= 701575 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2860433002
Cr-Commit-Position: refs/heads/master@{#471275}

[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/paint/invalidation/window-resize-centered-inline-under-fixed-pos-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/paint/invalidation/window-resize-frameset-expected.txt
[delete] https://crrev.com/bb9147ed072a94397e8d448ba7445c8f214230f3/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/resize-iframe-text-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/window-resize-vertical-writing-mode-expected.txt
[delete] https://crrev.com/bb9147ed072a94397e8d448ba7445c8f214230f3/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/resize-iframe-text-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/window-resize-vertical-writing-mode-expected.txt
[rename] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/iframe-display-none-to-display-block-expected.txt
[rename] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/resize-iframe-text-expected.txt
[rename] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/shift-relative-positioned-container-with-image-addition-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/window-resize-vertical-writing-mode-expected.txt
[rename] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/iframe-display-none-to-display-block-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/resize-iframe-text-expected.txt
[rename] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/shift-relative-positioned-container-with-image-addition-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/window-resize-vertical-writing-mode-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/iframe-display-none-to-display-block-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/resize-iframe-text-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/shift-relative-positioned-container-with-image-addition-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/window-resize-vertical-writing-mode-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/iframe-display-none-to-display-block-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/resize-iframe-text-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/shift-relative-positioned-container-with-image-addition-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/window-resize-vertical-writing-mode-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/window-resize-centered-inline-under-fixed-pos-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/window-resize-frameset-expected.txt
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/Source/core/layout/LayoutView.cpp
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/Source/core/layout/LayoutView.h
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/Source/core/paint/BoxPaintInvalidatorTest.cpp
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/54a03d79877c3cacd79e37e33ec715abb3460bd7/third_party/WebKit/Source/web/WebViewImpl.h
Project Member

Comment 9 by bugdroid1@chromium.org, May 17 2017 

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

commit 2088149da693bad956c1b2f2fc4e58fa0eedbab8
Author: szager <szager@chromium.org>
Date: Wed May 17 08:39:31 2017

Update layer size from LayoutBox::UpdateAfterLayout

Previously, layer size was set during a post-layout call to
PaintLayer::UpdateLayerPositions.  With this change,
layer size is updated during layout, as soon as the new size is
available (after its owning LayoutBox has finished layout).

BUG= 701575 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2854213004
Cr-Commit-Position: refs/heads/master@{#472387}

[modify] https://crrev.com/2088149da693bad956c1b2f2fc4e58fa0eedbab8/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
[modify] https://crrev.com/2088149da693bad956c1b2f2fc4e58fa0eedbab8/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/2088149da693bad956c1b2f2fc4e58fa0eedbab8/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
[modify] https://crrev.com/2088149da693bad956c1b2f2fc4e58fa0eedbab8/third_party/WebKit/Source/core/layout/LayoutView.cpp
[modify] https://crrev.com/2088149da693bad956c1b2f2fc4e58fa0eedbab8/third_party/WebKit/Source/core/paint/PaintLayer.cpp
[modify] https://crrev.com/2088149da693bad956c1b2f2fc4e58fa0eedbab8/third_party/WebKit/Source/core/paint/PaintLayer.h

Comment 10 by skobes@chromium.org, May 23 2017

Status: Verified (was: Available)
This is fixed now (yay!)
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 29 2017 

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

commit 8971f74f9977918237228a5c78e5a09e77799e0d
Author: Stefan Zager <szager@chromium.org>
Date: Sat Jul 29 04:01:59 2017

Fix PaintLayerScrollableArea::VisibleContentRect

PLSA::VisibleContentRect is conceptually the box's client rect, but
the current implementation of VisibleContentRect weirdly includes
the box's borders.  This patch fixes that.

BUG= 701575 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ie757164924e3f2fc1f6c46848985d6f9c167d437
Reviewed-on: https://chromium-review.googlesource.com/588273
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490615}
[modify] https://crrev.com/8971f74f9977918237228a5c78e5a09e77799e0d/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/8971f74f9977918237228a5c78e5a09e77799e0d/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
[modify] https://crrev.com/8971f74f9977918237228a5c78e5a09e77799e0d/third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp
►

Sign in to add a comment