1652967 - Crash in [@ nsPrintJob::DoPrint]

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Simon Giesecke [:sg] [he/him]]

This bug is for crash report bp-41b5229f-f369-4232-b44c-6e76f0200715.

Top 10 frames of crashing thread:

0 xul.dll nsPrintJob::DoPrint layout/printing/nsPrintJob.cpp:2265
1 xul.dll nsPrintJob::PrintDocContent layout/printing/nsPrintJob.cpp:2139
2 xul.dll nsPrintJob::SetupToPrintContent layout/printing/nsPrintJob.cpp:1624
3 xul.dll nsPrintJob::ResumePrintAfterResourcesLoaded layout/printing/nsPrintJob.cpp:1710
4 xul.dll nsPrintJob::InitPrintDocConstruction layout/printing/nsPrintJob.cpp:1685
5 xul.dll nsPrintJob::Observe layout/printing/nsPrintJob.cpp:2912
6 xul.dll mozilla::embedding::PrintProgressDialogChild::RecvDialogOpened toolkit/components/printingui/ipc/PrintProgressDialogChild.cpp:37
7 xul.dll mozilla::embedding::PPrintProgressDialogChild::OnMessageReceived ipc/ipdl/PPrintProgressDialogChild.cpp:228
8 xul.dll mozilla::dom::PContentChild::OnMessageReceived ipc/ipdl/PContentChild.cpp:8382
9 xul.dll mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2074

Started in nightly channel with build id 20200713095122.

mPrt and therefore printData is nullptr here.

Jonathan, I think this might be a regression from your recent landing from Bug 1652344, Bug 1652342 or Bug 1652318. Could you give that a look?

Comment 1

[Sean Voisen (:svoisen)]

Adding whiteboard tags to track this as part of potential fallout from recent print refactoring.

Comment 2

[Sean Voisen (:svoisen)]

I see crashes on Windows 10 and macOS as well.

Comment 3

[Jonathan Watt [:jwatt]]

First reported in 20200714083249 (almost 20 crashes, so very likely regressed by one of the changes that first appeared in that build).

Comment 4

[Jonathan Watt [:jwatt]]

The very wide regression range (going back to the previous days Windows build) is:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=22f5f7e914449bde3bee21a322d67248cd5e0254&tochange=23131da780b50130e3430dbdc7b121bfe896bb3a

Very likely to be one of bug 1652318, bug 1652322, bug 1652342, bug 1652344 or bug 1652368.

I guess we don't need regressionrange-window given these all probably landed in the same intraday build.

Comment 5

[Jonathan Watt [:jwatt]]

The only thing I've spotted in those patches so far is the right hand operand of the || operator here is a no-op:

https://searchfox.org/mozilla-central/rev/dcd9c2d2bc19d96d487825eb70c2333a4d60994e/layout/printing/nsPrintJob.cpp#696

But that code was essentially a no-op prior to the change made in https://phabricator.services.mozilla.com/D83260 / https://phabricator.services.mozilla.com/D83262 so it doesn't look like that's it.

Comment 6

[Jonathan Watt [:jwatt]]

It looks like the crash in nsPrintJob::DoPrint is caused by printData being null, which means that mPtr must be null. But further up the stack in nsPrintJob::SetupToPrintContent we'd have returned if mPrt != printData, and after that point we dereference printData and don't crash, implying it's not null. I don't see how we can get to having null mPrt between there and the crash.

For reference, here's the stack for one of the crashes:

nsPrintJob::DoPrint
nsPrintJob::PrintDocContent
nsPrintJob::ReconstructAndReflow
nsPrintJob::ResumePrintAfterResourcesLoaded
nsPrintJob::InitPrintDocConstruction
nsPrintJob::Observe
PrintProgressDialogChild::RecvDialogOpened

Comment 7

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Watt [:jwatt] from comment #6)

I don't see how we can get to having null mPrt between there and the crash.

bobowen figured out how - we spin a nested event loop under the BeginDocument() call made by nsPrintJob::ReconstructAndReflow, since that makes a virtual call to nsDeviceContextSpecProxy::BeginDocument which calls RemotePrintJobChild::InitializePrint which spins the event loop. (I forgot we have a proxy object nowadays, so had only looked at nsDeviceContextSpecWin::BeginDocument which is a no-op.)

He also figured out how https://phabricator.services.mozilla.com/D83260 caused the issue - moving the SetIsPrinting call to before when mPrt is set causes SetIsPrinting to fail the check for mPrt and so fail to set mPrt->mPreparingForPrint.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

While poking at trying to repro bug 1655179 I found trivial STR for this on Windows at least. Posting here in case it's useful:

• Open print preview of any page.
• Click "Print".
• Select "Microsoft print to PDF" as a printer.
• While the dialog to choose the file is open, close the print preview.
• Select a filename (i.e., continue printing).
• Tab crash.

Comment 9

[Julien Cristau [:jcristau]]

This is our top content crash signature on 80.0b1

Comment 10

[Jonathan Watt [:jwatt]]

Attachment: Bug 1652967. Fix crash in nsPrintJob::DoPrint. r=bobowen

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/02c264534c96
Fix crash in nsPrintJob::DoPrint. r=bobowen

Comment 12

[Jonathan Watt [:jwatt]]

Comment on attachment 9167296 [details]
Bug 1652967. Fix crash in nsPrintJob::DoPrint. r=bobowen

Beta/Release Uplift Approval Request

• User impact if declined: topcrash
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: See comment 8.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very simple, well understood.
• String changes made/needed: none

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9167296 [details]
Bug 1652967. Fix crash in nsPrintJob::DoPrint. r=bobowen

crash fix, approved for 80.0b3

Comment 14

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/856ab495c86a

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/02c264534c96

Comment 16

[Anca Soncutean, Desktop QA]

I've reproduced the crash with Fx 80.0a1 (2020-07-24) on Windows 10.
No longer reproducible with Fx 81.0a1 (2020-08-20) - (pref'd on and off) and Fx 80 on Windows 10.