Closed
Bug 691337
Opened 13 years ago
Closed 13 years ago
100% cpu svg file in tag <animateTransform> parameter "begin"
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: zpzp0909, Assigned: birtles)
References
Details
Attachments
(2 files)
1.04 KB,
image/svg+xml
|
Details | |
14.54 KB,
patch
|
longsonr
:
review+
|
Details | Diff | Splinter Review |
User Agent: Opera/9.80 (Windows NT 5.1; U; ru) Presto/2.9.168 Version/11.51 Steps to reproduce: open .svg file in browser Actual results: 100% cpu Expected results: browser should not load the CPU 100%
Updated•13 years ago
|
Component: General → SVG
Product: Firefox → Core
QA Contact: general → general
Comment 1•13 years ago
|
||
Looks like the begin time comes out as LL_MININT. This wraps around so that when you do 0 > LL_MININT and LL_MININT = 0 - LL_MININT. Perhaps SMIL needs to use LL_MININT + 1 in some places.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → birtles
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•13 years ago
|
||
Here's a proposed patch to catch situations where the time we're generating overflows the range of nsSMILTime and just ignore the time. I've also tweaked the parsing method to drop the isValid flag and just return early. (When I first wrote it I was working for a company whose coding standard discouraged early returns and I mistakenly applied that here.) I've also converted a few NS_ASSERTIONs to NS_ABORT_IF_FALSE.
Attachment #574220 -
Flags: review?(longsonr)
Comment 3•13 years ago
|
||
Comment on attachment 574220 [details] [diff] [review] Proposed patch v1a >- if (sign != 0) { >- // sign has already been set >- isValid = false; >- break; >- } >+ // check sign has not already been set >+ if (sign != 0) >+ return NS_ERROR_FAILURE; I'd rather you kept the braces even for one line tests, and fortunately the coding style agrees with me https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide :-)
Attachment #574220 -
Flags: review?(longsonr) → review+
Assignee | ||
Comment 4•13 years ago
|
||
(In reply to Robert Longson from comment #3) > I'd rather you kept the braces even for one line tests, and fortunately the > coding style agrees with me > https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide :-) Fair enough. I was trying to match the existing style (which was written at a time where there was a specific exception to that rule for early returns. In fact, the coding style still gives an example of an un-braced early return under "Return from errors immediately" snippet #4). I'm going to go ahead and break with the existing style and trust that we'll gradually add braces elsewhere (as well as replacing a number of uses of nsresult with bool).
Assignee | ||
Comment 5•13 years ago
|
||
Pushed to m-i: https://hg.mozilla.org/integration/mozilla-inbound/rev/36b19dca7f91
Whiteboard: [inbound]
Comment 6•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/36b19dca7f91
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Assignee | ||
Updated•13 years ago
|
Flags: in-testsuite+
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [inbound]
You need to log in
before you can comment on or make changes to this bug.
Description
•