Crash in [@ nsDeviceContext::UnregisterPageDoneCallback]
Categories
(Core :: Printing: Output, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox-esr78 | --- | unaffected |
firefox79 | --- | unaffected |
firefox80 | --- | wontfix |
firefox81 | --- | verified |
People
(Reporter: emilghitta, Assigned: hiro)
References
(Blocks 1 open bug)
Details
(Keywords: crash, Whiteboard: [print2020_v81])
Crash Data
Attachments
(3 files)
This bug is for crash report bp-305554e6-f7b5-4073-bede-1239f0200807.
Top 10 frames of crashing thread:
0 xul.dll nsDeviceContext::UnregisterPageDoneCallback
1 xul.dll nsPrintJob::DonePrintingPages layout/printing/nsPrintJob.cpp:2519
2 xul.dll nsPrintJob::FinishPrintPreview layout/printing/nsPrintJob.cpp:2701
3 xul.dll nsDocumentViewer::Destroy layout/base/nsDocumentViewer.cpp:1760
4 xul.dll nsDocShell::Destroy docshell/base/nsDocShell.cpp:4214
5 xul.dll nsWebBrowser::SetDocShell toolkit/components/browser/nsWebBrowser.cpp:1131
6 xul.dll nsWebBrowser::InternalDestroy toolkit/components/browser/nsWebBrowser.cpp:174
7 xul.dll nsWebBrowser::Destroy toolkit/components/browser/nsWebBrowser.cpp:855
8 xul.dll mozilla::dom::BrowserChild::DestroyWindow dom/ipc/BrowserChild.cpp:896
9 xul.dll mozilla::dom::BrowserChild::RecvDestroy dom/ipc/BrowserChild.cpp:2344
Affected Platforms:
- Windows 10 64bit
- macOS 10.14 (see comment 2)
Unaffected Platforms:
- Ubuntu 18.04 64bit.
Preconditions:
Ensure that print.tab_modal.enabled
is set to true
.
Steps to reproduce
- Launch Firefox.
- Access the following link.
- Open the print preview via the "Hamburger" menu.
- As soon as the print preview tries to open, rapidly hit the ESC button (you may have to redo steps 3 & 4 a couple of times to hit this)
Expected Result
- Print preview is successfully closed and Firefox is stable.
Actual Result
- Tab Crash
Regression Range
- I don't think that this is a regression
Additional Information
- [Suggested Severity] This is edgy enough to not be considered as an S1 bug, but I think that S2 fits here.
Reporter | ||
Comment 1•4 years ago
|
||
Reporter | ||
Comment 2•4 years ago
|
||
I just encountered this crash on macOS & Windows (maybe Ubuntu is affected as well..I will check on Monday) with different STR (only the step 4 is different from comment 0):
Step 4: Increment the number of copies from the "Copies" counter. At about 6 copies it will crash lead to a tab crash with this signature bp-11db3293-d232-4e5a-86a3-a66f30200808
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
Comment 3•4 years ago
|
||
jwatt, perhaps you could take a look here? [Tagging you as needinfo for now to be sure this is on someone's radar, but feel free to farm it out as-needed]
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
When I try to reproduce the crash locally on a debug build, I got an assertion MOZ_ASSERT(mIsUAWidget)
in ShadowRoot::ImportNodeAndAppendChildAt. I am trying to bisect to tell which commit causes the assertion. (I am unsure whether it's related to this crash though).
CCing Emilo (he might beat me).
Assignee | ||
Comment 5•4 years ago
|
||
Also note that, as far as I can tell, the assertion in comment 4 happens on the old print preview UI as well.
Comment 6•4 years ago
|
||
Hmm, I think that's unrelated. ShadowRoot::CloneInternalDataFrom
seems it should copy its mIsUaWidget
bit from aOther
.
Assignee | ||
Comment 7•4 years ago
|
||
Thank you, Emilo. Filed bug 1658469 for the assertion.
Assignee | ||
Comment 8•4 years ago
|
||
In either cases of the STR (in comment 0 or in comment 2), nsPrintJob::DonePrintingPages gets called before printData->mPrintDC gets initialized in nsPrintJob::DoCommonPrint at line 892.
In the case of comment 0, nsPrintJob::DonePrintingPages gets called via the ShowPrintDialog call at line 820.
In the case of comment 2, nsPrintJob::DonePrintingPages gets called via the InitAsRootObject call at line 691.
Though adding a null check for mPrintDC just before using the printData->mPrintDC should fix the crash, the mPrintDC usage was introduced in this commit, and it seems it's supposed to be called only in the parent process? (with checking IsSyncPagePrinting?), so we should also check whether it's in the parent process I guess.
Also note that, even with the null check, the STR in comment 0 still causes another crash at line 864 in DoCommonPrint. At the line, printSession
is nullptr since we try to get (or create) an nsIPrintSession only if mIsCreatingPrintPreview is false
, but in the case, mIsCreatingPrintPreview
is initially true, then it's set to false later in FinishPrintPreview().
So I suppose we should store the original mIsCreatingPrintPreview
value before the nsAutoScriptBlocker scope in DoCommonPrint, then we should check the value was changed along with mIsDestroying
check. I am also supposing we should also check the value change after the ShowPrintDialog
call along with mPrt != printData
check
I am unsure whether similar situations will happen in printing though.
Updated•4 years ago
|
Assignee | ||
Comment 9•4 years ago
|
||
The call sites of nsDeviceContext::RegisterPageDoneCallback are in the parent
process along with the IsSyncPagePrinting check.
Assignee | ||
Comment 10•4 years ago
|
||
I've uploaded the first part of the fix in comment 8. I am now suspecting the second part is not a right way to fix it.
I hope the first part will mitigate the QA process.
Assignee | ||
Comment 11•4 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)
Also note that, even with the null check, the STR in comment 0 still causes another crash at line 864 in DoCommonPrint.
In this case, given that the call stack in comment 0 is including nsDocumentViewer::Destroy, and in the function we call FinishPrintPreview() then call nsPrintJob::Destroy(), so I suppose we should check mIsDestroying after the ShowPrintDialog call. That said, unfortunately I can no longer reproduce the crash at lin 864 now without the change...
Assignee | ||
Comment 12•4 years ago
|
||
I've got another assertion in the STR in comment 0. The assertion is MOZ_ASSERT(!mIsBeingDestroyed)
at the top of nsDocShell::MaybeCreateInitialClientSource. This is a pretty bad signal.
The reason why the assertion happens is that in enterPrintPreview in PrintingChid.jsm we dispatch a function call to the main thread and in the function call we call initOrReusePrintPreviewViewer. In the meantime, if the print preview window gets destroyed, the docshell is not longer useful.
I am going to upload an additional fix for this case.
Assignee | ||
Comment 13•4 years ago
|
||
It's possible that the doc shell is being if the print preview window gets
closed soon after it's opened.
Comment 14•4 years ago
•
|
||
Hello,
I managed to encounter this crash signature with the following steps on macOS10.15.6:
- Launch FF.
- Go to https://en.wikipedia.org/wiki/Facebook
- Toggle between Portrait/Landscape a few times until Print Preview is no longer loading.
- Tab crashes. (in some instances it took around 1-2 minutes for the tab to crash)
Firefox 81.0a1 Crash Report [@ nsDeviceContext::UnregisterPageDoneCallback ]
This bug is for crash report bp-11c1aa2d-c833-469a-b930-b5bba0200813.
Top 10 frames of crashing thread:
0 XUL nsDeviceContext::UnregisterPageDoneCallback gfx/src/nsDeviceContext.cpp:713
1 XUL nsPrintJob::DonePrintingPages layout/printing/nsPrintJob.cpp:2519
2 XUL nsPrintJob::FinishPrintPreview layout/printing/nsPrintJob.cpp:2703
3 XUL nsPrintJob::MaybeResumePrintAfterResourcesLoaded layout/printing/nsPrintJob.cpp:1647
4 XUL {virtual override thunk}
5 XUL nsDocLoader::DoFireOnStateChange uriloader/base/nsDocLoader.cpp:1331
6 XUL nsDocLoader::FireOnStateChange uriloader/base/nsDocLoader.cpp:1294
7 XUL nsDocLoader::doStopURLLoad uriloader/base/nsDocLoader.cpp:898
8 XUL nsDocLoader::OnStopRequest uriloader/base/nsDocLoader.cpp:622
9 XUL {virtual override thunk}
Assignee | ||
Comment 15•4 years ago
|
||
Thank you for another STR, Vlad!
I can reproduce it locally on a debug build, and I got the MOZ_ASSERT(!mIsBeingDestroyed)
I wrote in comment 12. Now it makes me convince that the patch in comment 12 is a right fix.
Reporter | ||
Comment 16•4 years ago
|
||
Encountered another crash with this signature bp-e8ac739f-0b76-4440-9182-0051e0200813 (Different STR):
- Launch Firefox.
- Access the following link
- Open print preview.
- Click print as soon as the print preview is displayed (don't let the document load inside the print preview).
Reproduced on Windows 10 64bit & macOS 10.14
Comment 17•4 years ago
|
||
Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a34b649bd8f8 Bail out from printPreviewInitialize function if the doc shell is being destroyed during dispatching the function to the main-thread. r=jwatt
Comment 18•4 years ago
|
||
Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/977b2bfa0122 Call nsDeviceContext::UnregisterPageDoneCallback only in the parent process and only if IsSyncPagePrinting. r=jwatt
Comment 19•4 years ago
|
||
bugherder |
Assignee | ||
Comment 20•4 years ago
|
||
Hey Emil, has the crash been stopped on the latest nightly? If so, I am going to uplift the patches to beta.
Reporter | ||
Comment 21•4 years ago
|
||
Hey Hiro.
I wanted to give a little baking time for this issue since this has been triggered by using different STR. We have conducted another testing run today and we didn't encounter this crash signature using latest Nightly (with the steps provided in this bug & with our exploratory testing).
Assignee | ||
Comment 22•4 years ago
|
||
Thank you Emil!
I am closing this bug and it turns out we don't need to uplift the fix to beta since the new print preview UI pref is enabled on EARLY_BETA_OR_EARLIER
.
Updated•4 years ago
|
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Comment 23•4 years ago
|
||
Description
•