Platform code changes to prepare to validate prefs against printer to improve performance
Categories
(Core :: Printing: Setup, enhancement, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox96 | --- | fixed |
People
(Reporter: bobowen, Assigned: bobowen)
References
(Blocks 2 open bugs)
Details
(Keywords: perf, Whiteboard: [print2020_v90][old-ui-])
Attachments
(7 files, 2 obsolete files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
This is a sketch of an idea about how the PrintPreview API could be changed to improve performance and check settings against the printer.
It’s written as a change to the nsFrameLoader API, but could be done in frontend code, although some of the steps might need new platform APIs.
I don't know how the "Apply settings to printer" step works for CUPS printers or even if it is possible.
As it would now return print settings as well, it would now need to return a Promise that resolves to an object that contains the print settings and the current promise for the PrintPreviewSuccessInfo.
Steps involving printer interaction would need to be run async on background threads.
PrintPreview(nsIPrintSettings settings)->Promise->{nsIPrintSettings, Promise}:
1. settings passed in
2. call content API to create/update preview with settings
3. apply settings to printer and retrieve updated settings (like at [1])
4. compare settings from printer to settings from prefs
5. should normally be no difference so return Promise with settings and Promise
Alternatives
1a. no settings passed (this would be the first call)
1a1. Get settings from prefs
1a2. return to 2.
1b. no settings passed in or saved in prefs
1b1. get default settings from printer
1b2. call content API to create preview with settings
1b3. return Promise with settings and Promise
5a. settings from printer differ to settings from prefs or passed in
5a1. call content API to update preview with settings
5a2. return Promise with settings and Promise
Updated•4 years ago
|
Comment 1•4 years ago
|
||
(In reply to Bob Owen (:bobowen) from comment #0)
I don't know how the "Apply settings to printer" step works for CUPS printers or even if it is possible.
I don't believe it is possible. I expect we need to collect the capabilities of the printer and validate each setting against those manually. But whether the DEVMODE is used to validate/fix settings on Windows or settings are fixed up by manual comparison, that seems like an implementation detail we should abstract over.
Comment 2•4 years ago
|
||
One situation we need to cater for that wouldn't be provided for with this API is when the user has the "Save to PDF" printer selected, but then clicks on "Print using system dialog...". In that case we want to copy as many settings as possible (validating them) onto a new settings object that has its printer name set to the name of a system recognized printer (the system dialog doesn't know what to do with the "Save to PDF" printer). (We do not want to copy the printer name, obiously.) For that, it seem like we probably want to add something like the following to the nsIPrintSettings
interface:
// Copies all settings that are valid for this settings object's printer.
// Does not copy the printer name.
Promise copyFromWithValidation(in nsIPrintSettings);
Since we need that API (I think), it seems like it could also be used in the more general case that comment 0 aims to address.
So in the case of the user having never printed to the printer before (no settings saved to prefs) the frontend code would get an nsIPrinter and just pass the settings returned by createDefaultSettings
to FrameLoader.print/.printPreview().
In the case the user has used the printer before, the frontend code would get a new settings object, initialize it with initPrintSettingsFromPrefs, immediately dispatch it to FrameLoader.print/.printPreview(), but for now keeping the UI disabled. It would also create a second print settings object and call copyFromWithValidation() on it, passing the the initialized-from-prefs settings object. Once the returend Promise resolved, it would compare the two settings objects for differences (we should add API for that too). If there are no differences, it would simply remove the "creating preview" spinner and enable the UI. In the unlikely event there were differences, it would dispatch a second FrameLoader.print/.printPreview() call with the validated preferences and wait for that Promise to resolve before enabling the UI.
In the rare case where we have to dispatch fixed up settings, essentially we'd behave in the same way we do now, and have the same performance characteristics. In the common case where there are no differences and we don't need to regenerate the print doc with new settings then the content process would have been laying out the doc in parallel to the parent process validating settings. That would make things much faster for users for whom getting printer capabilities is very slow.
This approach would also have the advantage on not complicating the printPreview() API.
Thoughts?
Comment 3•4 years ago
|
||
As Bob just pointed out to me, the API wouldn't be on nsIPrintSettings. It would be on nsIPrinter, and be more like:
Promise copySettingsWithValidaton(fromSettings);
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
This is to allow settings saved in prefs to be validated against a printer and
for print preview to be updated if any changes affecting layout occur.
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 5•4 years ago
|
||
Depends on D98458
Assignee | ||
Comment 6•4 years ago
|
||
This means that more precise values can be stored, so that they match what we
actually retrieve from the printer and use in layout.
Depends on D99806
Assignee | ||
Comment 7•4 years ago
|
||
This allows us to detect if any prefs were read in JavaScript.
Depends on D99807
Assignee | ||
Comment 8•4 years ago
|
||
This more precisely matches what we get from the printer and use in layout.
Depends on D99808
Assignee | ||
Comment 9•4 years ago
|
||
This is always available and makes it more consistent for comparing print
settings.
Also change to round when converting back to DEVMODE for better accuracy.
Depends on D99810
Assignee | ||
Comment 10•4 years ago
|
||
The settings are then validated against the printer and if changes affecting layout occur a new preview is generated.
Depends on D99811
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
Depends on D98458
Updated•4 years ago
|
Assignee | ||
Comment 12•4 years ago
|
||
Finally got a green try push, one with just browser chrome:
https://treeherder.mozilla.org/jobs?repo=try&duplicate_jobs=visible&revision=42720467ea7ee0c5ae3a4bd673f7919991d8b5d8
Previous auto one that had browser chrome print test failures:
https://treeherder.mozilla.org/jobs?repo=try&duplicate_jobs=visible&revision=d243bd3033adfdc8e325ab0d3988d017dfebe00b
Updated•4 years ago
|
Comment 13•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 14•3 years ago
|
||
Comment 15•3 years ago
•
|
||
I've landed all the platform work as parts 1-7 (previously 1-6, but I renumbered part 1.5 before landing) and will split out the final frontend patch into a separate bug, and wait at least a Nightly release before landing it. That should prevent the platform patches from bitrotting again in the event that there are issues getting the frontend patch reviewed, and help with any potential regression tracking.
Comment 16•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2f7c6dff65fc
https://hg.mozilla.org/mozilla-central/rev/6e70617cbb79
https://hg.mozilla.org/mozilla-central/rev/1fb0f7dc84a9
https://hg.mozilla.org/mozilla-central/rev/a969b3be9e10
https://hg.mozilla.org/mozilla-central/rev/7cc7109100a6
https://hg.mozilla.org/mozilla-central/rev/9cd220f0da60
https://hg.mozilla.org/mozilla-central/rev/2f40a908044e
Updated•3 years ago
|
Updated•3 years ago
|
Description
•