1658299 - Implement a Promise returning nsIPrinter.createDefaultSettings()

Status: RESOLVED FIXED

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

Description

[Jonathan Watt [:jwatt]]

nsPrinterListWin::InitPrintSettingsFromPrinter makes a blocking call to system APIs, which unfortunately is unnecessarily slowing down the new printing UI since that is called under this frontend code. The frontend code should not need to get these defaults last-used settings for that printer have been saved to prefs and the frontend code will be using those values.

We could improve things by implementing a new Promise returning nsIPrinter.defaultSettings attribute, and get rid of nsIPrintSettingsService.initPrintSettingsFromPrinter altogether.

Comment 1

[Jonathan Watt [:jwatt]]

Although I guess the object should should be mutable, so a creation function would be better.

Comment 2

[Erik Nordin [:nordzilla]]

Attachment: Bug 1658299 - Part 1: Add createDefaultSettings() to nsIPrinter for CUPS r=jwatt,alaskanemily,emilio

Comment 3

[Jonathan Watt [:jwatt]]

This should really be P1. Without this the new UI will create a print preview with zero margins (at least on macOS and Linux), which will in most cases result in cropping of the print output, which would really annoy users.

Comment 4

[Bob Owen (:bobowen)]

Attachment: Bug 1658299 Part 2: Add createDefaultSettings() to nsIPrinter for Windows. r=jwatt!

This retrieves the same settings from the printer as
InitPrintSettingsFromPrinter.
There is probably a lot of scope for de-duplication of code here, but I decided
to try and protect the old fallback paths.

Comment 5

[Bob Owen (:bobowen)]

Try push with my latest Part 2 patch:
https://treeherder.mozilla.org/#/jobs?repo=try&duplicate_jobs=visible&selectedTaskRun=d5cAIqpRSxCcd6bpFthvEg.0&revision=3545f4ca0e355ccf1e28c33fc86000eb005cfdf6

Comment 6

[Erik Nordin [:nordzilla]]

Attachment: Bug 1658299 Part 2: Add createDefaultSettings() to nsIPrinter for Windows. r=jwatt!

This retrieves the same settings from the printer as
InitPrintSettingsFromPrinter.
There is probably a lot of scope for de-duplication of code here, but I decided
to try and protect the old fallback paths.

Phabricator Revision: https://phabricator.services.mozilla.com/D87604

Depends on D87125

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ebf8dfa204ea
Part 1: Add createDefaultSettings() to nsIPrinter for CUPS r=jwatt,emilio,geckoview-reviewers,owlish

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset ebf8dfa204ea (bug 1658299) for test_printer_default_settings.html failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=T8i79jNIQ2CbiaoWuMLD6w.0&fromchange=ebf8dfa204ea618c6e77ec6f06c4c7af786bee9b&searchStr=mochitest&tochange=cda275a5ba19aa62274f9d0264440216be89eaad

Backout link: https://hg.mozilla.org/integration/autoland/rev/cda275a5ba19aa62274f9d0264440216be89eaad

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=313511953&repo=autoland&lineNumber=8674

[task 2020-08-20T02:29:38.563Z] 02:29:38     INFO - TEST-START | layout/base/tests/chrome/test_printer_default_settings.html
[task 2020-08-20T02:29:38.602Z] 02:29:38     INFO - GECKO(1792) | [4280, Main Thread] WARNING: Failed to retarget HTML data delivery to the parser thread.: file /builds/worker/checkouts/gecko/parser/html/nsHtml5StreamParser.cpp, line 1131
[task 2020-08-20T02:29:38.681Z] 02:29:38     INFO - GECKO(1792) | [4280, Main Thread] WARNING: '!devmode', file /builds/worker/checkouts/gecko/widget/windows/nsPrintSettingsWin.cpp, line 183
[task 2020-08-20T02:29:38.681Z] 02:29:38     INFO - GECKO(1792) | console.log: "number"
[task 2020-08-20T02:29:38.700Z] 02:29:38     INFO - GECKO(1792) | [4280, Main Thread] WARNING: '!devmode', file /builds/worker/checkouts/gecko/widget/windows/nsPrintSettingsWin.cpp, line 183
[task 2020-08-20T02:29:38.701Z] 02:29:38     INFO - GECKO(1792) | console.log: "number"
[task 2020-08-20T02:29:38.701Z] 02:29:38     INFO - Buffered messages logged at 02:29:38
[task 2020-08-20T02:29:38.701Z] 02:29:38     INFO - Microsoft XPS Document Writer
[task 2020-08-20T02:29:38.701Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Printer name should be a string. 
[task 2020-08-20T02:29:38.702Z] 02:29:38     INFO - TEST-FAIL | layout/base/tests/chrome/test_printer_default_settings.html | Print settings' printer should match the printer that created them. - got "", expected "Microsoft XPS Document Writer"
[task 2020-08-20T02:29:38.702Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper name should never be null. 
[task 2020-08-20T02:29:38.702Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper width hould never be null. 
[task 2020-08-20T02:29:38.702Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper height hould never be null. 
[task 2020-08-20T02:29:38.703Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.703Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.703Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.703Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-FAIL | layout/base/tests/chrome/test_printer_default_settings.html | Print settings' color mode should match the printer's color support. - got false, expected true
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Print settings were initialized from printer 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | undefined assertion name 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - Fax
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Printer name should be a string. 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-FAIL | layout/base/tests/chrome/test_printer_default_settings.html | Print settings' printer should match the printer that created them. - got "", expected "Fax"
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper name should never be null. 
[task 2020-08-20T02:29:38.704Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper width hould never be null. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper height hould never be null. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Paper margins should be greater than or equal to zero. 
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - Buffered messages finished
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-UNEXPECTED-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Print settings' color mode should match the printer's color support. - false should equal false
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-INFO | expected FAIL
[task 2020-08-20T02:29:38.705Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | Print settings were initialized from printer 
[task 2020-08-20T02:29:38.706Z] 02:29:38     INFO - TEST-PASS | layout/base/tests/chrome/test_printer_default_settings.html | undefined assertion name 
[task 2020-08-20T02:29:38.721Z] 02:29:38     INFO - GECKO(1792) | MEMORY STAT | vsize 913MB | vsizeMaxContiguous 650MB | residentFast 295MB | heapAllocated 117MB

Comment 9

[Erik Nordin [:nordzilla]]

Gah, the guards I put on the test to ensure that those assertions didn't run on Windows only worked for Windows 10 and not Windows 7.

I'm just going to remove those assertions from the test in part 1, since part 2 adds that functionality is working just fine.

Here is a try run that includes all the platforms, including Windows 7:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=919e30b7ef0ce675c3035872c0ed7cd47b3d463e

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35d913aaea4d
Part 1: Add createDefaultSettings() to nsIPrinter for CUPS r=jwatt,emilio,geckoview-reviewers,owlish

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/35d913aaea4d

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/96c093dbd493
Part 2: Add createDefaultSettings() to nsIPrinter for Windows. r=jwatt,jfkthame

Comment 13

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/96c093dbd493