Closed Bug 1663426 Opened 1 year ago Closed 1 year ago

Crash in [@ nsDocumentViewer::SetDocumentInternal]

Categories

(Core :: Printing: Setup, defect)

All
Windows
defect

Tracking

()

VERIFIED FIXED
82 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox80 --- unaffected
firefox81 --- verified
firefox82 --- verified
firefox83 --- verified
firefox84 --- verified

People

(Reporter: emilghitta, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, regression, Whiteboard: [print2020_v81] [old-ui?])

Crash Data

Attachments

(2 files)

Attached image crash.gif

Crash report: https://crash-stats.mozilla.org/report/index/e9259672-2850-42b3-b419-97ec20200907

Top 10 frames of crashing thread:

0 xul.dll nsDocumentViewer::SetDocumentInternal layout/base/nsDocumentViewer.cpp:1858
1 xul.dll nsGlobalWindowOuter::Print dom/base/nsGlobalWindowOuter.cpp:5441
2 xul.dll nsFrameLoader::PrintPreview dom/base/nsFrameLoader.cpp:3286
3 xul.dll mozilla::dom::FrameLoader_Binding::printPreview_promiseWrapper dom/bindings/FrameLoaderBinding.cpp:884
4 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ConvertExceptionsToPromises> dom/bindings/BindingUtils.cpp:3227
5 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:599
6 xul.dll Interpret js/src/vm/Interpreter.cpp:3336
7 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:636
8 xul.dll JS::Call js/src/jsapi.cpp:2831
9 xul.dll mozilla::dom::IdleRequestCallback::Call dom/bindings/WindowBinding.cpp:865

Affected versions

  • Firefox 81.0b7 (BuildId:20200906164749)
  • Firefox 82.0a1 (BuildId:20200907094115)

Affected Platforms

  • Windows 10 64bit.

Unaffected Platforms

  • macOS 10.14
  • Ubuntu 18.04 64bit

Steps to reproduce

  1. Launch firefox.
  2. Access any random webpage and print it via "Save to PDF"
  3. Access about:sessionrestore.
  4. Hit CTRL + P
  5. Change the destination several times.

Expected Result

  • FIrefox is stable.

Actual Result

  • Firefox crashes.

Looks like the documentviewer's previous mDocument can be null, and the code here shouldn't assume it can deref it? The code in nsGlobalWindowOuter::Print has changed a bit recently, and I'm not sure if previously this couldn't happen, but now it can, or something...

Flags: needinfo?(emilio)

I assume this is related to bug 1659470.

(In reply to :Gijs (he/him) from comment #1)

Looks like the documentviewer's previous mDocument can be null, and the code here shouldn't assume it can deref it? The code in nsGlobalWindowOuter::Print has changed a bit recently, and I'm not sure if previously this couldn't happen, but now it can, or something...

Yeah. Note that the previous mDocument should be valid basically (e.g. about:blank etc.). It looks like we call nsDocumentViewer::OnDonePrinting accidentally? (due to bug 1659470?)

See Also: → 1659470
Component: Printing → Printing: Setup
Product: Toolkit → Core

I can just reproduce the crash with these steps;

  1. Open whatever about pages (e.g. about: preferences)
  2. Ctrl+P
  3. Click "OK" button in the print preview error dialog
  4. Change orientation in the print preview window

I'd say we should close the print preview window when the OK button in the error dialog is pressed until we fix bug 1659470?.

  1. Please make sure the print destination is "Print to PDF".
QA Whiteboard: [qa-regression-triage]

I fixed the underlying error in the other bug, but null-checking here is harmless.

Flags: needinfo?(emilio)

This shouldn't generally happen, but seems it can under some circumstances and
even though I've fixed the error condition that triggers this a null-check here is harmless.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/10fc7701db1b
Add a null-check to SetDocumentInternal. r=bobowen
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

Please nominate this for Beta approval.

Comment on attachment 9174376 [details]
Bug 1663426 - Add a null-check to SetDocumentInternal. r=jwatt,bobowen

Beta/Release Uplift Approval Request

  • User impact if declined: Crashes
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): null-check
  • String changes made/needed: none
Flags: needinfo?(emilio)
Attachment #9174376 - Flags: approval-mozilla-beta?

Comment on attachment 9174376 [details]
Bug 1663426 - Add a null-check to SetDocumentInternal. r=jwatt,bobowen

Approved for 81.0b9.

Attachment #9174376 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-regression-triage] → [qa-regression-triage][qa-triaged]
Has Regression Range: --- → yes
Has STR: --- → yes

Hello,

Confirming this issue as verified fixed on 82.0.2, 83.0b7 and 84.0a1 with Windows 10x64.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.