New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 672335 link

Starred by 1 user
Status: Fixed
Owner:
skobes@chromium.org
Closed: Dec 2016
Cc:
szager@chromium.org
bokan@chromium.org
█ glebl@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 417782



Sign in to add a comment

[root layer scrolls] scrollbars may disappear after style recalc

Project Member Reported by skobes@chromium.org, Dec 8 2016 
Observed on Linux ToT.  Run:

content_shell --enable-blink-features=RootLayerScrolling https://www.google.com/intl/en/about/

While the page is loading, the scrollbars flicker.  Also if you try to scroll too soon (mousewheel) we crash:

SHOULD NEVER BE REACHED
../../third_party/WebKit/Source/core/frame/FrameView.cpp(3752) : virtual void blink::FrameView::updateScrollOffset(const ScrollOffset &, blink::ScrollType)
1   0x7fb49afd3da8 blink::FrameView::updateScrollOffset(blink::FloatSize const&, blink::ScrollType)
2   0x7fb4a51dbea7 blink::ScrollableArea::scrollOffsetChanged(blink::FloatSize const&, blink::ScrollType)
3   0x7fb4a51dbc1a blink::ScrollableArea::setScrollOffset(blink::FloatSize const&, blink::ScrollType, blink::ScrollBehavior)
4   0x7fb4a4f6ebc2 blink::GraphicsLayer::didScroll()
5   0x7fb49fcd0c25
6   0x7fb49fcd0b41
7   0x7fb49fcd0ae7
8   0x7fb49fcd09fc
9   0x7fb4ab19885b
10  0x7fb4ab1ad14c cc::Layer::SetScrollOffsetFromImplSide(gfx::ScrollOffset const&)
11  0x7fb4ab44bde5 cc::LayerTreeHostInProcess::ApplyScrollAndScale(cc::ScrollAndScaleSet*)
12  0x7fb4ab4c13d2 cc::ProxyMain::BeginMainFrame(std::unique_ptr<cc::BeginMainFrameAndCommitState, std::default_delete<cc::BeginMainFrameAndCommitState> >)

Comment 1 by skobes@chromium.org, Dec 12 2016 

Minimization of scrollbar flicker attached.

Document::updateStyle sets overflow: visible on the document (based on StyleResolver::styleForDocument) before flipping it back to auto in inheritHtmlAndBodyElementStyles.  This causes PaintLayerScrollableArea to remove the scrollbars.
rls-scrollbar-removal.html
428 bytes View Download

Comment 2 by skobes@chromium.org, Dec 12 2016

Status: Started (was: Assigned)

Comment 3 by skobes@chromium.org, Dec 14 2016

Summary: [root layer scrolls] scrollbars may disappear after style recalc (was: [root layer scrolls] during load, scrollbar flashes and scrolling crashes)
Patch up for scrollbar flicker: http://crrev.com/2571893003

The impl-scroll crash is unrelated; tracking that in  issue 673963 .
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 14 2016 

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

commit 0cc860d2c6075b9afb96f566a370f62972a758e0
Author: skobes <skobes@chromium.org>
Date: Wed Dec 14 16:57:21 2016

Set overflow: auto in StyleResolver::styleForDocument.

Without this, the call to setStyle in Document::updateStyle would temporarily
mark the LayoutView as overflow: visible.  With root layer scrolling this causes
PaintLayerScrollableArea to remove the scrollbars.

This change has no effect on the document's final overflow style, which comes
from inheritHtmlAndBodyElementStyles (and is never "visible" regardless of RLS).

BUG= 672335 

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

[modify] https://crrev.com/0cc860d2c6075b9afb96f566a370f62972a758e0/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
[modify] https://crrev.com/0cc860d2c6075b9afb96f566a370f62972a758e0/third_party/WebKit/Source/web/BUILD.gn
[add] https://crrev.com/0cc860d2c6075b9afb96f566a370f62972a758e0/third_party/WebKit/Source/web/tests/ScrollbarsTest.cpp
[modify] https://crrev.com/0cc860d2c6075b9afb96f566a370f62972a758e0/third_party/WebKit/Source/web/tests/sim/SimTest.cpp
[modify] https://crrev.com/0cc860d2c6075b9afb96f566a370f62972a758e0/third_party/WebKit/Source/web/tests/sim/SimTest.h
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 14 2016 

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

