1427109 - PDFium printing paths should not be enabled by default on Windows

Status: RESOLVED FIXED

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

Description

[Jonathan Watt [:jwatt]]

We currently shouldn't be taking any of the PDFium printing code paths introduced in bug 1399787. The define MOZ_ENABLE_SKIA_PDF is set by default in Nightly, so guarding entry into the PDFium code paths based on that define is not correct. Entry needs to be guarded by the pref print.print_via_pdf_encoder=skia-pdf.

We get this wrong it at least one place, which has resulted in bug 1425895. We should audit the use of MOZ_ENABLE_SKIA_PDF in the code landed for bug 1399787:

  hg diff -r6d431fb67f3a:972a3efac0d7

Comment 1

[u459114]

I guess the reason is mPrintViaSkPDF was not initiated as false.

Comment 2

[u459114]

In this push, gecko will crash if we ever try to create PrintTargetEMF
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e1bdfa7fc3049c4ef58b91c636b58ba51ba3fa8&selectedJob=154979367

Seeing the try result, we do create PrintTargetEMF on these three platform:
Windows 10 x64 opt 	
Windows 10 x64 pgo 	
Windows 10 x64 debug

Then, I initialize mPrintViaSkPDF in the ctor of nsDeviceContextSpecWin, this problem been fixed:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bc037d1a2192558a1beb6b480fc7597063e7b17

So, confirm the root cause.

Comment 3

[u459114]

Attachment: Bug 1427109 - Initialize mPrintViaSkPDF as false to guarantee PrintTargetEMF is used only if 'print.print_via_pdf_encoder == skia-pdf'.

Review commit: https://reviewboard.mozilla.org/r/211592/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/211592/

Comment 4

[u459114]

Comment on attachment 8941300 [details]
Bug 1427109 - Initialize mPrintViaSkPDF as false to guarantee PrintTargetEMF is used only if 'print.print_via_pdf_encoder == skia-pdf'.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211592/diff/1-2/

Comment 5

[Jonathan Watt [:jwatt]]

Comment on attachment 8941300 [details]
Bug 1427109 - Initialize mPrintViaSkPDF as false to guarantee PrintTargetEMF is used only if 'print.print_via_pdf_encoder == skia-pdf'.

https://reviewboard.mozilla.org/r/211592/#review218002

Thanks, C.J.!

::: widget/windows/nsDeviceContextSpecWin.cpp:100
(Diff revision 2)
>  };
>  
>  //----------------------------------------------------------------------------------
>  nsDeviceContextSpecWin::nsDeviceContextSpecWin()
> +#ifdef MOZ_ENABLE_SKIA_PDF
> + : mPrintViaSkPDF(false)

Indent one more space, please.

Comment 6

[u459114]

Comment on attachment 8941300 [details]
Bug 1427109 - Initialize mPrintViaSkPDF as false to guarantee PrintTargetEMF is used only if 'print.print_via_pdf_encoder == skia-pdf'.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/211592/diff/2-3/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/179d25b30000
Initialize mPrintViaSkPDF as false to guarantee PrintTargetEMF is used only if 'print.print_via_pdf_encoder == skia-pdf'. r=jwatt

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/179d25b30000