Closed Bug 1658196 Opened 1 year ago Closed 1 year ago

Crash in [@ mozilla::layout::PRemotePrintJobChild::SendInitializePrint]

Categories

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

80 Branch
All
Windows
defect

Tracking

()

VERIFIED FIXED
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox79 --- unaffected
firefox80 --- wontfix
firefox81 --- verified

People

(Reporter: philipp, Unassigned)

References

Details

(Keywords: crash, regression, Whiteboard: [print2020_v81])

Crash Data

Attachments

(1 file)

This bug is for crash report bp-4eed8673-e9c7-48bb-aa0b-81b8a0200809.

Top 10 frames of crashing thread:

0 xul.dll mozilla::layout::PRemotePrintJobChild::SendInitializePrint ipc/ipdl/PRemotePrintJobChild.cpp:77
1 xul.dll mozilla::layout::RemotePrintJobChild::InitializePrint layout/printing/ipc/RemotePrintJobChild.cpp:27
2 xul.dll nsDeviceContextSpecProxy::BeginDocument widget/nsDeviceContextSpecProxy.cpp:140
3 xul.dll nsDeviceContext::BeginDocument gfx/src/nsDeviceContext.cpp:523
4 xul.dll nsPrintJob::SetupToPrintContent layout/printing/nsPrintJob.cpp:1597
5 xul.dll nsPrintJob::MaybeResumePrintAfterResourcesLoaded layout/printing/nsPrintJob.cpp:1731
6 xul.dll nsPrintJob::OnStateChange layout/printing/nsPrintJob.cpp:1754
7 xul.dll nsDocLoader::DoFireOnStateChange uriloader/base/nsDocLoader.cpp:1331
8 xul.dll nsDocLoader::FireOnStateChange uriloader/base/nsDocLoader.cpp:1294
9 xul.dll nsDocLoader::doStopURLLoad uriloader/base/nsDocLoader.cpp:898

this is a low volume content crash signature while printing, that's starting to show up in firefox 80.0b - i'm unsure about what might have regressed it.

jwatt: could this be a regression from some of your recent print infrastructure changes for getting us out of nested event loops etc?

(Note that it's showing up in Firefox 80 builds, so it would've hypothetically regressed from something in that timeline, i.e. a month or so ago.)

Severity: -- → S2
Flags: needinfo?(jwatt)
Whiteboard: [print2020]
Attached image crash_windows.gif

I've reproduced this crash signature with Fx 81.0a1 on Windows 10 x64 by simple pressing the Print button from the new Print modal twice; sometimes the crash is trigger directly, sometimes I have to interact with the popup message error.

Has STR: --- → yes
Whiteboard: [print2020] → [print2020_v81]

I think this is another version of bug 1657911, but happens on printing instead of on print preview.

