Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 784828 - SVG Code to query/insert into nsSVGAttrTearoffTable should use nsRefPtr instead of explicit NS_ADDREF
: SVG Code to query/insert into nsSVGAttrTearoffTable should use nsRefPtr inste...
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Daniel Holbert [:dholbert]
: Jet Villegas (:jet)
Depends on:
  Show dependency treegraph
Reported: 2012-08-22 14:53 PDT by Daniel Holbert [:dholbert]
Modified: 2012-08-23 19:22 PDT (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch 1: DOMSVG*List (6.75 KB, patch)
2012-08-22 15:22 PDT, Daniel Holbert [:dholbert]
longsonr: review+
Details | Diff | Splinter Review
patch 2: nsSVGLength2 (5.57 KB, patch)
2012-08-22 15:32 PDT, Daniel Holbert [:dholbert]
longsonr: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2012-08-22 14:53:31 PDT
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 }

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.
Comment 1 Daniel Holbert [:dholbert] 2012-08-22 15:22:51 PDT
Created attachment 654384 [details] [diff] [review]
patch 1: DOMSVG*List

This patch fixes all instances of this pattern in DOMSVG*List.cpp.
Comment 2 Daniel Holbert [:dholbert] 2012-08-22 15:32:02 PDT
Created attachment 654390 [details] [diff] [review]
patch 2: nsSVGLength2

...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)
Comment 3 Daniel Holbert [:dholbert] 2012-08-22 15:33:23 PDT
Try run:

Note You need to log in before you can comment on or make changes to this bug.