548899 - "ASSERTION: Resample dirty flag set during sample!" on full-animate-elem-30-t.html SVG animation test

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

When loading URL in a mozilla-central debug build, I get many copies of:

###!!! ASSERTION: Resample dirty flag set during sample!: '!mResampleNeeded',
file ../../../mozilla/content/smil/nsSMILAnimationController.cpp, line 387
assertion-spam from full-animate-elem-30-t.html.

It looks like we get one assertion-failure per SMIL sample.

I think the strategy described at the end of bug 483584 comment 12 would fix this.  To summarize, I'm pretty sure the problem here is that in  nsSVGTransformSMILAttr::SetAnimValue, we're calling DidModify to issue notifications for the transform attribute changing, and that makes us request a new SMIL sample (while we're already sampling).  Instead of DidModify, we should be calling a new method ("DidAnimateTransform"?) that would trigger the necessary invalidation & repainting, but that would NOT make us request a new sample (nor would it make us re-clone the content for the <use> element).

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (repro's 50% of the time)

Here's a reduced testcase that reproduces the assertion-spam about 50% of the times that I reload it. (The nondeterminism is probably due to the random ordering in which we traverse our animation targets during a sample.)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix: add "DidAnimateTransform"

This just adds a "DidAnimateTransform" method, just like all the other DidAnimateXXX methods, and it appears to work fine.

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to comment #2)
> and it appears to work fine.

That is to say: it fixes the assertions (since we no longer notify), and it doesn't break any tests -- passed a TryServer run, earlier today.

Comment 4

[Daniel Holbert [:dholbert]]

pushed: http://hg.mozilla.org/mozilla-central/rev/6776f09c1bc6