(In reply to Anca Soncutean [:Anca], Desktop Release QA from comment #2)

I've reproduced this crash signature with Fx 81.0a1 on Windows 10 x64 by simple pressing the Print button from the new Print modal twice; sometimes the crash is trigger directly, sometimes I have to interact with the popup message error.

Just wondering what is the expected behavior on the twice clicking print? Two sets of the print?

Looks like subsequent print requests would be discarded basically, but yeah in this case unfortunately the document just finished loading so that the check fails?

I've confirmed that adding (mPrintJob && mPrintJob->IsDoingPrint())here mitigates the crash though, I think the print button should be disabled until the Promise returned by print function gets fullfilled.

I will leave the decision up to :jwatt.

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

I've confirmed that adding (mPrintJob && mPrintJob->IsDoingPrint())here mitigates the crash though, I think the print button should be disabled until the Promise returned by print function gets fullfilled.

I will leave the decision up to :jwatt.

hiro - can you reproduce this crash then?

I'd briefly looked at it and it appeared to be that we had already had a failure causing mRemotePrintJob to be set to null at [1]
Then this was being recalled on docshell destruction (I don't see nsDocumentViewer::Print in the stack), presumably because some other changes had meant we were now recalling this by mistake.
It would be good to find that mistake, but given how much all of this is changing currently, I was just going to add a null check at the top of nsDeviceContextSpecProxy::BeginDocument.

[1] https://searchfox.org/mozilla-central/rev/50cb0892948fb4291b9a6b1b30122100ec7d4ef2/widget/nsDeviceContextSpecProxy.cpp#147

Flags: needinfo?(hikezoe.birchill)

I haven't disabled the button, so I am not 100% sure it fixes the crashes entirely. With the IsDoingPrint IIRC I still saw a crash, it was quite hard to reproduce though.

(In reply to Bob Owen (:bobowen) from comment #6)

It would be good to find that mistake, but given how much all of this is changing currently, I was just going to add a null check at the top of nsDeviceContextSpecProxy::BeginDocument.

Though I am not sure what you mean by "mistake", a mistake I noticed is that the docshell busy check in nsDocumentViewer::Print.

Since I saw the crash with the IsDoingPrint, I tried to find a place to tell whether "we are not yet ready to process the next printing", but I couldn't find it so I'd suggest to disable the print button.

Flags: needinfo?(hikezoe.birchill)

An additional note I should share is that there are two different crashes; one is triggered by nsLoadGroup::Cancel (bp-4eed8673-e9c7-48bb-aa0b-81b8a0200809), the other is triggered by nsLoadGroup::RemoveRequest (bp-ea7189bf-3a01-4e24-92b2-cfaa70200812). I think the latter is a kind of normal operation case, and what I saw with the IsDoinPtint is this case.

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

I haven't disabled the button, so I am not 100% sure it fixes the crashes entirely. With the IsDoingPrint IIRC I still saw a crash, it was quite hard to reproduce though.

So, when you do get the crash does it match this signature and stack?

(In reply to Bob Owen (:bobowen) from comment #6)

It would be good to find that mistake, but given how much all of this is changing currently, I was just going to add a null check at the top of nsDeviceContextSpecProxy::BeginDocument.

Though I am not sure what you mean by "mistake", a mistake I noticed is that the docshell busy check in nsDocumentViewer::Print.

By mistake I meant some fairly recent change meant that we were recalling RemotePrintJobChild::InitializePrint after it had previously failed.
Also, like I said, I don't see nsDocumentViewer::Print in the stack, do you think that is related because of another code path leading up to this crash?

Flags: needinfo?(hikezoe.birchill)

(In reply to Bob Owen (:bobowen) from comment #9)

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

I haven't disabled the button, so I am not 100% sure it fixes the crashes entirely. With the IsDoingPrint IIRC I still saw a crash, it was quite hard to reproduce though.

So, when you do get the crash does it match this signature and stack?

Yes, I saw the both case I mentioned in comment 8.

(In reply to Bob Owen (:bobowen) from comment #6)

It would be good to find that mistake, but given how much all of this is changing currently, I was just going to add a null check at the top of nsDeviceContextSpecProxy::BeginDocument.

Though I am not sure what you mean by "mistake", a mistake I noticed is that the docshell busy check in nsDocumentViewer::Print.

By mistake I meant some fairly recent change meant that we were recalling RemotePrintJobChild::InitializePrint after it had previously failed.
Also, like I said, I don't see nsDocumentViewer::Print in the stack, do you think that is related because of another code path leading up to this crash?

I suppose this crash is caused by multiple (in the least case it's two) print requests. And I suppose the later print request is submitted or breaks the previous request (I believe the latter is plausible), and when the previous request is processed by the doc loader state change (either it's normally loaded or stopped by an error), the crash happens.

Flags: needinfo?(hikezoe.birchill)

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

So, when you do get the crash does it match this signature and stack?

Yes, I saw the both case I mentioned in comment 8.

Ah, apologies hiro, I missed this second comment to my needinfo. :-)

...

I suppose this crash is caused by multiple (in the least case it's two) print requests. And I suppose the later print request is submitted or breaks the previous request (I believe the latter is plausible), and when the previous request is processed by the doc loader state change (either it's normally loaded or stopped by an error), the crash happens.

Right sorry for being slow to get this.

For what it's worth I think disabling the Print button, until the print has completed should happen (if the below doesn't).
However to be honest I think we should close the new combined Print/Print Preview dialog when the Print button is clicked.
That's what happens with the native dialog and that's what Chrome does.

Our old Print Preview doesn't though.
One argument against closing might be that our page range printing doesn't yet allow multiple ranges, but I still think that would be a minority of cases, so having to Ctrl+P again shouldn't be too bad.
We could even add a tick box to "Keep Preview open after printing" or something for special cases.

Depends on: 1659622

(In reply to Bob Owen (:bobowen) from comment #11)

For what it's worth I think disabling the Print button, until the print has completed should happen (if the below doesn't).

I filed bug 1659622.

However to be honest I think we should close the new combined Print/Print Preview dialog when the Print button is clicked.
That's what happens with the native dialog and that's what Chrome does.

I filed bug 1659624, but this needs platform work. See the comment there.

Our old Print Preview doesn't though.

Oh! I really thought it did, but then I'm a mac user.

Flags: needinfo?(jwatt)
Priority: -- → P1

I am expecting bug 1659432 also fixed this.

Anca or Emil, can you still see the crash on your side?

Flags: needinfo?(emil.ghitta)
Flags: needinfo?(anca.soncutean)

I can no longer reproduce this on Win 10 ARM since bug 1659432 landed. Instead, double-clicking the Print button gives the "Some printing functionality is not currently available" error, but no crash. Bug 1659622 and bug 1659624 should help mitigate this.

We've had no Nightly 81 crashes since 20200818092452.

This is verified fixed with Fx 81.0a1 (2020-08-20) across platforms (Windows 10, Ubuntu 18.04 and macOS 10.14) since I can no longer trigger the crash.

Flags: needinfo?(emil.ghitta)
Flags: needinfo?(anca.soncutean)

Thank you, everyone.

Status: NEW → RESOLVED
Closed: 1 year ago
Depends on: 1659432
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.