Closed
Bug 196273
Opened 22 years ago
Closed 14 years ago
Using nsRefPtr on nsCSSDeclaration may improve code readability
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Whiteboard: [whitebox])
In particular, the AddRef/Release rules in nsCSSStyleRule could go if it used an
nsRefPtr member; we may also be able to eliminate the various RuleAbort() calls
(replacing them with a simple AddRef/Release pair by passing a getter_AddRefs to
the NS_NewWhatever function)....
bz, I don't quite get it why there can't be a leak.
With |GetCSSDeclaration(decl, /*aAllocate=*/PR_TRUE)|, there is a code path that
does a |new|, and so with the early return that I pointed in bug 171830 comment
37 the |decl| will be left dangling, am I missing something?
Never mind... got it. SetCSSDeclaration() passes the ownership to somebody along
the way.
Assignee | ||
Comment 3•22 years ago
|
||
Yeah; I should just comment the ownership model here more clearly....
Updated•22 years ago
|
Whiteboard: [whitebox]
Assignee | ||
Comment 4•22 years ago
|
||
So the real question here is whether we should stick to the whole "only
nsCSSStyleRule can refcount nsCSSDeclaration" thing....
Assignee | ||
Comment 5•22 years ago
|
||
I should just take this. I'll be making nsCSSDeclaration a normal refcounted
object, a la nsStyleContext.
Assignee: dbaron → bzbarsky
Priority: -- → P2
Target Milestone: --- → mozilla1.5beta
Updated•18 years ago
|
QA Contact: ian → style-system
Assignee | ||
Comment 6•14 years ago
|
||
Zack, is this at all still relevant after your changes?
Assignee | ||
Updated•14 years ago
|
Target Milestone: mozilla1.5beta → ---
Comment 7•14 years ago
|
||
I don't think this makes sense as is after my changes, no. We might be able to do nsAutoPtr instead for the CSSStyleRuleImpl->css::Declaration pointer, though.
Assignee | ||
Comment 8•14 years ago
|
||
OK, not, going to worry about this then.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•