Last Comment Bug 699325 - Assertion failure in nsSMILAnimationFunction::ComputePacedPosition when doing paced animation between identical values
: Assertion failure in nsSMILAnimationFunction::ComputePacedPosition when doing...
Status: RESOLVED FIXED
: assertion, testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla10
Assigned To: Daniel Holbert [:dholbert]
:
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-02 21:25 PDT by Jesse Ruderman
Modified: 2011-11-04 11:38 PDT (History)
3 users (show)
dholbert: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (see comment 0) (162 bytes, image/svg+xml)
2011-11-02 21:25 PDT, Jesse Ruderman
no flags Details
stack trace (1.62 KB, text/plain)
2011-11-02 21:29 PDT, Jesse Ruderman
no flags Details
testcase 2 (209 bytes, image/svg+xml)
2011-11-03 10:23 PDT, Daniel Holbert [:dholbert]
no flags Details
fix (1.37 KB, patch)
2011-11-03 11:50 PDT, Daniel Holbert [:dholbert]
no flags Details | Diff | Splinter Review
fix (now with crashtest) (2.12 KB, patch)
2011-11-03 12:48 PDT, Daniel Holbert [:dholbert]
bbirtles: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2011-11-02 21:25:41 PDT
Created attachment 571554 [details]
testcase (see comment 0)

With
  user_pref("browser.display.use_document_colors", false);
this testcase triggers

###!!! ASSERTION: shouldn't complete loop & get here -- if we do, then aSimpleProgress was probably out of bounds: 'Not Reached', file content/smil/nsSMILAnimationFunction.cpp, line 616

(Reduced from layout/reftests/svg/smil/style/anim-css-color-2-paced-rgb.svg)
Comment 1 Jesse Ruderman 2011-11-02 21:29:40 PDT
Created attachment 571556 [details]
stack trace
Comment 2 Daniel Holbert [:dholbert] 2011-11-03 10:23:15 PDT
Created attachment 571696 [details]
testcase 2

Looks like we're hitting this because the distances between the list-values are all 0, so we never hit the return clause inside the loop.

Here's a simpler testcase animating "x".  This one only triggers the assertion once per simple-duration, so I've added a fancy repeating 'begin' attr to make it fire once per second.
Comment 3 Daniel Holbert [:dholbert] 2011-11-03 11:37:46 PDT
(fixing summary. note that the assertion-failure is actually inside of ComputePacedPosition)
Comment 4 Daniel Holbert [:dholbert] 2011-11-03 11:50:23 PDT
Created attachment 571727 [details] [diff] [review]
fix
Comment 5 Daniel Holbert [:dholbert] 2011-11-03 12:48:46 PDT
Created attachment 571747 [details] [diff] [review]
fix (now with crashtest)

Here's the patch again, now with a crashtest based off of testcase 2 (so as not to need any about:config-pref-tweaking).

I verified that the crashtest fails (due to unexpected assertion) inside the crashtest harness, but passes once the fix is applied.
Comment 6 Brian Birtles (:birtles) 2011-11-03 17:23:27 PDT
Comment on attachment 571747 [details] [diff] [review]
fix (now with crashtest)

Review of attachment 571747 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great. Thanks Daniel!
Comment 7 Daniel Holbert [:dholbert] 2011-11-04 01:15:27 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/22b16b552828
Comment 8 Matt Brubeck (:mbrubeck) 2011-11-04 11:38:25 PDT
https://hg.mozilla.org/mozilla-central/rev/22b16b552828

Note You need to log in before you can comment on or make changes to this bug.