Remove heap allocated nsSVGLength in nsSVGUtils::CoordToFloat

RESOLVED FIXED

Status

()

Core
SVG
--
trivial
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Craig Topper, Assigned: Craig Topper)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
nsSVGUtils::CoordToFloat uses a short lived heap allocated nsSVGLength to do a calculation. This can be replaced with a stack allocated nsSVGLength2 to get the same effect.
(Assignee)

Comment 1

10 years ago
Created attachment 357090 [details] [diff] [review]
Patch
Attachment #357090 - Flags: review?(roc)
Attachment #357090 - Flags: superreview+
Attachment #357090 - Flags: review?(roc)
Attachment #357090 - Flags: review+
Keywords: checkin-needed
Whiteboard: [needs landing]
Can you remove the include too?

Also, any reason to not use:

  case eStyleUnit_Percent:
    nsSVGSVGElement* ctx = aContent->GetCtx();
    return ctx ? aCoord.GetPercentValue() * ctx->GetLength(nsSVGUtils::XY) : 0;
(Assignee)

Comment 3

10 years ago
I considered doing it, but the real length code also has a check for GetLength returning 0.0f and changes it to 1e-20. So I was trying to make this code simple by reuse existing stuff.

While typing this and re-reading the code I realized the 1e-20 was to prevent a divide by 0. Obviously there is no division in your proposed code. I'll go ahead and make the change.
(Assignee)

Comment 4

10 years ago
Created attachment 357120 [details] [diff] [review]
Changes to jwatt's suggestion
Attachment #357090 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Attachment #357120 - Flags: review?(roc)
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Whiteboard: [needs landing]
(Assignee)

Updated

10 years ago
Summary: Replace heap allocated nsSVGLength in nsSVGUtils::CoordToFloat with stack allocated nsSVGLength2 → Remove heap allocated nsSVGLength in nsSVGUtils::CoordToFloat
Attachment #357120 - Flags: superreview+
Attachment #357120 - Flags: review?(roc)
Attachment #357120 - Flags: review+
Keywords: checkin-needed
Whiteboard: [needs landing]
Pushed http://hg.mozilla.org/mozilla-central/rev/07c89c67e4b9
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.