1659432 - Crash in [@ nsPrintJob::DoCommonPrint]

Status: VERIFIED FIXED

(Core :: Print Preview, defect, P1)

Description

[Emil Ghitta, QA [:emilghitta]]

This bug is for crash report bp-bffb8373-9cd1-4cb8-b5e3-9a22a0200817.

Top 10 frames of crashing thread:

0 xul.dll nsPrintJob::DoCommonPrint layout/printing/nsPrintJob.cpp:831
1 xul.dll nsPrintJob::CommonPrint layout/printing/nsPrintJob.cpp:586
2 xul.dll nsPrintJob::PrintPreview layout/printing/nsPrintJob.cpp:960
3 xul.dll nsDocumentViewer::PrintPreview layout/base/nsDocumentViewer.cpp:3226
4 xul.dll XPTC__InvokebyIndex 
5  @0x26b735324ff 
6 xul.dll trunc 
7 xul.dll static XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1141
8 xul.dll XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:946
9 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:599

Affected versions

• 81.0a1 (BuildId:20200816214203)

Affected platforms

• Windows 10 64bit
• macOS 10.14
• Ubuntu 18.04 64bit.

Preconditions

• Ensure that the print.tab_modal.enabled pref is set to true

Steps to reproduce

1. Launch Firefox.
2. Access the following page
3. Open print preview.
4. Switch between orientatons while the print preview is still loading.

Expected result

• Print preview switches orientation once the document is successfully loaded inside print preview.

Actual result

• Tab Crash

Regression Window

• Will get back with a regression range asap.

Additional Information

• For further information regarding this issue in print preview observe the attached screencast.
• [Suggested Seveirty] S2

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Attachment: doCommonPrint.gif

Comment 2

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Managed to reproduce this crash signature with the same repro steps as provided in Description on a macOS 10.15.6

It is worth mentioning that encountering this crash on this OS, will render the browser to be completely unusable as it freezes permanently and a Force Quit is required.

Screen cast of the issue: https://drive.google.com/file/d/1SWrTVc9_pmwwPEU68wUebDMxTlgYoh8n/view?usp=sharing

Comment 3

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

Seems like a Core::Print Preview to me, but feel free to throw it back to us if you disagree, Sean… :)

Comment 4

[Hiroyuki Ikezoe (:hiro)]

My wild guess on this is this is another symptoms that mIsCreatingPrintPreview flag was changed during the print preview initialization.

CCing :bobowen.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1659432 - Stop listening the previous progress listener when we are going to process a new print preview request during processing the previous one. r?emilio

Note that the progress listener here is not the given listener as an argument
of the printPreview function, it's nsPrintJob itself.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

I just uploaded a patch to fix this crash without review requests since I haven't succeeded in writing automated tests yet. It seems sjs file doesn't get loaded as a script file in chrome mochitest? I don't know, haven't traced down in detail.

I am really worried about this crash, but jwatt wants me to finish bug 1657550 first (since it's blocking frontend work), so I am going to head over to the bug now.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

And I am also ~100% sure the patch fixes the unexpected print dialog issue (bug 1659300).

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1659432 - Stop listening the previous progress listener when we are going to process a new print preview request during processing the previous one. r?emilio

Note that the progress listener here is not the given listener as an argument
of the printPreview function, it's nsPrintJob itself.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 9170573 [details]
Bug 1659432 - Stop listening the previous progress listener when we are going to process a new print preview request during processing the previous one. r?emilio

I think I came up with an idea to defer calling MaybeResumePrintAfterResourcesLoaded in nsPrintJob::OnStateChange(), but Emilo and Bob agreed on another way to fix this crash.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #9)

Comment on attachment 9170573 [details]
Bug 1659432 - Stop listening the previous progress listener when we are going to process a new print preview request during processing the previous one. r?emilio

I think I came up with an idea to defer calling MaybeResumePrintAfterResourcesLoaded in nsPrintJob::OnStateChange(), but Emilo and Bob agreed on another way to fix this crash.

Well, looks like we don't need to defer the MaybeResumePrintAfterResourcesLoaded call in the case where the next request is coming, we can just skip calling MaybeResumePrintAfterResourcesLoaded, FWIW.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1659432 - Don't reuse the existing print job when restarting print preview. r=bobowen

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c75183b39d1c
Don't reuse the existing print job when restarting print preview. r=bobowen

Comment 13

[Andreea Pavel [:apavel]]

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

Comment 14

[Emil Ghitta, QA [:emilghitta]]

I am no longer able to reproduce this issue using Firefox 81.0a1 (BuildId:20200819212829) on Windows 10 64bit, macOS 10.14 and Ubuntu 20.04.

Comment 15

[Maria Berlinger [:mberlinger], Services QA]

https://testrail.stage.mozaws.net/index.php?/cases/view/1084776