commit 503a8299269f0cf0e838ce922228646b7bde6d21
Author: paulmeyer <paulmeyer@chromium.org>
Date: Wed Dec 14 17:13:39 2016

Revert of Set overflow: auto in StyleResolver::styleForDocument. (patchset #1 id:1 of https://codereview.chromium.org/2571893003/ )

Reason for revert:
This breaks compiles on Android Cast bot: https://build.chromium.org/p/chromium.linux/builders/Cast%20Android%20%28dbg%29/builds/58645

Original issue's description:
> Set overflow: auto in StyleResolver::styleForDocument.
>
> Without this, the call to setStyle in Document::updateStyle would temporarily
> mark the LayoutView as overflow: visible.  With root layer scrolling this causes
> PaintLayerScrollableArea to remove the scrollbars.
>
> This change has no effect on the document's final overflow style, which comes
> from inheritHtmlAndBodyElementStyles (and is never "visible" regardless of RLS).
>
> BUG= 672335 
>
> Committed: https://crrev.com/0cc860d2c6075b9afb96f566a370f62972a758e0
> Cr-Commit-Position: refs/heads/master@{#438533}

TBR=bokan@chromium.org,szager@chromium.org,skobes@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 672335 

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

[modify] https://crrev.com/503a8299269f0cf0e838ce922228646b7bde6d21/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
[modify] https://crrev.com/503a8299269f0cf0e838ce922228646b7bde6d21/third_party/WebKit/Source/web/BUILD.gn
[delete] https://crrev.com/78efbde6ae8fa8409079dc0ef14eadf447ffd8d2/third_party/WebKit/Source/web/tests/ScrollbarsTest.cpp
[modify] https://crrev.com/503a8299269f0cf0e838ce922228646b7bde6d21/third_party/WebKit/Source/web/tests/sim/SimTest.cpp
[modify] https://crrev.com/503a8299269f0cf0e838ce922228646b7bde6d21/third_party/WebKit/Source/web/tests/sim/SimTest.h
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 14 2016 

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

commit b8ad7cfdc0f044af7f16598b5cbc4170a7783e61
Author: skobes <skobes@chromium.org>
Date: Wed Dec 14 20:48:33 2016

[Reland] Set overflow: auto in StyleResolver::styleForDocument.

Without this, the call to setStyle in Document::updateStyle would temporarily
mark the LayoutView as overflow: visible.  With root layer scrolling this causes
PaintLayerScrollableArea to remove the scrollbars.

This change has no effect on the document's final overflow style, which comes
from inheritHtmlAndBodyElementStyles (and is never "visible" regardless of RLS).

This relands r438533 which was reverted in r438537.

BUG= 672335 

Committed: https://crrev.com/0cc860d2c6075b9afb96f566a370f62972a758e0
Review-Url: https://codereview.chromium.org/2571893003
Cr-Original-Commit-Position: refs/heads/master@{#438533}
Cr-Commit-Position: refs/heads/master@{#438616}

[modify] https://crrev.com/b8ad7cfdc0f044af7f16598b5cbc4170a7783e61/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
[modify] https://crrev.com/b8ad7cfdc0f044af7f16598b5cbc4170a7783e61/third_party/WebKit/Source/web/BUILD.gn
[add] https://crrev.com/b8ad7cfdc0f044af7f16598b5cbc4170a7783e61/third_party/WebKit/Source/web/tests/ScrollbarsTest.cpp
[modify] https://crrev.com/b8ad7cfdc0f044af7f16598b5cbc4170a7783e61/third_party/WebKit/Source/web/tests/sim/SimTest.cpp
[modify] https://crrev.com/b8ad7cfdc0f044af7f16598b5cbc4170a7783e61/third_party/WebKit/Source/web/tests/sim/SimTest.h

Comment 7 by skobes@chromium.org, Dec 16 2016

Status: Fixed (was: Started)
►

Sign in to add a comment