1682877 - Crash in [@ mozilla::ipc::FatalError | mozilla::ipc::IPDLParamTraits<T>::Read] after cancelling file picker with print to PDF

Status: RESOLVED FIXED

(Core :: Printing: Output, defect, P1)

Description

[Alice0775 White]

[Tracking Requested - why for this release]: Reproducible crash after cancel file picker when print using "Microsoft Print to PDF"

Crash report: https://crash-stats.mozilla.org/report/index/c8d7e558-a2a1-486a-87cf-d20490201216

MOZ_CRASH Reason: MOZ_CRASH(IPC FatalError in the parent process!)

Top 10 frames of crashing thread:

0 xul.dll mozilla::ipc::FatalError ipc/glue/ProtocolUtils.cpp:189
1 xul.dll static mozilla::ipc::IPDLParamTraits<mozilla::embedding::PrintData>::Read ipc/ipdl/PPrintingTypes.cpp:230
2 xul.dll mozilla::embedding::PPrintingParent::OnMessageReceived ipc/ipdl/PPrintingParent.cpp:606
3 xul.dll mozilla::dom::PContentParent::OnMessageReceived ipc/ipdl/PContentParent.cpp:14913
4 xul.dll mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2073
5 xul.dll mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:739
6 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1200
7 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:87
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:327
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:309

Steps to reproduce:

1. Open any web page. e.g. https://ftp.mozilla.org/pub/firefox/nightly/2020/11/
2. Ctrl+P and choose "Microsoft Print to PDF" and Print
3. When File picker dialog opened, cancel the dialog.

Actual results:
Browser crash

Expected results:
No crash.

Comment 1

[Andrew McCreight [:mccr8]]

IPCFatalErrorMsg is "Error deserializing 'remotePrintJobParent' (PRemotePrintJob) member of 'PrintData'".

Comment 2

[Andrew McCreight [:mccr8]]

My guess is that canceling the dialog causes some failure that leaves a field of PrintData unset, but the error isn't propagated up properly.

Comment 3

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5971cb7ff60f550622a44e9bbaebdc661aeda040&tochange=7ca6cc8d723adf6b40691494c01bfbf669b6ad2a

Comment 4

[Julien Cristau [:jcristau] (back April 22)]

We should get a fix or backout in beta today or tomorrow if at all possible, so we don't leave this crash there over the break.

Comment 5

[Blake Winton (:bwinton) (:☕️)]

I feel like this is more a Sean/Jonathan question…

Comment 6

[Mark Striemer [:mstriemer]]

I ran another mozregression and indeed this points to my commit moving the progress state to in-content.

I wasn't able to reproduce on the Wikipedia homepage, so probably safest to use https://ftp.mozilla.org/pub/firefox/nightly/2020/11/ from comment 0 to reproduce.

This appears to be related the this.settings.showPrintProgress = false; [0] line. Setting that back to true will avoid the crash.

[0] https://searchfox.org/mozilla-central/rev/8d722de75886d6bffc116772a1db8854e34ee6a7/toolkit/components/printing/content/print.js#381

Comment 7

[Julien Cristau [:jcristau] (back April 22)]

Fixed by backout for 85.0b3:
https://hg.mozilla.org/releases/mozilla-beta/rev/ea099d677676

Comment 8

[Emily McDonough [:alaskanemily]]

mstriemer: Can we close this? Has this also been fixed in nightly, or only backed out of beta?

Comment 9

[Emily McDonough [:alaskanemily]]

Whoops--didn't mean to close it.

Comment 10

[Mark Striemer [:mstriemer]]

The regressor was only backed out on Beta so it's still affecting Nightly. Comment 6 still applies for Nightly, changing settings.showPrintProgress = true avoids the crash.

Comment 11

[Boris Chiou [:boris]]

Perhaps jwatt is more suitable to give this severity. However, I give this S3 for now because the volume is not high.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

It seems like we're trying to pass around a PRemotePrintJob link as part of the print data, but that's dead already. Bob, I think you were familiar with the remote print job setup? I guess showing the native progress dialog may keep the job alive on the parent.

Comment 13

[Bob Owen (:bobowen)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)

It seems like we're trying to pass around a PRemotePrintJob link as part of the print data, but that's dead already. Bob, I think you were familiar with the remote print job setup? I guess showing the native progress dialog may keep the job alive on the parent.

Yeah, it looks like a SavePrintSettings is being sent up even when the print is cancelled or failed.
I guess it is being sent from [1].
Maybe we shouldn't send on failure, to be honest perhaps the decision over savings prefs should only be made in the parent.
We should also probably null out the RemotePrintJob that is in the PrintSession in the mPrintData->mPrintSettings at [2] as well.

[1] https://searchfox.org/mozilla-central/rev/692a10e624a02fe79bf45046b586e50ea0ff17f1/layout/printing/nsPrintJob.cpp#842-845
[2] https://searchfox.org/mozilla-central/rev/692a10e624a02fe79bf45046b586e50ea0ff17f1/widget/nsDeviceContextSpecProxy.cpp#147

Comment 14

[Daniel Bodea [:danibodea]]

I have stumbled upon this crash signature on Windows 7 with Nightly v86.0a1 (2021-01-13) and I can't reproduce it on Firefox v85.0b8.
My STR:

1. Open any page (ex. this bug).
2. Press CTRL+P for prind modal with print preview.
3. At "Destination" drop-down choose "KX DRIVER for Universal Printing" (Kyocera universal printing driver installed)
4. Click on "Print".
5. Discover Printing System window/modal is displayed; click "Cancel"
   -> CRASH:
   https://crash-stats.mozilla.org/report/index/533e892d-d5bb-4f50-915a-293880210113

I have tried reproducing it on my Windows 10 machine in v86, in hope to validate fix on v85, but I was unable. I assume it may be related to the printing driver installed.

Maybe you can validate the fix on v85 in Windows 10, Alice?

Comment 15

[Alice0775 White]

No crash on Firefox85.0b8, because new tab modal print dialog is disabled by default on Firefox85.0b8.

Still reproduce the crash bp-c470ce5b-1fd1-48ae-80fb-143c90210113 on Nightly86.0a1 Windows10.

Comment 16

[Bob Owen (:bobowen)]

After looking at a few possible options, I think the safest thing is to not try and always serialize the RemotePrintJob into the PrintData.
There is only one place [1] where we actually want to send the RemotePrintJob from child to parent and we can be sure that the parent won't have deleted it at this point.
Patch on the way.

[1] https://searchfox.org/mozilla-central/rev/2c06b16a0c15ae340a0532e319cbf89ef9d21b68/toolkit/components/printingui/ipc/nsPrintingProxy.cpp#91

Comment 17

[Bob Owen (:bobowen)]

Attachment: Bug 1682877: Only send the RemotePrintJob to the parent in PrintData for SendShowPrintDialog. r=jwatt!

Comment 18

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=TFszTx78TXq1C7S-Nb6VDg.0&duplicate_jobs=visible&revision=01904517e651adc569a56dc4d8f366d7b2fb9eeb

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c54d947541f1
Only send the RemotePrintJob to the parent in PrintData for SendShowPrintDialog. r=jwatt

Comment 20

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/c54d947541f1