New issue
Advanced search Search tips

Issue 881318 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Check failed: "Lifecycle().GetState() >= DocumentLifecycle::kLayoutClean" when making a video element fullscreen

Project Member Reported by rog...@vewd.com, Sep 6

Issue description

Chrome Version: 71.0.3544.0 (locally build chrome with dchecks)
OS: Linux

What steps will reproduce the problem?
(1) Load the attached TC
(2) Click fullscreen button
(3)

What is the expected result?
Fullscreen video element but more importantly no DCHECK.

What happens instead?
FATAL:local_frame_view.cc(2438)] Check failed: Lifecycle().GetState() >= DocumentLifecycle::kLayoutClean. 
#0 0x55564e5d7e3c base::debug::StackTrace::StackTrace()
#1 0x55564e51bb7b logging::LogMessage::~LogMessage()
#2 0x555651e74e0e blink::LocalFrameView::UpdateLifecyclePhasesInternal()
#3 0x555651e73d99 blink::LocalFrameView::UpdateLifecyclePhases()
#4 0x555651e73b81 blink::LocalFrameView::UpdateAllLifecyclePhases()
#5 0x5556524fccbe blink::PageAnimator::UpdateAllLifecyclePhases()
#6 0x555651dc84a2 blink::WebViewImpl::UpdateLifecycle()
#7 0x555651ee0228 blink::WebViewFrameWidget::UpdateLifecycle()
#8 0x555653453934 content::RenderWidget::UpdateVisualState()
#9 0x55564f86073f cc::ProxyMain::BeginMainFrame()

Please use labels and text to provide additional information.
This is a DCHECK that was triggered by an internal testing page that makes use of qunit.js. I've spent some time investigating and managed to create a minimal testcase for it (attached).

The DCHECK seem to trigger when there is a window.onerror handler that inserts elements into the DOM, which qunit does. The specific error that is caught is "ResizeObserver loop limit exceeded".

I also did a bisect and I believe it was introduced by:
https://chromium-review.googlesource.com/1070762
But back then the dcheck looked different ("FATAL:local_frame_view.cc(2020)] Check failed: false").
 
small_tc.html
484 bytes View Download
Description: Show this description
Owner: vmp...@chromium.org
Status: Assigned (was: Untriaged)
vmpstr@, you recently did some cleanup here in https://chromium-review.googlesource.com/c/chromium/src/+/1136882 - do you know why we're asserting that the lifecycle is at least at LayoutClean?

The DCHECK fires when we're in kVisualUpdatePending which, naively, seems like a valid time to call UpdateAllLifecyclePhases. 
Hmm, this DCHECK is right after RunStyleAndLayoutLifecyclePhases, which should advance the lifecycle to layout clean. There are some DCHECKS in that function as well asserting the same thing, which aren't triggering, so we must be entering kVisualUpdatePending from the middle of lifecycle updates, which seems wrong. 

I'll investigate
Looks like frame_view.NotifyResizeObservers() at the end of RunStyleAndLayoutLifecyclePhases is setting it back to kVisualUpdatePending
ResizeObserver loop limit exceeded is a key thing that I missed initially :) It happens when resize observers change the layout and we run out of opportunities to re-layout right there and then. This rewinds the lifecycle state and schedules a new animation, we just have to abort running more lifecycle phases at that point.

fix: https://chromium-review.googlesource.com/c/chromium/src/+/1220811
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 12

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

commit 1b553a7bccbbb7ff1d31c26025ccc06db1d89338
Author: Vladimir Levin <vmpstr@chromium.org>
Date: Wed Sep 12 17:24:55 2018

Abort the lifecycle updates when resizeobserver limit was reached.

This patch ensures that when we reach the limit of ResizeObserver
re-layouts, we abort the current lifecycle updates. At that time, it
rewound the state to be in kVisualUpdatesPending and scheduled an
animation.

R=bokan@chromium.org, chrishtr@chromium.org

Bug:  881318 
Change-Id: I846e65b4be599e995dfbfc8b5dcf3a37a83856b0
Reviewed-on: https://chromium-review.googlesource.com/1220811
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590738}
[modify] https://crrev.com/1b553a7bccbbb7ff1d31c26025ccc06db1d89338/third_party/blink/renderer/core/frame/local_frame_view.cc

Status: Fixed (was: Assigned)

Sign in to add a comment