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

Issue 673963 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 417782



Sign in to add a comment

[root layer scrolls] impl thread scrolling crashes

Project Member Reported by skobes@chromium.org, Dec 14 2016

Issue description

Observed on Linux ToT.  Run:

out/d/content_shell --enable-blink-features=RootLayerScrolling 'data:text/html,<style>body{height:1000px}</style>'

Without clicking inside the window, try to scroll with the mousewheel.

SHOULD NEVER BE REACHED
../../third_party/WebKit/Source/core/frame/FrameView.cpp(3768) : virtual void blink::FrameView::updateScrollOffset(const ScrollOffset &, blink::ScrollType)
1   0x7f03df76f838 blink::FrameView::updateScrollOffset(blink::FloatSize const&, blink::ScrollType)
2   0x7f03e2fd3de7 blink::ScrollableArea::scrollOffsetChanged(blink::FloatSize const&, blink::ScrollType)
3   0x7f03e2fd3b5a blink::ScrollableArea::setScrollOffset(blink::FloatSize const&, blink::ScrollType, blink::ScrollBehavior)
4   0x7f03e2d65ed2 blink::GraphicsLayer::didScroll()
5   0x7f03d903ec25
6   0x7f03d903eb41
7   0x7f03d903eae7
8   0x7f03d903e9fc
9   0x7f03e4cb1a9b
10  0x7f03e4cc638c cc::Layer::SetScrollOffsetFromImplSide(gfx::ScrollOffset const&)
11  0x7f03e4f63ba5 cc::LayerTreeHostInProcess::ApplyScrollAndScale(cc::ScrollAndScaleSet*)
12  0x7f03e4fd8182 cc::ProxyMain::BeginMainFrame(std::unique_ptr<cc::BeginMainFrameAndCommitState, std::default_delete<cc::BeginMainFrameAndCommitState> >)

If you click inside the window first, it works.  Possibly related to focus.
 

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

Owner: skobes@chromium.org
Status: Started (was: Available)
I think this is because the frame scrolling layer (PLC::m_scrollLayer, which shouldn't be used) and the document's scrolling contents layer (CLM::m_scrollingContentsLayer) have the same cc::ElementId.

This might be fixed by getting rid of PLC::m_scrollLayer (which we want to do anyway).
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 18 2017

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

commit f547b3e1d559fd2abd8c9ed897ab627d8e23420a
Author: skobes <skobes@chromium.org>
Date: Wed Jan 18 17:17:00 2017

Add DCHECK for duplicate layers with the same cc::ElementId.

BUG= 673963 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/f547b3e1d559fd2abd8c9ed897ab627d8e23420a/cc/trees/layer_tree_impl.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 18 2017

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

commit bc1dbcfdf6fa0db6e78c6ac3af02b6d68e1e2c72
Author: jianli <jianli@chromium.org>
Date: Wed Jan 18 20:08:56 2017

Revert of Add DCHECK for duplicate layers with the same cc::ElementId. (patchset #2 id:20001 of https://codereview.chromium.org/2639103002/ )

Reason for revert:
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/6968

Original issue's description:
> Add DCHECK for duplicate layers with the same cc::ElementId.
>
> BUG= 673963 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Review-Url: https://codereview.chromium.org/2639103002
> Cr-Commit-Position: refs/heads/master@{#444407}
> Committed: https://chromium.googlesource.com/chromium/src/+/f547b3e1d559fd2abd8c9ed897ab627d8e23420a

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

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

[modify] https://crrev.com/bc1dbcfdf6fa0db6e78c6ac3af02b6d68e1e2c72/cc/trees/layer_tree_impl.cc

Comment 4 by jianli@chromium.org, Jan 18 2017

 Issue 682407  has been merged into this issue.

Comment 5 by jianli@chromium.org, Jan 18 2017

 Issue 682408  has been merged into this issue.

Comment 6 by jianli@chromium.org, Jan 18 2017

 Issue 682409  has been merged into this issue.

Comment 7 by jianli@chromium.org, Jan 18 2017

 Issue 682410  has been merged into this issue.

Comment 8 by jianli@chromium.org, Jan 18 2017

 Issue 682411  has been merged into this issue.

Comment 9 by jianli@chromium.org, Jan 18 2017

 Issue 682412  has been merged into this issue.
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 18 2017

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

commit 5c1e664a82399b427466ab057edbc3d0cacc09d3
Author: skobes <skobes@chromium.org>
Date: Wed Jan 18 22:54:52 2017

Root layer scrolling: fix element ID collision.

We were giving PLC::m_scrollLayer and CLM::m_scrollingContentsLayer the same
CompositorElementId.  This caused crashes when scrolling because we look up the
layer by the element ID in LTHI::SetTreeLayerScrollOffsetMutated, and sometimes
find the wrong one.

(In http://crrev.com/444407 I have also added a DCHECK for this condition in
the LayerTreeImpl.)

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

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

[modify] https://crrev.com/5c1e664a82399b427466ab057edbc3d0cacc09d3/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp

Cc: skobes@chromium.org
 Issue 682477  has been merged into this issue.
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 19 2017

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

commit 2409c02697954b6733938f95095423b93ef09e73
Author: skobes <skobes@chromium.org>
Date: Thu Jan 19 16:56:49 2017

[Reland] Add DCHECK for duplicate layers with the same cc::ElementId.

This relands r444407 which was reverted in r444452.

BUG= 673963 

Review-Url: https://codereview.chromium.org/2639103002
Cr-Commit-Position: refs/heads/master@{#444407}
Committed: https://chromium.googlesource.com/chromium/src/+/f547b3e1d559fd2abd8c9ed897ab627d8e23420a
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/2409c02697954b6733938f95095423b93ef09e73/cc/trees/layer_tree_impl.cc

 Issue 682735  has been merged into this issue.
Status: Verified (was: Started)
This is fixed.

Sign in to add a comment