615002 - "ABORT: Got a regular sample during startup state, expected an end sample instead"

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase (asserts fatally when loaded)

###!!! ABORT: Got a regular sample during startup state, expected an end sample instead: 'mElementState != STATE_STARTUP || aEndOnly', file content/smil/nsSMILTimedElement.cpp, line 506

Comment 1

[Brian Birtles (:birtles)]

Attachment: Patch v1a

Proposed patch.

As described in the comment in the patch, the problem occurs when we register a milestone before t=0s (t=-2s in this case) and then get re-bound to the tree which sends us back to the startup state.

Comment 2

[Daniel Holbert [:dholbert]]

Comment on attachment 495730 [details] [diff] [review]
Patch v1a

One nit on the test...

>+++ b/content/smil/crashtests/615002-1.svg
>+  document.documentElement.appendChild(a);
>+  // Allow time for a sample before finishing
>+  setTimeout(
>+    function() { document.documentElement.removeAttribute("class"); }, 1);
>+}

I don't think we actually guarantee that a SMIL sample will happen before that setTimeout fires, right?

And even if that does reliably work, it's still best to avoid unnecessary setTimeouts when better alternatives are available.  I'd suggest:
 - adding setCurrentTime(0) to force a synchronous sample before clearing reftest-wait
...or...
 - using some combination of "window.mozRequestAnimationFrame()" and probably MozAfterPaint (see mxr for sample usage)

r=dholbert with that test-fix.

Comment 3

[Brian Birtles (:birtles)]

Attachment: Patch v1b; r=dholbert

(In reply to comment #2)
> And even if that does reliably work, it's still best to avoid unnecessary
> setTimeouts when better alternatives are available.  I'd suggest:
>  - adding setCurrentTime(0) to force a synchronous sample before clearing
> reftest-wait

Fixed. Thanks Daniel!

Comment 4

[Brian Birtles (:birtles)]

Requesting approval to land. Fixes fatal assertion. Includes test.

Comment 5

[Brian Birtles (:birtles)]

Pushed: http://hg.mozilla.org/mozilla-central/rev/4dbbc3061278