When print preview settings are changed, the frontend calls nsIWebBrowserPrint.printPreview with a new nsIPrintSettings object. Unfortunately that is the same API that is used to initiate the print preview and involves re-cloning the print preview document.

We have bug 1636728 open to stop doing the re-clone, but in preparation for fixing that it would be great if the frontend code could start using new API that has the semantics of just-apply-these-updated-settings. Initially the implementation would call the current code paths, resulting in a re-clone, but it would allow us to fix bug 1636728 once we get do it, allow us to easily test if the new implementation is working, and once landed, have the print preview functionality immediately switch over to the no-clone implementation without needing to wait for the frontend to make changes.

Ah, I had this in my WIP for bug 1636728... I called it void updatePrintPreviewSettings(in nsIPrintSettings aSettings);, but :-)

Cool, put the patch up here and I'll review it. ;-)

(I don't have a strong opinion on the name, although if it takes an nsIPrintSettings as an argument then maybe 'Settings' in the name is a bit redundant? But whatever...)

Actually, this should be on the FrameLoader interface (alongside the print() function), instead of on nsIWebBrowserPrint, so that the frontend code can invoke it from the parent process.

We need this so that the frontend can avoid saving to prefs in order to pass settings to the platform code. That makes it hazardous to have the new tab-mobal print UI open in multiple tabs.

Basically done other than the DoCommonPrint logic needing tweaked.

https://hg.mozilla.org/mozilla-central/rev/3103a7279ab9

Comment on attachment 9172831 [details]
Bug 1653340. Add new FrameLoader.printPreview() API. r=emilio,bobowen

Beta/Release Uplift Approval Request

• User impact if declined: Requesting uplift on jwatt's behalf as this is time-sensitive and this one seems to have been overlooked.
• 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):
• String changes made/needed: None

Comment on attachment 9172831 [details]
Bug 1653340. Add new FrameLoader.printPreview() API. r=emilio,bobowen

Approved for 81.0b6.

https://hg.mozilla.org/releases/mozilla-beta/rev/cae2da3677a9

I should probably have called out this comment in the phabricator revision here in the bug comments too. I said:

This approach makes things much more robust than our current nsIWebProgressListener
setup BTW, where if an error occurs along that codepath (there are tons of potential
return points) then the caller in the parent process is never notified of failure (the
spinner showing the user we're waiting for the PP doc to load will spin for ever).

So if we at some point discover that the printing error rate is higher in the new printing UI, it may [in part] be due to the error catching approach (and thus error reporting) being more robust.