430921 - chrome mochitest for bug 304188 leaks the world

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Andrew Schultz]

Attachment: leak log

With current trunk SeaMonkey, the chrome mochitest for bug 304188 (chrome/toolkit/content/tests/chrome/test_bug304188.xul) leaks the world.  This started with bug 386782.  Leak log is attached.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: v1

* Fixed the leak. It was caused by the nsSHEntry stored in nsDocShell::mOSHE holding a raw pointer to a detached mEditorData, a non-xpcom class. The nsEditorData held xpcom-references into the editor, editor session, and the editor's listeners. Somewhere in there was a cycle or something which was preventing the nsSHEntry from being collected, causing the leak. Explicitly freeing nsDocShell's mOSHE->mEditorData and mLSHE->mEditorData in nsDocShell::Destroy() fixes the leak.
* Removes the assertions in mEditorFlags must be non-zero in nsEditingSession::DetachFromWindow()/ReattachToWindow(). mEditorFlags can be zero if we have a non-interactive HTMLEditor. This occurs in the testcase that was leaking.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 318739 [details] [diff] [review]
v1

Requesting approval 1.9. This is a low risk patch which stops a memory leak.

Comment 3

[Damon Sicore (:damons)]

Comment on attachment 318739 [details] [diff] [review]
v1

a1.9+=damons

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Attachment: unbitrotted patch

Comment 5

[Reed Loden [:reed]]

Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.902; previous revision: 1.901
done
Checking in editor/composer/src/nsEditingSession.cpp;
/cvsroot/mozilla/editor/composer/src/nsEditingSession.cpp,v  <--  nsEditingSession.cpp
new revision: 1.63; previous revision: 1.62
done

Comment 6

[Reed Loden [:reed]]

Backed out to see if this is causing crashes.

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Did this cause crashes?

Comment 8

[Peter Van der Beken [:peterv]]

Looks like those were due to bug 432492. We should reland this when the tree reopens.

Comment 9

[Reed Loden [:reed]]

Relanded.

Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.904; previous revision: 1.903
done
Checking in editor/composer/src/nsEditingSession.cpp;
/cvsroot/mozilla/editor/composer/src/nsEditingSession.cpp,v  <--  nsEditingSession.cpp
new revision: 1.65; previous revision: 1.64
done

Comment 10

[Reed Loden [:reed]]

Backed out again, as we're crashing again. Either this patch or bug 422172 is causing the crashes.

Comment 11

[Reed Loden [:reed]]

Relanded (again). I forgot to repackage jemalloc, so I bet that was causing the crashes. I don't know for sure yet, though.

Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.906; previous revision: 1.905
done
Checking in editor/composer/src/nsEditingSession.cpp;
/cvsroot/mozilla/editor/composer/src/nsEditingSession.cpp,v  <--  nsEditingSession.cpp
new revision: 1.67; previous revision: 1.66
done