Closed Bug 1290994 Opened 4 years ago Closed 4 years ago

Assertion failure: SpecifiedKeyframeArraysAreEqual(mKeyframes, keyframesCopy), at dom/animation/KeyframeEffect.cpp:567

Categories

(Core :: DOM: Animation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: truber, Assigned: hiro)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file testcase.html
Assertion failure: SpecifiedKeyframeArraysAreEqual(mKeyframes, keyframesCopy) (Apart from the computed offset members, the keyframes array should not be modified), at dom/animation/KeyframeEffect.cpp:567

Found in m-c-1470060549-debug

Stack:
    #0 0x7f0599082fb0 in mozilla::dom::KeyframeEffectReadOnly::UpdateProperties(nsStyleContext*) dom/animation/KeyframeEffect.cpp:565:5
    #1 0x7f059908b2b7 in mozilla::dom::KeyframeEffectReadOnly::SetKeyframes(nsTArray<mozilla::Keyframe>&&, nsStyleContext*) dom/animation/KeyframeEffect.cpp:489:5
    #2 0x7f059908ab1d in mozilla::dom::KeyframeEffectReadOnly::SetKeyframes(JSContext*, JS::Handle<JSObject*>, mozilla::ErrorResult&) dom/animation/KeyframeEffect.cpp:466:3
    #3 0x7f05990918c9 in already_AddRefed<mozilla::dom::KeyframeEffect> mozilla::dom::KeyframeEffectReadOnly::ConstructKeyframeEffect<mozilla::dom::KeyframeEffect, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions>(mozilla::dom::GlobalObject const&, mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) dom/animation/KeyframeEffect.cpp:861:3
    #4 0x7f059909158d in mozilla::dom::KeyframeEffect::Constructor(mozilla::dom::GlobalObject const&, mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) dom/animation/KeyframeEffect.cpp:1598:10
Attached file stack
Flags: in-testsuite?
Priority: -- → P3
>              {borderLeftColor:"hsl(0,0e309%,0%)"}]);

The second parameter, green component, is parsed as -NaN.

(gdb) p *mKeyframes.ElementAt(1).mPropertyValues.ElementAt(0).mValue.mValue.mFloatColor
$17 = {mRefCnt = {static isThreadSafe = false, mValue = 3}, _mOwningThread = {mThread = 0x7ffff6b7a5c0}, mComponent1 = 0, mComponent2 = -nan(0x400000), mComponent3 = 0, mAlpha = 1}
(gdb) p *keyframesCopy.ElementAt(1).mPropertyValues.ElementAt(0).mValue.mValue.mFloatColor
$18 = {mRefCnt = {static isThreadSafe = false, mValue = 3}, _mOwningThread = {mThread = 0x7ffff6b7a5c0}, mComponent1 = 0, mComponent2 = -nan(0x400000), mComponent3 = 0, mAlpha = 1}
From CSSParserImpl::ParseHSLColor;
 http://hg.mozilla.org/mozilla-central/file/ffac2798999c/layout/style/nsCSSParser.cpp#l6896

  s = mToken.mNumber;                                                           
  if (s < 0.0f) s = 0.0f;                                                       
  if (s > 1.0f) s = 1.0f; 

We can not catch NaN value with this kind of if statement because it always becomes false.  There are several places of such if statement in nsCSSParser.cpp.

Hi, Cameron.  What should we do there?  Should we use mozilla::clamped in such places, or return false from nsCSSScanner::ScanNumber if the value is NaN?
Flags: needinfo?(cam)
I think we should prevent NaN from being returned from nsCSSScanner::ScanNumber.  Why does that function return NaN -- does it come from the pow() call?
Flags: needinfo?(cam) → needinfo?(hiikezoe)
I did not check it, but that's right.  More precisely;

 value *= pow(10.0, double(expSign * exponent));

here, the value was 0, (expSign * exponent) is +Infinity.

So we should skip the multiplication if value == 0.
Flags: needinfo?(hiikezoe)
Comment on attachment 8777272 [details]
Bug 1290994 - Do not multiply 0 by infinity in nsCSSScanner::ScanNumber.

https://reviewboard.mozilla.org/r/68862/#review65932

Looks great, thanks.

::: layout/style/nsCSSScanner.cpp:933
(Diff revision 1)
>  
>    // Time to reassemble our number.
>    // Do all the math in double precision so it's truncated only once.
>    double value = sign * (intPart + fracPart);
>    if (gotE) {
> +    // Avoid multiplication 0 by Infinity.

"Avoid multiplication of 0 by Infinity."
Attachment #8777272 - Flags: review?(cam) → review+
Comment on attachment 8777272 [details]
Bug 1290994 - Do not multiply 0 by infinity in nsCSSScanner::ScanNumber.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68862/diff/1-2/
Pushed by hiikezoe@mozilla-japan.org:
https://hg.mozilla.org/integration/autoland/rev/8f033722e3fc
Do not multiply 0 by infinity in nsCSSScanner::ScanNumber. r=heycam
Assignee: nobody → hiikezoe
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/8f033722e3fc
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1334591
See Also: → 1340344
Depends on: 1355135
See Also: → 1373712
You need to log in before you can comment on or make changes to this bug.