Closed Bug 423233 Opened 15 years ago Closed 12 years ago
cycle collect libeditor
Editor holds references to input elements as soon as you open the browser.
Comment on attachment 309723 [details] [diff] [review] partial patch This patch identifies some references to nsGenericElements that were previously unaccounted for.
OS: Mac OS X → All
Hardware: PC → All
Version: unspecified → Trunk
attachment 309723 [details] [diff] [review] basically still applied, except for some whitespace-only changes that we're better off without anyway. I also changed a drop of commenting-out into removal. This fixes bug 420670. Sayre, was there any reason you hadn't requested review on this back when you wrote it?
Attachment #309723 - Attachment is obsolete: true
(In reply to comment #3) > > Sayre, was there any reason you hadn't requested review on this back when you > wrote it? I think it's because I couldn't actually show improved CC behavior. I assumed it was docshell's alleged thread safety preventing adequate collection. But since you find it fixes bugs, we should probably take it.
So I can't reproduce the testing that I did a few days ago that shows it fixes bug 420670. But I *can* reproduce that this *plus* bug 488799 (add editor transactions and transaction manager to cycle collector) fixes bug 420670. And I tried it about 3 times both with and without the patch this time. That said, the above patch also makes crashtests crash. I'll post a patch with the fix; nsHTMLEditRules derives from nsTextEditRules but (without my revision) didn't forward to its QueryInterface. My patch makes the QueryInterface implementation forward (and thus get the cycle collection helper and cycle collection isupports entries) and also switches the canonical nsISupports pointer to the forwarded one.
Comment on attachment 373270 [details] [diff] [review] partial patch, udpated to fix crash >diff --git a/editor/libeditor/text/nsTextEditRules.cpp b/editor/libeditor/text/nsTextEditRules.cpp >+NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsTextEditRules) >+ NS_INTERFACE_MAP_ENTRY(nsIEditRules) This is missing the entry for nsISupports. Adding unlinking always makes me nervous in case we end up executing code that didn't expect the pointers to be null. We'll wee how it goes :-).
This fixes the crashes I was seeing on mochitests on Mac only (why Mac only? maybe the ordering of malloc influencing the ordering of Unroot?). In addition, I added the following two lines to the main patch: + if (tmp->mRules) + tmp->mRules->DetachEditor(); in nsPlaintextEditor's unlink method.
Comment on attachment 373270 [details] [diff] [review] partial patch, udpated to fix crash These patches make leak-monitor and DEBUG_CC a good bit less noisy, and may well fix some more permanent leaks.
Comment on attachment 376341 [details] [diff] [review] pre-patch a=shaver
Attachment #376341 - Flags: approval1.9.1? → approval1.9.1+
Looks like this never got resolved. Marking fixed.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.