1754308 - Stop sending nsIPrintSettings's printerName and toFileName properties to content process

Status: RESOLVED FIXED

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

Description

[Hsin-Yi Tsai (she/her) [:hsinyi]]

+++ This bug was initially created as a clone of Bug #1749598 +++

According to https://bugzilla.mozilla.org/show_bug.cgi?id=1749598#c3, we want to open a separate bug to tackle the work.

Comment 1

[Mark Striemer [:mstriemer]]

Jonathan, do these bugs belong in Core somewhere? Seems like we can't handle this from the frontend and the references to DeserializeToPrintSettings and the WindowDotPrint flag are all initiated from Core code.

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Mark/:jwatt,
I'm suggesting the Priority as P1 to indicate the impact, importance and urgency for this bug, as it's a blocker of removing jit mitigations. Please feel free to let me know if you have any questions.

Comment 3

[Mark Striemer [:mstriemer]]

Hey Emilio, do you know how we can help with this? I'm not sure of the context or the solution exactly.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I think Jonathan is actively looking into this, but if that's not the case I'm happy to dig into this a bit.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

I think Jonathan is actively looking into this, but if that's not the case I'm happy to dig into this a bit.

Hi Jonathan/Emilio,
If the fix turns out to be high-effort, please feel free to circle back to Tom/me, we want to evaluate the trade-off before diving in.

Comment 6

[Jonathan Watt [:jwatt]]

Quick update:

Regarding printerName, I believe the only remaining GetPrinterName call that can happen in a child process and actually matters is the call in nsDeviceContextSpecWin::Init. (The call in nsDeviceContextSpecGTK::PrinterEnumerator is only invoked in nsDeviceContextSpecGTK::EndDocument, and since nsDeviceContextSpecProxy::EndDocument does not dereference mRealDeviceContextSpec, that one isn't relevant).

Regarding toFileName, I believe the only remaining GetToFileName call that can happen in a child process and actually matter is the call in nsPrintJob::SetupToPrintContent. That's passed through to nsDeviceContext::BeginDocument which passes it to the virtual PrintTarget::BeginPrinting and potentially nsDeviceContextSpecProxy::BeginDocument (again, the latter isn't relevant since it doesn't dereference mRealDeviceContextSpec). The only relevant override of PrintTarget::BeginPrinting is PrintTargetWindows::BeginPrinting.

So I believe there's now only a single instance of each in Windows code that we need to resolve somehow before we can can drop these two lines to resolve this bug:

https://searchfox.org/mozilla-central/rev/8fe6930c0832009b3162bebee7d4ede1a4c8c9a8/widget/nsPrintSettingsService.cpp#150,154

I'll try to catch Bob tomorrow to get his thoughts on the above.

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: Bug 1754308 p1 - Stop sending nsIPrintSettings.toFileName to content processes. r=bobowen

The roundtripping of the toFileName value from RemotePrintJobParent's
mPrintSettings to the content process and then back to
RemotePrintJobParent::InitializePrintDevice was silly anyway.

Comment 8

[Jonathan Watt [:jwatt]]

Attachment: Bug 1754308 p2 - Stop sending nsIPrintSettings.printerName to content processes. r=bobowen

The changes in bug 1770211 mean that we no longer create platform specific
nsIDeviceContextSpec instances in the content process, so we no longer need
the printer name to instantiate an nsDeviceContextSpecWin there.

Depends on D146867

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:alaskanemily, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f1f462836376
p1 - Stop sending nsIPrintSettings.toFileName to content processes. r=bobowen
https://hg.mozilla.org/integration/autoland/rev/dd526a6c446e
p2 - Stop sending nsIPrintSettings.printerName to content processes. r=bobowen

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/f1f462836376
https://hg.mozilla.org/mozilla-central/rev/dd526a6c446e