New issue
Advanced search Search tips

Issue 701250 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Check failed: *m_baseComputedStyle == *computedStyle for registered custom property transitions

Project Member Reported by alancutter@chromium.org, Mar 14 2017

Issue description

When playing around with this demo https://gist.github.com/alancutter/869b77b9ba0029a2eca75f869a527bb0 with https://codereview.chromium.org/2730683002 applied we hit an assert at the end of a transition:

[1:1:0314/142134.561924:613790109965:FATAL:ElementAnimations.cpp(115)] Check failed: *m_baseComputedStyl
e == *computedStyle. 
#0 0x7fb9d0adb1e7 base::debug::StackTrace::StackTrace()
#1 0x7fb9d0aff50b logging::LogMessage::~LogMessage()
#2 0x7fb9cbf9207e blink::ElementAnimations::updateBaseComputedStyle()
#3 0x7fb9cc15b1f6 blink::StyleResolver::styleForElement()
#4 0x7fb9cc1fcffd blink::Element::originalStyleForLayoutObject()
#5 0x7fb9cc1fcc38 blink::Element::styleForLayoutObject()
#6 0x7fb9cc1fd7f6 blink::Element::recalcOwnStyle()
#7 0x7fb9cc1fd275 blink::Element::recalcStyle()


Haven't been able to create a deterministic LayoutTest for this as it depends on animation triggered style recalcs.

Note that this crash requires experimental features, debug and a CL patched in to hit. This does not affect users.
 
Summary: Check failed: *m_baseComputedStyle == *computedStyle for registered custom property transitions (was: Check failed: *m_baseComputedStyl e == *computedStyle for registered custom property transitions)
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 14 2017

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

commit 04263813eccfbf85dc2143e4154b3c65b0f036fb
Author: alancutter <alancutter@chromium.org>
Date: Tue Mar 14 07:07:43 2017

Enable control over frame time in SimTests

This change adds beginFrame(double timeDelta) to SimCompositor to allow
SimTests to control the time progression used by each frame.
This is in preparation for adding animation SimTests.

This change also fixes a bug in the beginFrame() implementation where
it used monotonicallyIncreasingTime() instead of m_lastFrameTimeMonotonic.

BUG= 701250 

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

[modify] https://crrev.com/04263813eccfbf85dc2143e4154b3c65b0f036fb/third_party/WebKit/Source/web/tests/sim/SimCompositor.cpp
[modify] https://crrev.com/04263813eccfbf85dc2143e4154b3c65b0f036fb/third_party/WebKit/Source/web/tests/sim/SimCompositor.h

Labels: Stability-Crash
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 20 2017

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

commit 2a9fb0166f08fecc7b3b68deff1b6523f1610d23
Author: alancutter <alancutter@chromium.org>
Date: Mon Mar 20 00:42:52 2017

Remove header ordering checks from check-webkit-style

This change removes checks for header ordering in check-webkit-style.
We use clang-format to format and lint our headers so it is unnecessary
for check-webkit-style to check this.

See AnimationSimTest.cpp in https://codereview.chromium.org/2757543002 for
something linted by clang-format that would fail check-webkit-style
if this feature weren't removed.

BUG= 701250 

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

[modify] https://crrev.com/2a9fb0166f08fecc7b3b68deff1b6523f1610d23/third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py
[modify] https://crrev.com/2a9fb0166f08fecc7b3b68deff1b6523f1610d23/third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py

Have a fix for this (https://codereview.chromium.org/2757543002) but the SimTest I wrote is crashing on Mac on test shut down.
lldb log: https://hastebin.com/uyofuzorah.go

Seems to fail to destruct the PropertyRegistration's Vector<std::unique_ptr<const InterpolationType>> member. Going to test with ASAN.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 3 2017

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

commit 767e438649154d1985a803c93111e0860a29fa2f
Author: alancutter <alancutter@chromium.org>
Date: Mon Apr 03 14:08:46 2017

Ensure baseComputedStyle optimisation is cleared during registered custom property animations

This change to StyleResolver ensures that the cached baseComputedStyle
on an animated element is removed while custom property animations are
active. This prevents stale baseComputedStyles from persisting and
failing the equality assertion in ElementAnimations::updateBaseComputedStyle().

The regression test for this bug needed to be a SimTest as it depended
on style recalcs being triggered by animations and not Javascript.
It's not possible to do this deterministically from Javascript as it
requires control over the frame times.

As part of making the SimTest ElementAnimation::animate() and
PropertyRegistration::registerProperty() required changes to reduce their
V8 dependencies. Additionally a test-only method was added to
AnimationClock to turn off generating synthetic frame times.

BUG= 701250 

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

[modify] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/core/animation/AnimationClock.cpp
[modify] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/core/animation/AnimationClock.h
[modify] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/core/animation/ElementAnimation.h
[modify] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/core/css/PropertyRegistration.cpp
[modify] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/core/css/PropertyRegistration.h
[modify] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/core/css/PropertyRegistration.idl
[modify] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
[modify] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/web/BUILD.gn
[add] https://crrev.com/767e438649154d1985a803c93111e0860a29fa2f/third_party/WebKit/Source/web/tests/AnimationSimTest.cpp

Status: Fixed (was: Started)

Sign in to add a comment