Last Comment Bug 531550 - "ASSERTION: Trying to do sandwich add of more than one value" with svg:animateTransform
: "ASSERTION: Trying to do sandwich add of more than one value" with svg:animat...
Status: RESOLVED FIXED
: assertion, testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla2.0b8
Assigned To: Brian Birtles (:birtles)
:
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks: stirdom
  Show dependency treegraph
 
Reported: 2009-11-28 12:04 PST by Jesse Ruderman
Modified: 2010-11-23 15:38 PST (History)
7 users (show)
bbirtles: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final+


Attachments
testcase (114 bytes, image/svg+xml)
2009-11-28 12:04 PST, Jesse Ruderman
no flags Details
Patch v1a (2.45 KB, patch)
2010-11-18 18:31 PST, Brian Birtles (:birtles)
no flags Details | Diff | Splinter Review
Patch v1b (2.64 KB, patch)
2010-11-23 04:04 PST, Brian Birtles (:birtles)
dholbert: review+
Details | Diff | Splinter Review

Description User image Jesse Ruderman 2009-11-28 12:04:55 PST
Created attachment 414924 [details]
testcase

###!!! ASSERTION: Trying to do sandwich add of more than one value.: 'srcTransforms.Length() == 1', file /Users/jruderman/central/content/svg/content/src/nsSVGTransformSMILType.cpp, line 169

###!!! ASSERTION: invalid array index: 'i < Length()', file ../../../../dist/include/nsTArray.h, line 326

The first assertion is misleading: srcTransforms.Length() is actually 0.
Comment 1 User image Jesse Ruderman 2010-11-16 20:20:37 PST
This prevents me from finding other "ASSERTION: invalid array index" bugs, which are often security holes :(
Comment 2 User image Brian Birtles (:birtles) 2010-11-18 18:31:53 PST
Created attachment 491746 [details] [diff] [review]
Patch v1a

Proposed patch.
Comment 3 User image Daniel Holbert [:dholbert] 2010-11-18 21:37:09 PST
Comment on attachment 491746 [details] [diff] [review]
Patch v1a

>+  // [...] but since the duration is indefinite we'll actually try
>+  // to add it.

I'm confused about what this means -- why does an indefinite duration mean we'll actually try to add, when we wouldn't otherwise?
Comment 4 User image Brian Birtles (:birtles) 2010-11-19 02:45:19 PST
(In reply to comment #3)
> I'm confused about what this means -- why does an indefinite duration mean
> we'll actually try to add, when we wouldn't otherwise?

Yeah, good point this comment needs to be tweaked. For by-animation, normally the interpolation step would ensure that we don't end up with an empty transform animation value. However, when there's an indefinite duration we skip interpolation altogether and just set the first value (the empty 'from' value we'd normally interpolate from).

However, I think there are other cases where this can arise too such as when we have values="1". So I'll tweak the comment to make this clearer.
Comment 5 User image Brian Birtles (:birtles) 2010-11-23 04:04:48 PST
Created attachment 492618 [details] [diff] [review]
Patch v1b

Fix up comment as per comment 3.
Comment 6 User image Daniel Holbert [:dholbert] 2010-11-23 08:20:56 PST
Comment on attachment 492618 [details] [diff] [review]
Patch v1b

Makes much more sense now -- thanks for clarifying that! r=dholbert
Comment 7 User image Brian Birtles (:birtles) 2010-11-23 15:38:21 PST
Pushed: http://hg.mozilla.org/mozilla-central/rev/b578485c389e

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