New issue
Advanced search Search tips

Issue 788219 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 578252



Sign in to add a comment

Crash when trying to update style or layout while lifecycle is paused in paint by paint worklet paint function breakpoint.

Project Member Reported by flackr@chromium.org, Nov 23 2017

Issue description

Chrome Version: 64.0.3277
OS: Linux

What steps will reproduce the problem?
(1) Load a paintworklet demo
(2) Set a breakpoint in the paint function (note, due to issue 737988 this requires also including the paint worklet js in the main frame)
(3) Hit the breakpoint by causing the element to repaint
(4) Cause a request to repaint the window (e.g. minimize and show)

What is the expected result?
I suppose we would expect the PaintWorklet graphics to be missing since we can't invoke paintworklet while paused in paintworklet or for the tab to be hung.

What happens instead?
Instead, the tab crashes with the following:
[1:1:1123/133346.770348:FATAL:Document.cpp(2437)] Check failed: Lifecycle().StateAllowsTreeMutations().
#0 0x7f9ddaf1d09c base::debug::StackTrace::StackTrace()
#1 0x7f9ddaf3c7ec logging::LogMessage::~LogMessage()
#2 0x7f9dd4e702a0 blink::Document::UpdateStyleAndLayoutTreeIgnorePendingStylesheets()
#3 0x7f9dd4e6fe62 blink::Document::UpdateStyleAndLayoutIgnorePendingStylesheets()
#4 0x7f9dd4f2d1a9 blink::FrameSelection::SetSelectionFromNone()
#5 0x7f9dd4f2d100 blink::FrameSelection::FocusedOrActiveStateChanged()
#6 0x7f9dd5002478 blink::WebViewImpl::SetFocus()
#7 0x7f9dd92e6830 content::RenderWidget::OnSetFocus()
#8 0x7f9dd92dcea5 content::RenderViewImpl::OnSetFocus()
#9 0x7f9dd9247eec content::(anonymous namespace)::RunClosureIfNotSwappedOut()
#10 0x7f9dd92484a5 _ZN4base8internal7InvokerINS0_9BindStateIPFvNS_7WeakPtrIN7content12RenderWidgetEEENS_12OnceCallbackIFvvEEEEJS6_S9_EEES8_E7RunOnceEPNS0_13BindStateBaseE
#11 0x7f9dd9245a0d content::(anonymous namespace)::QueuedClosure::Dispatch()
#12 0x7f9dd9244dea content::MainThreadEventQueue::DispatchEvents()
#13 0x7f9ddaf1d94f base::debug::TaskAnnotator::RunTask()
#14 0x7f9dd4116079 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
#15 0x7f9dd4114137 blink::scheduler::TaskQueueManager::DoWork()
#16 0x7f9ddaf1d94f base::debug::TaskAnnotator::RunTask()
#17 0x7f9ddaf44e17 base::MessageLoop::RunTask() 
#18 0x7f9ddaf453e8 base::MessageLoop::DoWork()  
#19 0x7f9ddaf4609a base::MessagePumpDefault::Run()
#20 0x7f9ddaf44ab4 base::MessageLoop::Run()
#21 0x7f9ddaf6ca74 base::RunLoop::Run()
#22 0x7f9dd92045c1 content::(anonymous namespace)::WebKitClientMessageLoopImpl::Run()
#23 0x7f9dd4fdb774 blink::ClientMessageLoopAdapter::RunLoop()
#24 0x7f9dd6438f90 v8_inspector::V8Debugger::handleProgramBreak()
#25 0x7f9dd5fa3918 v8::internal::Debug::OnDebugBreak()
#26 0x7f9dd5fa33ee v8::internal::Debug::Break() 
#27 0x7f9dd6253f8b v8::internal::Runtime_DebugBreakOnBytecode()
#28 0x0f6213b8565d <unknown>
 

Comment 1 by flackr@chromium.org, Nov 23 2017

Summary: Crash when trying to update style or layout while lifecycle is paused in paint by paint worklet paint function breakpoint. (was: Crash when repainting while paused in paint worklet paint function.)
This can also be caused by switching over to the elements panel and selecting an element. Dev tools tries to get the computed style for the element and crashes:

