Closed
Bug 784828
Opened 12 years ago
Closed 12 years ago
SVG Code to query/insert into nsSVGAttrTearoffTable should use nsRefPtr instead of explicit NS_ADDREF
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: dholbert, Assigned: dholbert)
Details
Attachments
(2 files)
6.75 KB,
patch
|
longsonr
:
review+
|
Details | Diff | Splinter Review |
5.57 KB,
patch
|
longsonr
:
review+
|
Details | Diff | Splinter Review |
Currently, we've got a code-pattern across our "look up or insert into nsSVGAttrTearoffTable" code that looks like this: 55 /* static */ already_AddRefed<DOMSVGAnimatedNumberList> 56 DOMSVGAnimatedNumberList::GetDOMWrapper(SVGAnimatedNumberList *aList, 57 nsSVGElement *aElement, 58 uint8_t aAttrEnum) 59 { 60 DOMSVGAnimatedNumberList *wrapper = 61 sSVGAnimatedNumberListTearoffTable.GetTearoff(aList); 62 if (!wrapper) { 63 wrapper = new DOMSVGAnimatedNumberList(aElement, aAttrEnum); 64 sSVGAnimatedNumberListTearoffTable.AddTearoff(aList, wrapper); 65 } 66 NS_ADDREF(wrapper); 67 return wrapper; 68 } https://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/DOMSVGAnimatedNumberList.cpp#55 The manual NS_ADDREF boilerplate there is a bit boilerplate-ish and scary. I think we should clean these up to use nsRefPtr for the refcounting, and use .forget() to return our (type-safe) already_AddRefed return-value.
Assignee | ||
Updated•12 years ago
|
Summary: SVG Code to query/insert into nsSVGAttrTearoffTable should use nsRefPtr instead of NS_ADDREF → SVG Code to query/insert into nsSVGAttrTearoffTable should use nsRefPtr instead of explicit NS_ADDREF
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
QA Contact: dholbert
Assignee | ||
Comment 1•12 years ago
|
||
This patch fixes all instances of this pattern in DOMSVG*List.cpp.
Attachment #654384 -
Flags: review?
Assignee | ||
Updated•12 years ago
|
Attachment #654384 -
Flags: review? → review?(longsonr)
Assignee | ||
Comment 2•12 years ago
|
||
...and this one fixes nsSVGLength2. The patch also does the following: (a) It makes the tearoff tables in nsSVGLength2 store their actual concrete types, instead of nsIDOMWhatever types. This matches the tearoff tables elsewhere, and it lets us use nsRefPtr<ConcreteClass> when we query the table. (b) It makes the DOMBaseVal and DOMAnimVal structs public, since that's required for (a) to compile, and there's no strong reason for them to be private. (and the similar helper-class DOMAnimatedLength is already public)
Attachment #654390 -
Flags: review?(longsonr)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → dholbert
QA Contact: dholbert
Assignee | ||
Comment 3•12 years ago
|
||
Try run: https://tbpl.mozilla.org/?tree=Try&rev=d4958b0e9fda
OS: Linux → All
Hardware: x86_64 → All
Updated•12 years ago
|
Attachment #654384 -
Flags: review?(longsonr) → review+
Updated•12 years ago
|
Attachment #654390 -
Flags: review?(longsonr) → review+
Assignee | ||
Comment 4•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/165cd64fb08a https://hg.mozilla.org/integration/mozilla-inbound/rev/c80863b9bf12
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/165cd64fb08a https://hg.mozilla.org/mozilla-central/rev/c80863b9bf12
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•