I guess this is also a fallout from bug 534785. The attached testcase crashes in current trunk build on print preview or when closing print preview. http://crash-stats.mozilla.com/report/index/6bb2dd74-5ca8-4c5c-9a57-6f0f82100910 0 xul.dll nsTextEditorState::InitializeKeyboardEventListeners content/html/content/src/nsTextEditorState.cpp:1813 1 xul.dll nsHTMLInputElement::InitializeKeyboardEventListeners content/html/content/src/nsHTMLInputElement.cpp:4280 2 xul.dll nsTextControlFrame::SetInitialChildList layout/forms/nsTextControlFrame.cpp:1430 3 xul.dll nsCSSFrameConstructor::ConstructFrameFromItemInternal layout/base/nsCSSFrameConstructor.cpp:3862 4 xul.dll nsCSSFrameConstructor::ConstructFramesFromItem layout/base/nsCSSFrameConstructor.cpp:5452 5 xul.dll nsCSSFrameConstructor::ConstructFramesFromItemList layout/base/nsCSSFrameConstructor.cpp:9430 6 xul.dll nsCSSFrameConstructor::ProcessChildren layout/base/nsCSSFrameConstructor.cpp:9546 7 xul.dll nsCSSFrameConstructor::ConstructBlock layout/base/nsCSSFrameConstructor.cpp:10596 8 xul.dll nsCSSFrameConstructor::ConstructDocElementFrame layout/base/nsCSSFrameConstructor.cpp:2483 9 xul.dll nsCSSFrameConstructor::ContentRangeInserted layout/base/nsCSSFrameConstructor.cpp:6848 10 xul.dll nsCSSFrameConstructor::ContentInserted layout/base/nsCSSFrameConstructor.cpp:6747 11 xul.dll PresShell::InitialReflow layout/base/nsPresShell.cpp:2645 etc..

I couldn't reproduce this (on a Mac). Can you please provide the exact steps to reproduce?

- Load testcase - Click on File->Print Preview - Close Print Preview Mac doesn't have print preview, afaik, so you probably should try on linux or windows.

(In reply to comment #2) > - Load testcase > - Click on File->Print Preview > - Close Print Preview > Mac doesn't have print preview, afaik, so you probably should try on linux or > windows. Yes, but its printing code should use the same code path as Windows and Linux... Can you reproduce this by dropping the testcase in layout/forms/crashtest and adding the reftest-print class to the HTML element?

Afaik, that is setting the testcase in page-mode, according to this code: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest.js#831 And it can very well not crash in page mode, but still crash in print preview.

I'm not getting a crash in page mode.

It turns out this regressed between 2010-07-12 and 2010-07-14: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2010-07-12+03%3A00%3A00&enddate=2010-07-14+09%3A00%3A00 During that time period, it regressed from not crashing at all, to crashing directly on print preview (in current trunk builds, it's crash on print preview close). I guess a regression from bug 289384.

I managed to reproduce it under Linux...

InitializeKeyboardEventListeners tries to grab the first child frame, which might not exist for the duplicate frames. This patch makes sure that doesn't happen.

Nominating as blocking, since this is a regression crash.

Backed out: http://hg.mozilla.org/mozilla-central/rev/e9fbcd30350b This showed up on Linux debug and Linux 64 debug. The test failure in browser_privatebrowsing_protocolhandler.js showed up on Linux 64 opt. s: talos-r3-fed-007 TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 373235 bytes during test execution TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 140 instances of AtomImpl with size 20 bytes each (2800 bytes total) TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of BackstagePass with size 24 bytes TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 2 instances of CSSImportRuleImpl with size 48 bytes each (96 bytes total) TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 46 instances of CSSImportantRule with size 16 bytes each (736 bytes total) TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 11 instances of CSSNameSpaceRuleImpl with size 44 bytes each (484 bytes total) TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | application timed out after 330 seconds with no output PROCESS-CRASH | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_protocolhandler.js | application crashed (minidump found) Thread 0 (crashed) TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!

This was not the cause of the orange, and should be relanded.

Backed out again because of leaks...

The leaks were coming from the test, as I suspected. This patch fixes this.

Comment on attachment 477645 [details] [diff] [review] Patch (v2) Isn't the fact that that test caused leaks a bug in our printpreview code or thereabouts? Better file it.

(In reply to comment #17) > Isn't the fact that that test caused leaks a bug in our printpreview code or > thereabouts? Better file it. Filed bug 599080.

So I was mistaken in thinking that the version 2 of this patch doesn't leak. It indeed does. It seems like the contenteditable attribute on the input element, and having the |page-break-before:left;| set is what triggers the leak...

Didn't this always leak. Afaik, it's normal that print preview leaks.

(In reply to comment #20) > Didn't this always leak. Yes, the leak is not a result of my patch, it was only uncovered by the test I added. > Afaik, it's normal that print preview leaks. What do you mean it's normal? We shouldn't be leaking, should we?

Sorry, I didn't mean it's normal, just that there are probably many more cases of leaking in print preview.

(In reply to comment #22) > Sorry, I didn't mean it's normal, just that there are probably many more cases > of leaking in print preview. That might be the case, but this particular leak has to be addressed before I can land this patch anyway...

Well, as long as it gets fixed before 2.0, then, I guess. It would be bad if this would not get fixed, because the patch gets the code back into a state where it's leaking instead of crashing.

OK, I finally figured this leak out! It's related to leaking frames if we fail to inject them in the frame tree. I'm going to post a patch in bug 599080, so this bug will be dependent on bug 599080.