Closed Bug 336791 Opened 14 years ago Closed 14 years ago

land leak fixes making wrapped JS closures work with nsIDOMGCParticipant on 1.8 branch


(Core :: DOM: Core & HTML, defect, P1, major)






(Reporter: dbaron, Assigned: dbaron)



(Keywords: fixed1.8.1)


(2 files, 1 obsolete file)

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
Flags: blocking1.8.1?
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8.1beta1
No longer depends on: 324871
Attached patch merged patch (obsolete) — Splinter Review
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.
No longer depends on: 341096
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: superreview?(darin)
Attachment #225346 - Flags: review?(mrbkap)
Attachment #225346 - Flags: review?(mrbkap) → review+
Attachment #225343 - Flags: approval-branch-1.8.1?(darin)
Attachment #225346 - Flags: superreview?(darin) → superreview+
Attachment #225343 - Flags: approval-branch-1.8.1?(darin) → approval-branch-1.8.1+
Flags: blocking1.8.1? → blocking1.8.1+
Checked in to MOZILLA_1_8_BRANCH.
Closed: 14 years ago
Keywords: fixed1.8.1
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
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.