Closed Bug 1360350 Opened 3 years ago Closed 3 years ago
Crash in ns
Device Context::End Document
This bug was filed from the Socorro interface and is report bp-a9fed661-63d6-49dc-83a7-0ee310170427. ============================================================= Crashing Thread (0) Frame Module Signature Source 0 XUL nsDeviceContext::EndDocument() gfx/src/nsDeviceContext.cpp:501 1 XUL nsPrintData::~nsPrintData() layout/printing/nsPrintData.cpp:77 2 XUL nsPrintEngine::CommonPrint(bool, nsIPrintSettings*, nsIWebProgressListener*, nsIDOMDocument*) layout/printing/nsPrintData.cpp:55 3 XUL nsPrintEngine::Print(nsIPrintSettings*, nsIWebProgressListener*) layout/printing/nsPrintEngine.cpp:783 4 XUL nsDocumentViewer::Print(nsIPrintSettings*, nsIWebProgressListener*) layout/base/nsDocumentViewer.cpp:3852 5 XUL nsDocumentViewer::LoadComplete(nsresult) layout/base/nsDocumentViewer.cpp:1073 6 XUL nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) docshell/base/nsDocShell.cpp:7603 7 XUL nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) docshell/base/nsDocShell.cpp:7407 8 XUL non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) docshell/base/nsDocShell.cpp:7304 9 XUL nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) uriloader/base/nsDocLoader.cpp:1255 10 XUL nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) uriloader/base/nsDocLoader.cpp:840 11 XUL nsDocLoader::DocLoaderIsEmpty(bool) uriloader/base/nsDocLoader.cpp:730 12 XUL nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) uriloader/base/nsDocLoader.cpp:612 13 XUL non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) uriloader/base/nsDocLoader.cpp:468 14 XUL mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) netwerk/base/nsLoadGroup.cpp:633 15 XUL nsDocument::DoUnblockOnload() dom/base/nsDocument.cpp:8715 16 XUL <name omitted> dom/events/AsyncEventDispatcher.cpp:98 17 XUL <name omitted> xpcom/glue/nsThreadUtils.cpp:45 18 XUL nsThread::ProcessNextEvent(bool, bool*) xpcom/glue/nsCOMPtr.h:294 19 XUL NS_ProcessPendingEvents(nsIThread*, unsigned int) xpcom/glue/nsThreadUtils.cpp:332 20 XUL nsBaseAppShell::NativeEventCallback() widget/nsBaseAppShell.cpp:97 21 XUL nsAppShell::ProcessGeckoEvents(void*) widget/cocoa/nsAppShell.mm:392 22 CoreFoundation __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ 23 CoreFoundation __CFRunLoopDoSources0 24 CoreFoundation __CFRunLoopRun 25 CoreFoundation CFRunLoopRunSpecific 26 HIToolbox RunCurrentEventLoopInMode 27 HIToolbox ReceiveNextEventCommon 28 HIToolbox _BlockUntilNextEventMatchingListInModeWithFilter 29 AppKit _DPSNextEvent 30 AppKit -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 31 CoreFoundation -[__NSArrayM insertObject:atIndex:] 32 CoreFoundation _CFRetain 33 CoreFoundation CFRunLoopAddObserver 34 CoreFoundation CFRunLoopObserverCreateWithHandler 35 AppKit -[NSApplication setWindowsNeedUpdate:] 36 AppKit __Block_byref_object_dispose_2624 37 AppKit __destroy_helper_block_2620 38 AppKit __Block_byref_object_copy_2623 39 AppKit -[NSEvent window] this cross-platform printing related crash is regressing in firefox 53 builds and onwards, which looks related to bug 1309272.
(In reply to [:philipp] from comment #0) > looks related to bug 1309272. I'm not sure what makes you think that. There's been a fair bit of churn on the printing code (and bug 1309272 isn't shipped, although some groundwork patches for it have been).
(In reply to Jonathan Watt [:jwatt] from comment #1) > I'm not sure what makes you think that. There's been a fair bit of churn on > the printing code (and bug 1309272 isn't shipped, although some groundwork > patches for it have been). in the bp-a9fed661-63d6-49dc-83a7-0ee310170427 report, frame 0 points to https://hg.mozilla.org/releases/mozilla-release/annotate/d345b657d381/gfx/src/nsDeviceContext.cpp#l501 which was last touched in 1309272 (and is coming from a firefox 53 installation). sorry if the issue is unrelated to bug 1309272 after all, but as a non-developer i can only go by correlations/coincidences like these...
No, that's all fine. I was just wondering if you were seeing something that I wasn't seeing since you didn't specify your reasons. ;)
(In reply to Jonathan Watt [:jwatt] from comment #3) > No, that's all fine. I was just wondering if you were seeing something that > I wasn't seeing since you didn't specify your reasons. ;) That change did remove a null check though.
Yeah. I don't understand why we would need that null check when we no longer create a separate PrintTarget for each individual page though. I'm trying to figure that out ATM but I haven't managed to repro the crash and nothing has jumped out from looking at the code.
One of the macOS crash report comments says that in the print settings the page size was set to "Other" and 0 x 0 mm for some reason, and that changing the page size to A4 fixed the crashes. The print dialog doesn't let me set the dimensions to 0 manually though...
What happens in this instance is that MakePrintTarget fails because it refuses to create a zero sized surface: nsDeviceContextSpecProxy::MakePrintTarget nsDeviceContext::InitForPrinting nsPrintEngine::DoCommonPrint nsPrintEngine::CommonPrint nsPrintEngine::Print nsDocumentViewer::Print That failure gets propagated up to nsPrintEngine::CommonPrint where we enter the NS_FAILED(rv) block and set mPrt to nullptr, under which we crash calling nsDeviceContext::EndDocument as reported. Ultimately I don't think the nsPrintData dtor should be calling nsDeviceContext::EndDocument if nsDeviceContext::BeginDocument has not been called.
Assignee: nobody → jwatt
Attachment #8865426 - Flags: review?(bobowencode)
Attachment #8865428 - Flags: review?(bobowencode) → review+
[Tracking Requested - why for this release]: Crashes while printing are especially rage-inducing. Asking for 53 so that it maybe can ride along in a dot-release?
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/1980dae3e06b Prevent crashes when nsPrintEngine::DoCommonPrint fails between when it sets mPrt and when it calls InitPrintDocConstruction. r=bobowen
(In reply to Nathan Froyd [:froydnj] from comment #10) > Crashes while printing are especially rage-inducing. FWIW while the crash should be gone, for these users printing will still fail due to their page size being zero. I guess at least then they have a chance to search for the "An error occurred while printing" message in the error popup dialog and find the solution on: https://support.mozilla.org/en-US/questions/1110000
Comment on attachment 8865428 [details] [diff] [review] patch Approval Request Comment [Feature/Bug causing the regression]: bug 1309272 [User impact if declined]: some users with broken settings will crash printing [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: manually by myself to force the error condition [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: just the patch here [Is the change risky?]: low risk [Why is the change risky/not risky?]: it makes sure we only call a "finish" function if the "start" function has been called [String changes made/needed]: none
Bob, assuming this gets approval, would you be able to help make sure this gets landed? I'm possibly going to find it difficult to get online over the next week or so.
(In reply to Jonathan Watt [:jwatt][back 18th May] from comment #17) > Bob, assuming this gets approval, would you be able to help make sure this > gets landed? I'm possibly going to find it difficult to get online over the > next week or so. Yes, no problem.
Comment on attachment 8865428 [details] [diff] [review] patch Fix a crash. Beta54+. Should be in 54 beta 7.
(In reply to Jonathan Watt [:jwatt][back 18th May] from comment #16) > [Is this code covered by automated tests?]: no > [Has the fix been verified in Nightly?]: manually by myself to force the > error condition > [Needs manual test from QE? If yes, steps to reproduce]: no Setting qe-verify- based on Jonathan's assessment on manual testing needs.
You need to log in before you can comment on or make changes to this bug.