Closed
Bug 690994
Opened 12 years ago
Closed 12 years ago
"ABORT: Attempting to make self-dependent instance time"
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: jruderman, Assigned: birtles)
References
Details
(Keywords: assertion, testcase)
Attachments
(3 files)
###!!! ABORT: Attempting to make self-dependent instance time: '!mEnd || aEnd.GetBaseTime() != mEnd', file content/smil/nsSMILInterval.cpp, line 134
Reporter | ||
Comment 1•12 years ago
|
||
Might depend on GC timing.
Reporter | ||
Comment 2•12 years ago
|
||
Assignee | ||
Comment 3•12 years ago
|
||
This will almost certainly be my fault, but I'm away for the next month. If someone else feels brave, feel free to take it or otherwise assign it to me.
![]() |
||
Comment 4•12 years ago
|
||
Assigning to Brian at the moment so it doesn't fall off the radar.
Assignee: nobody → birtles
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•12 years ago
|
||
Proposed patch. The root problem was that we were testing for self-referential end times in the general case, but not in the specific case where we detected two zero-duration intervals in a row. All the fix really requires is to move the zero-duration checking inside the loop where we check for self-referential times. However, in trying to fix this I realised that this code is not very clear and not strictly correct either so I made the following changes: * Move the comment explaining the do...while loops next to the 'while' condition since generally that makes a lot more sense * Change the check for coincident zero-duration intervals to tempEnd->Time() == beginAfter which is basically a simplification of: tempBegin->Time() == tempEnd->Time && tempBegin->Time() == beginAfter (beginAfter is generally the end of the previous interval. In the couple of cases where it's not, prevIntervalWasZeroDur is set to false.) * This second change also allowed removing some code at the start of the method (which was confusing and possibly wrong) * Other tweaks to comments and code to make it easier to read
Attachment #573428 -
Flags: review?(dholbert)
Comment 6•12 years ago
|
||
Try run for a53d59484ede is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=a53d59484ede Results (out of 214 total builds): exception: 1 success: 194 warnings: 17 failure: 2 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bbirtles@mozilla.com-a53d59484ede
Comment 7•12 years ago
|
||
Comment on attachment 573428 [details] [diff] [review] Patch v1a Looks good -- just some comment nits: >+ // SMIL doesn't allow for coincident zero-duration intervals so if the >+ // previous interval was zero-duration, and tempEnd is going to give us >+ // another zero duration interval then look for another end to use >+ // instead. Readability nit: add comma before "so if the" and "then look for". (makes it easier to read, for me at least. :)) >+ // When we choose the interval endpoints we don't allow coincident >+ // zero-duration intervals so if we arrive here and we have a zero-duration >+ // interval starting at the same point as a previous zero-duration interval >+ // then it must be because we've applied constraints to the active duration. >+ // In that case we will potentially run into an infinite loop so we break it >+ // by searching for the next interval that starts AFTER our current >+ // zero-duration interval. >+ if (prevIntervalWasZeroDur && tempEnd->Time() == beginAfter) { This comment is a bit complex & would also get a readability-win from some commas. :) In particular, seems like we could use a comma before "we don't allow coincident", "so if we arrive", "then it must be", and "so we break it". r=me with that.
Attachment #573428 -
Flags: review?(dholbert) → review+
Assignee | ||
Comment 8•12 years ago
|
||
Thanks Daniel! Pushed with review feedback addressed: https://hg.mozilla.org/integration/mozilla-inbound/rev/dc82c3485d1a
OS: Mac OS X → All
Hardware: x86_64 → All
Whiteboard: [inbound]
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dc82c3485d1a
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Assignee | ||
Updated•12 years ago
|
Flags: in-testsuite+
Whiteboard: [inbound]
You need to log in
before you can comment on or make changes to this bug.
Description
•