New issue
Advanced search Search tips

Issue 812364 link

Starred by 4 users

Issue metadata

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



Sign in to add a comment

DCHECK fails while ScrollingCoordinator is setting touch event targets

Project Member Reported by kenrb@chromium.org, Feb 14 2018

Issue description


I can probably have a deeper look at this tomorrow, but I have confirmed that a DCHECK crash I am seeing on trunk is caused by r535797. Reverting it fixes the problem.

I can reproduce consistently by just visiting https://www.youtube.com/unboxed using a build with DCHECKs enabled and with the --site-per-process flag. It crashes within 10-20 seconds with the following stack:
[1:1:0214/133750.045749:FATAL:PaintLayer.cpp(2563)] Check failed: IsAllowedToQueryCompositingState(). 
#0 0x7f7f698389fd base::debug::StackTrace::StackTrace()
#1 0x7f7f69836eec base::debug::StackTrace::StackTrace()
#2 0x7f7f698bf21a logging::LogMessage::~LogMessage()
#3 0x7f7f574111aa blink::PaintLayer::GetCompositingState()
#4 0x7f7f5741bca0 blink::PaintLayer::GraphicsLayerBacking()
#5 0x7f7f573934fe blink::ScrollingCoordinator::SetTouchEventTargetRects()
#6 0x7f7f5738f203 blink::ScrollingCoordinator::UpdateTouchEventTargetRectsIfNeeded()
#7 0x7f7f5738e439 blink::ScrollingCoordinator::UpdateAfterCompositingChangeIfNeeded()
#8 0x7f7f56ab63c5 blink::LocalFrameView::UpdateLifecyclePhasesInternal()
#9 0x7f7f56ab5a52 blink::LocalFrameView::UpdateAllLifecyclePhases()
#10 0x7f7f57366c4b blink::PageAnimator::UpdateAllLifecyclePhases()
#11 0x7f7f5736c2ec blink::PageWidgetDelegate::UpdateLifecycle()
#12 0x7f7f56b2e10e blink::WebFrameWidgetImpl::UpdateLifecycle()
#13 0x7f7f6503ed06 content::RenderWidget::UpdateVisualState()
#14 0x7f7f64e2bee0 content::RenderWidgetCompositor::UpdateLayerTreeHost()
#15 0x7f7f611161e0 cc::LayerTreeHost::RequestMainFrameUpdate()
#16 0x7f7f61206200 cc::ProxyMain::BeginMainFrame()

It is strange that it only repros with --site-per-process, but it is the main frame that is crashing, as if the presence of the remote frame is somehow causing the problem.
 

Comment 1 by kenrb@chromium.org, Feb 15 2018

Cc: wjmaclean@chromium.org
Components: Internals>Sandbox>SiteIsolation
Labels: OS-Android OS-Chrome OS-Mac OS-Windows
Owner: kenrb@chromium.org
Status: Started (was: Assigned)
I'm pretty sure this has to do with youtube embedding a third-party site that again embeds youtube, creating an A-B-A nesting. I don't quite understand what in that patch is causing the problem but I am trying to work it out. There are a number of places in ScrollingCoordinator that check if the main frame is local, which will tend to behave incorrectly in this situation. In that case, the embedded A frame is an OOPIF, but resides in the same process as the main frame, confusing that check.
Issue 813568 has been merged into this issue.

Comment 3 by kenrb@chromium.org, Feb 20 2018

Issue 813035 has been merged into this issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 23 2018

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

commit 446abe5ce915a0ff48a9a3860c21ca10bb57cd77
Author: Ken Buchanan <kenrb@chromium.org>
Date: Fri Feb 23 18:05:50 2018

Refactor LocalFrameView state for ScrollingCoordinator

This CL adds a new class ScrollingCoordinatorContext which contains
state that used to belong to ScrollingCoordinator but needs to be
duplicated for each local frame root when OOPIFs are instantiated. It
is lazily created on LocalFrameViews that are belong to local roots
(i.e. OOPIFs or main frames).

This also moves layers_with_touch_rects_ out of ScrollingCoordinator,
because reuse of that HashSet between frame roots was causing the crash
in  bug 812364 .

Bug: 680606,  812364 
Change-Id: I09feb2add534b460dd9425fb918a6c856e325efb
Reviewed-on: https://chromium-review.googlesource.com/922702
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538819}
[modify] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
[modify] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/animation/DocumentAnimations.cpp
[modify] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/frame/LocalFrameView.h
[modify] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/page/BUILD.gn
[modify] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
[modify] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.h
[add] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinatorContext.cpp
[add] https://crrev.com/446abe5ce915a0ff48a9a3860c21ca10bb57cd77/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinatorContext.h

Comment 5 by kenrb@chromium.org, Feb 26 2018

Status: Fixed (was: Started)

Sign in to add a comment