New issue
Advanced search Search tips

Issue 679994 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug-Regression



Sign in to add a comment

Check failed: property != CSSPropertyVariable in CSSAnimations.cpp:createKeyframeEffectModel()

Project Member Reported by alancutter@chromium.org, Jan 11 2017

Issue description

The following test case fails an assert:
<style>
@keyframes test {
  to { --x: pants; }
}
body {
  animation: test 1s;
}
</style>


[1:1:0111/173019.268387:716385765938:FATAL:PropertyHandle.h(26)] Check failed: property != CSSPropertyVa
riable (2 vs. 2)
#0 0x7fa07c21d30e base::debug::StackTrace::StackTrace()
#1 0x7fa07c24189b logging::LogMessage::~LogMessage()
#2 0x7fa0778a8494 blink::PropertyHandle::PropertyHandle()
#3 0x7fa077939f12 blink::(anonymous namespace)::createKeyframeEffectModel()
#4 0x7fa077938023 blink::CSSAnimations::calculateAnimationUpdate()
#5 0x7fa077ab323c blink::StyleResolver::calculateAnimationUpdate()
#6 0x7fa077ab1058 blink::StyleResolver::applyMatchedPropertiesAndCustomPropertyAnimations()
#7 0x7fa077ab0a31 blink::StyleResolver::styleForElement()


The offending line is "!startKeyframeProperties.contains(PropertyHandle(property));".
This is part of use counting SyntheticKeyframesInCompositedCSSAnimation.

The use counting logic is broken ever since custom properties were permitted in CSS @keyframes Animations.
 
This has no adverse implications for release builds and no incorrect behaviour as no custom properties are currently compositable. This is purely a code health issue.

Comment 2 by suzyh@chromium.org, Jan 11 2017

Labels: Update-Monthly
Labelling as Update-Monthly since custom properties animation is current project work.
Project Member

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

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

commit 2d136863c16220ecc767607d4615624168d99212
Author: alancutter <alancutter@chromium.org>
Date: Thu Jan 12 07:58:12 2017

Refactor how UseCounter::SyntheticKeyframesInCompositedCSSAnimation is counted

This patch rewrites the detection logic for composited animations that
have neutral keyframes.

The previous code was relatively expensive and failed a DCHECK when
custom properties were used. This patch moves the use counting down into
where we actually handle compositing neutral keyframes.

BUG= 679994 

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

[add] https://crrev.com/2d136863c16220ecc767607d4615624168d99212/third_party/WebKit/LayoutTests/animations/use-count-SyntheticKeyframesInCompositedCSSAnimation.html
[modify] https://crrev.com/2d136863c16220ecc767607d4615624168d99212/third_party/WebKit/Source/core/animation/Keyframe.h
[modify] https://crrev.com/2d136863c16220ecc767607d4615624168d99212/third_party/WebKit/Source/core/animation/KeyframeEffectModel.cpp
[modify] https://crrev.com/2d136863c16220ecc767607d4615624168d99212/third_party/WebKit/Source/core/animation/animatable/AnimatableValueKeyframe.h
[modify] https://crrev.com/2d136863c16220ecc767607d4615624168d99212/third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp

Status: Fixed (was: Assigned)

Sign in to add a comment