Form controls' mForm pointer should be true weak reference

VERIFIED INVALID

Status

()

Core
Layout: Form Controls
VERIFIED INVALID
16 years ago
16 years ago

People

(Reporter: John Keiser (jkeiser), Assigned: John Keiser (jkeiser))

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Assignee)

Description

16 years ago
The mForm pointer in nsGenericHTML(Leaf|Container)FormElement is a raw pointer
that is not addrefed.  While there are no ways for this pointer to get
invalidated currently (we always null it out in the form when document is set to
null), it could be easily (and more safely) done using nsWeakRef, "for a small fee."

There is not much of a space problem (our neato proxy nsSupportsWeakReference
stuff makes it possible to support weak refs with just a few extra bytes in the
form).  However, there is a (small?) speed cost.  Specifically, because of the
way nsWeakRef works, it adds a QueryInterface for every function that accesses
mForm. (QueryReferent will call QueryInterface.)

More info on weak references can be found at
http://www.mozilla.org/projects/xpcom/weak_references.html.

Thanks to scc for pointing this out.
I must say I really don't think this is worh the trouble nor the cost (even if
it is small). There's nothing wrong with using raw pointers as long as you make
sure the code does the right thing, and we haven't seen this code not do the
right thing in about a year or so, so why fix it when it ain't broken? I vote
for WONTFIX.
(Assignee)

Comment 2

16 years ago
I agree there.  There isn't much extra reason to use a weak pointer and it does
add overhead.  The comments, moreover, seem sufficient.
(Assignee)

Comment 3

16 years ago
Aaaand since I am form owner.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → INVALID

Comment 4

16 years ago
Verified :-)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.