Closed Bug 336791 Opened 14 years ago Closed 14 years ago
land leak fixes making wrapped JS closures work with ns
IDOMGCParticipant on 1 .8 branch
241.62 KB, patch
|Details | Diff | Splinter Review|
41.19 KB, patch
|Details | Diff | Splinter Review|
This is a tracking bug for landing the collection of leak fixes (and regression fixes following up on them) related to nsIDOMEventListener and XMLHttpRequest leaks on the 1.8 branch.
Severity: normal → major
Status: NEW → ASSIGNED
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8.1beta1
This is the necessary patches merged to the branch (except for bug 324865, which I landed separately this afternoon). The next step will be reducing the interface changes; this is simply what landed on the trunk merged to the branch.
This patch has all the interface changes taken out (at a 1-word cost for a bunch of objects, most significantly nsImageLoadingContent) except for the changes to nsIContent. (Taking those out would be pretty trivial, but it's a 1-word cost for all elements.)
Attachment #225107 - Attachment is obsolete: true
This is an interdiff showing the changes I made to the merge result in order to preserve API compatibility. It excludes some fixes to incorrect merging shown in attachment 225107 [details] [diff] [review] (conflicts in nsHTMLSharedObjectElement.cpp and a missed piece that needed to be merged into nsGenericDOMDataNode.cpp as well). Note when reading these interdiffs that a bunch of the patch segments begin with "reverted:", which I presume means that all of the changes in the file in the first patch are not present in the second. My notes on the merging are as follows (fixed up since previous attachment): Bugs merged: bug 241518 bug 319293 bug 319642 bug 322985 bug 323807 bug 323534 bug 325279 bug 326646 bug 339412 bug 324865 (in separate tree; already landed) bug 206520 bug 321054 bug 339488 I could fix nsDOMAttribute for branch/trunk differences, but having too much reachability shouldn't be a problem, so I may as well just leave it matching the trunk even though nsDOMAttribute ownership is (IIRC) different on the branch. I could remove nsIDOMGCParticipant from nsIContent to preserve nsIContent API compatibility too, at a 1-word cost for each object. If so, I'd need to deal with both nsGenericElement and nsGenericDOMDataNode.
Attachment #225346 - Flags: review?(mrbkap) → review+
Attachment #225343 - Flags: approval-branch-1.8.1?(darin)
Attachment #225343 - Flags: approval-branch-1.8.1?(darin) → approval-branch-1.8.1+
Checked in to MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Is it possible not to modify nsIContent? > Checked in to MOZILLA_1_8_BRANCH. >
(In reply to comment #5) > Is it possible not to modify nsIContent? Not without increasing the size of all content nodes by 4 bytes or something a good bit more complicated. This was discussed a good bit on mozilla-dev-planning before I landed.
So make it by increasing size of all content nodes by 4 bytes (no interface change) in mozilla 1_8 branch. And make it by the right way (changing interface) in mozilla 1_9 branch. Because FF 1.5 and FF 2.0 APIs must be compatible as maximum as possible. For me, i dont' wish to rebuild my extension dll because just one mozilla's interface has being changed.
Dbaron: Would it make sense to add this interface as a tearoff and make sure to have a pretty generous cache for the tearoffs? I guess you'd have to make separate tearoffs for nsGenericElement, nsDocument and nsGenericDOMDataNode, which would kind'a suck (and cost some time).
No point having a tearoff from nsDocument, and no point having a tearoff if there isn't a rarely-allocated slots object in which the pointer to it can be stored, but that would work. I don't plan to work on it, though.
On elements we do have a rarly allocated slots object where we can stick a pointer (nsGenericElement::nsDOMSlots), but i'm not sure if sticking the pointer in there will cause us to create lots and lots more of those slots? I.e. how often we QI to nsIGCParticipant
could this change be related to problems in https://bugzilla.mozilla.org/show_bug.cgi?id=344699
You need to log in before you can comment on or make changes to this bug.