PDFium printing paths should not be enabled by default on Windows

RESOLVED FIXED in Firefox 59

Status

()

P1
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jwatt, Assigned: u459114)

Tracking

unspecified
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
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
(Reporter)

Updated

a year ago
Priority: -- → P1
(Assignee)

Updated

a year ago
Assignee: nobody → cku
(Assignee)

Comment 1

a year ago
I guess the reason is mPrintViaSkPDF was not initiated as false.
(Assignee)

Comment 2

a year ago
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 hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8941300 - Flags: review?(jwatt)
(Reporter)

Comment 5

a year ago
mozreview-review
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.
Attachment #8941300 - Flags: review?(jwatt) → review+
Comment hidden (mozreview-request)

Comment 7

a year ago
Pushed by cku@mozilla.com:
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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/179d25b30000
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
(Assignee)

Updated

a year ago
Duplicate of this bug: 1426880
You need to log in before you can comment on or make changes to this bug.