We're actually leaking quite a bit more than this, but none of these classes are annotated.
With more classes marked, the root of the leak becomes apparent. We're leaking a txUnknownHandler when the call at http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xslt/txUnknownHandler.cpp#67 because we return before deleting the txUnknownHandler. Hopefully this is enough information for someone who knows this code to fix this.
Created attachment 483732 [details] [diff] [review] Mark classes for leak logging This makes the leak here more apparent. I also added MOZ_COUNT_[C|D]TOR_INHERITED for more accurate stats on classes that inherit from a class that is already marked for leak logging.
Comment on attachment 483732 [details] [diff] [review] Mark classes for leak logging r=me, but definitely want dbarons input on the new macros
This should be easy enough to fix now that the problem is known. Note that this same pattern happens several times in that file, so it's probably possible to similar testcases that leak different things.
Created attachment 488206 [details] [diff] [review] v1
Fix is in txUnknownHandler::createHandler, the rest is mostly cleanup.
There's still an assertion that we're hitting (because we don't have a root node in the source, style or result documents).
Comment on attachment 488206 [details] [diff] [review] v1 Huh?? So none of the flushToHandler implementations replaced the handler? Was that just a leftover?
Comment on attachment 483732 [details] [diff] [review] Mark classes for leak logging sr=dbaron
Backed out due to 64 ERROR TEST-UNEXPECTED-FAIL | /tests/content/xslt/tests/mochitest/test_bug551412.html | Result of transform should be '1 <b>2</b> 3' - got "<b></b>", expected "1 <b>2</b> 3" http://hg.mozilla.org/mozilla-central/rev/c994a3574125
The test assert too, which needs to be noted.
Not going to block on this at this late point. If a safe fix for this appears, we would consider taking it.
Created attachment 505406 [details] [diff] [review] v1.1 The patch did this: nsAFlatString::const_char_iterator& start = aIter; nsAFlatString::const_char_iterator end = start + charTransaction->mLength; aIter = end; start is a reference to aIter, so at this point start == aIter == end, and then we do: return aHandler->characters(Substring(start, end), Changing start to not be a reference fixed it.
Created attachment 524674 [details] [diff] [review] v1.2 With fixes for an assertion triggered by the test. Got r=sicking in person.
http://hg.mozilla.org/mozilla-central/rev/a6822a5df633 http://hg.mozilla.org/mozilla-central/rev/c1ef3c3740cb http://hg.mozilla.org/mozilla-central/rev/47f6e81257f9
6 years ago
Mozilla/5.0 (X11; Linux i686; rv:8.0a1) Gecko/20110719 Firefox/8.0a1 Running the test-case from Comment 0, displays a Rectangle with a 0 inside of it. Was that the intended behavior for this patch? Thanks!
I'm guessing the patch wasn't intended to change the appearance of the testcase, just whether it causes Firefox to report a memory leak when it shuts down after displaying the testcase.