[1:1:1123/135614.758365:FATAL:Document.cpp(2437)] Check failed: Lifecycle().StateAllowsTreeMutations().
#0 0x7f61514f409c base::debug::StackTrace::StackTrace()
#1 0x7f61515137ec logging::LogMessage::~LogMessage()
#2 0x7f614b4472a0 blink::Document::UpdateStyleAndLayoutTreeIgnorePendingStylesheets()
#3 0x7f614b446e2d blink::Document::UpdateStyleAndLayoutIgnorePendingStylesheetsForNode()
#4 0x7f614b2f4006 blink::CSSComputedStyleDeclaration::GetPropertyCSSValue()
#5 0x7f614b2f396f blink::CSSComputedStyleDeclaration::GetPropertyValueInternal()
#6 0x7f614b7c12eb blink::InspectorCSSAgent::getComputedStyleForNode()
#7 0x7f614b7c15c2 blink::InspectorCSSAgent::getComputedStyleForNode()
#8 0x7f614bd7f06a blink::protocol::CSS::DispatcherImpl::getComputedStyleForNode()
#9 0x7f614bd68901 blink::protocol::Accessibility::DispatcherImpl::dispatch()
#10 0x7f614bddadee blink::protocol::UberDispatcher::dispatch()
#11 0x7f614b814486 blink::InspectorSession::DispatchProtocolMessage()
#12 0x7f614b5b144c blink::WebDevToolsAgentImpl::DispatchMessageFromFrontend()
#13 0x7f614b5b124d blink::WebDevToolsAgentImpl::DispatchOnInspectorBackend()
#14 0x7f614f7da320 content::DevToolsAgent::DispatchOnInspectorBackend()
#15 0x7f614f01f78f content::mojom::DevToolsSessionStubDispatch::Accept()
#16 0x7f61508c47e6 IPC::(anonymous namespace)::ChannelAssociatedGroupController::AcceptOnProxyThread()
#17 0x7f61508c2f09 _ZN4base8internal7InvokerINS0_9BindStateIMN3IPC12_GLOBAL__N_132ChannelAssociatedGroupControllerEFvN4mojo7MessageEEJ13scoped_refptrIS5_ENS0_13PassedWrapperIS7_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#18 0x7f61514f494f base::debug::TaskAnnotator::RunTask()
#19 0x7f614a6ed079 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
#20 0x7f614a6eb137 blink::scheduler::TaskQueueManager::DoWork()
#21 0x7f61514f494f base::debug::TaskAnnotator::RunTask()
#22 0x7f615151be17 base::MessageLoop::RunTask()
#23 0x7f615151c3e8 base::MessageLoop::DoWork()
#24 0x7f615151d09a base::MessagePumpDefault::Run()
#25 0x7f615151bab4 base::MessageLoop::Run()
#26 0x7f6151543a74 base::RunLoop::Run()  
#27 0x7f614f7db5c1 content::(anonymous namespace)::WebKitClientMessageLoopImpl::Run()
#28 0x7f614b5b2774 blink::ClientMessageLoopAdapter::RunLoop()
#29 0x7f614ca0ff90 v8_inspector::V8Debugger::handleProgramBreak()
#30 0x7f614c57a918 v8::internal::Debug::OnDebugBreak()
#31 0x7f614c57a3ee v8::internal::Debug::Break()
#32 0x7f614c82af8b v8::internal::Runtime_DebugBreakOnBytecode()
#33 0x3bc79770565d <unknown>

I feel like we'll need a way to pause the worklet without pausing the lifecycle to avoid these problems.
Blocking: 578252
flackr@: this is much more complicated than I thought. Are you suggesting that somehow we pause painting from the paint worklet, while carry on other tasks in the lifecycle, and come back to the paint worklet tasks once the getComputedStyle is done?
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 11 2018

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

commit d9f90c9b2ea6dd588717556b1a2db9cd51c9f88b
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Jan 11 18:31:01 2018

Disable breakpoint for PaintWorklet

Right now, if we put a break point in the paint function, let the debugger
hit the break point, and switch to the element panel, it will crash the
renderer. The reason is that when we break in the middle of the paint
function, we are pausing the lifecycle, and switching to the element panel
will cause dev tool to get the computed style, and then we hit this check:
CHECK(Lifecycle().StateAllowsTreeMutations());
in Document.cpp.

For now, the solution is completely disable break point for paint worklet,
for one release. We will fix this problem in M66 with a better solution.
This patch checks that if we are going to break in the middle of a
lifecycle, then skip the break point and never break.

Bug:  788219 
Change-Id: Ieb4f86c040df88a09b196fb605aa66cfe93940d0
Reviewed-on: https://chromium-review.googlesource.com/860559
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528676}
[modify] https://crrev.com/d9f90c9b2ea6dd588717556b1a2db9cd51c9f88b/third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp

update: the above CL is a temporary solution, will keep paintworklet not debuggable for one release and have a proper fix in M66.
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 15 2018

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

commit 1d692b441a4fe9771fbefa6cd9647e6f8ddf1822
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Feb 15 13:44:29 2018

[PaintWorklet] Fix crash when hitting breakpoint during lifecycle

If we put a devtool's breakpoint in the paint() function and start
debugging, the renderer will crash when we resize the window or switch
to the element tab. This is because when we are in the middle of
lifecycle when we get to the paint function, and anything triggers
layout update will cause a renderer crash.

The fix in CL is that when we hit a breakpoint, check if we are in the
middle of lifecycle or not, if we are, flip a bit to be true in the
|LocalFrameView|, so that later on whenever there is a layout update we
can check that bit and do early exit.

Bug:  788219 
Change-Id: I4fbafab868508ed4ba453339d91364253d17f523
Reviewed-on: https://chromium-review.googlesource.com/899484
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537004}
[modify] https://crrev.com/1d692b441a4fe9771fbefa6cd9647e6f8ddf1822/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/1d692b441a4fe9771fbefa6cd9647e6f8ddf1822/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/1d692b441a4fe9771fbefa6cd9647e6f8ddf1822/third_party/WebKit/Source/core/dom/DocumentLifecycle.h
[modify] https://crrev.com/1d692b441a4fe9771fbefa6cd9647e6f8ddf1822/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/1d692b441a4fe9771fbefa6cd9647e6f8ddf1822/third_party/WebKit/Source/core/inspector/InspectorLayerTreeAgent.cpp
[modify] https://crrev.com/1d692b441a4fe9771fbefa6cd9647e6f8ddf1822/third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp
[modify] https://crrev.com/1d692b441a4fe9771fbefa6cd9647e6f8ddf1822/third_party/WebKit/Source/core/inspector/MainThreadDebugger.h
[add] https://crrev.com/1d692b441a4fe9771fbefa6cd9647e6f8ddf1822/third_party/WebKit/Source/core/inspector/MainThreadDebuggerTest.cpp

Status: Fixed (was: Assigned)

Sign in to add a comment