535691 - "ASSERTION: unknown nsISVGValue" with animateTransform

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: unknown nsISVGValue: 'Not Reached', file /Users/jruderman/mozilla-central/content/svg/content/src/nsSVGElement.cpp, line 969

Comment 1

[Daniel Holbert [:dholbert]]

This is WORKSFORME in a Linux debug build from today. I get this output at pageload:
{
nsSVGStringProxyValue CTOR
nsSVGStringProxyValue(0xaf971b40)::SetValueString(10 20)
  -> call failed, now using cached value
}
but no assertion failures.

Jesse, mind verifying?

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to comment #1)
> nsSVGStringProxyValue CTOR
> nsSVGStringProxyValue(0xaf971b40)::SetValueString(10 20)
>   -> call failed, now using cached value

FWIW, this appears to be non-scary output, from glancing at the source:
http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/nsSVGStringProxyValue.cpp#112
http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/nsSVGStringProxyValue.cpp#150

Comment 3

[Daniel Holbert [:dholbert]]

(see also Bug 556808, for another instance of this "unknown nsISVGValue" assertion failure.)

Comment 4

[Jesse Ruderman]

Yeah, WFM.  I'll add this as a crashtest.

Comment 5

[Jesse Ruderman]

Crashtest added: http://hg.mozilla.org/mozilla-central/rev/d3e7c18f58fc

Comment 6

[Henri Sivonen (:hsivonen)]

The patch for bug 552938 regresses this. Why might that be?

Comment 7

[Henri Sivonen (:hsivonen)]

I can't reproduce the problem on either Mac 32-bit or Linux 64-bit. How should I proceed with the patch for bug 552938? Would it be too inappropriate to land it and reopen this bug?

Comment 8

[Robert Longson [:longsonr]]

I don't get any asserts on Windows with the testcase with your patch from bug 552938.

Comment 9

[Jesse Ruderman]

I wouldn't especially mind.  Be sure to add asserts(0-1) or asserts(1) to the crashtest manifest so you don't turn the tree orange.

Comment 10

[Henri Sivonen (:hsivonen)]

Attachment: Add an assertion annotation

Comment 11

[Jesse Ruderman]

Comment on attachment 449588 [details] [diff] [review]
Add an assertion annotation

I take it the assertion can happen up to twice?

Please add a comment on the line pointing back to this bug, so we're more likely to remember to remove the asserts() if this bug becomes WFM again:

+asserts(0-2) load 535691-1.svg # bug 535691

Btw, if a stack trace from Try would help you figure out why this happens, you can push your patch for bug 552938 with my patch for bug 570287.

Comment 12

[Henri Sivonen (:hsivonen)]

(In reply to comment #11)
> (From update of attachment 449588 [details] [diff] [review])
> I take it the assertion can happen up to twice?
> 
> Please add a comment on the line pointing back to this bug, so we're more
> likely to remember to remove the asserts() if this bug becomes WFM again:
> 
> +asserts(0-2) load 535691-1.svg # bug 535691

Thanks. Landed with the comment:
http://hg.mozilla.org/mozilla-central/rev/d36a5c77457f

> Btw, if a stack trace from Try would help you figure out why this happens, you
> can push your patch for bug 552938 with my patch for bug 570287.

OK. Thanks.

Comment 13

[Brian Birtles (:birtles)]

This should now be fixed by bug 603917, specifically changeset http://hg.mozilla.org/mozilla-central/rev/4e06966e8dfe

I'll wait to check that everything is fine on tinderbox (TryServer was all good) and, unless there are any objections, mark this fixed.