Closed Bug 691337 Opened 8 years ago Closed 8 years ago

100% cpu svg file in tag <animateTransform> parameter "begin"

Categories

(Core :: SVG, defect)

7 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: zpzp0909, Assigned: birtles)

References

Details

Attachments

(2 files)

Attached image svg0.svg
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%
Component: General → SVG
Product: Firefox → Core
QA Contact: general → general
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: nobody → birtles
Status: NEW → ASSIGNED
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 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+
(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).
https://hg.mozilla.org/mozilla-central/rev/36b19dca7f91
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
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.