1601694 - Remove the redundant Windows range/ifame selection code

Status: RESOLVED FIXED

(Core :: Printing: Setup, task, P3)

Description

[Jonathan Watt [:jwatt]]

The Windows file nsPrintDialogUtil.cpp has some code that checks the nsIWebBrowserPrint (a proxy object with e10s enabled) for range/iframe selection and sets the kEnableSelectionRB bit on the nsIPrintSettings if so:

https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/widget/windows/nsPrintDialogUtil.cpp#363-377

But nsPrintJob already sets these flags on the nsIPrintSettings before calling ShowPrintDialog:

https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/layout/printing/nsPrintJob.cpp#890-892,941

When e10s is enabled the nsIWebBrowserPrint is actually a MockWebBrowserPrint instance, but the kEnableSelectionRB bit is still propagated/deserialized correctly:

https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/widget/nsPrintSettingsService.cpp#165

So this code in nsPrintDialogUtil.cpp is redundant.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: Bug 1601694. Remove the redundant Windows range/ifame selection code. r=bobowen

There is no need for this code to set the kEnableSelectionRB bit. nsPrintJob
already sets it before it calls nsIPrintingPromptService::ShowPrintDialog.

Comment 2

[Jonathan Watt [:jwatt]]

It's worth mentioning that the macOS and Linux code works without this extra bit setting that the windows code has precisely because it can rely on nsPrintJob setting it.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a6a9814936d
Remove the redundant Windows range/ifame selection code. r=bobowen

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: Bug 1601694 pt2. Remove redundant Windows range/ifame selection code. r=bobowen

This code should have been removed in the original part, landed as:
https://hg.mozilla.org/mozilla-central/rev/3a6a9814936d

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c125d40bb4d8
pt2. Remove redundant Windows range/ifame selection code. r=bobowen

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3a6a9814936d
https://hg.mozilla.org/mozilla-central/rev/c125d40bb4d8

Comment 7

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Does the improvement below make sense for this?
== Change summary for alert #24302 (as of Fri, 06 Dec 2019 21:00:15 GMT) ==

Improvements:

57% raptor-tp6m-cnn-geckoview-cold loadtime android-hw-g5-7-0-arm7-api-16 pgo 14,311.17 -> 6,125.67
26% raptor-tp6m-cnn-geckoview-cold android-hw-g5-7-0-arm7-api-16 pgo 4,560.85 -> 3,357.70
18% raptor-tp6m-cnn-geckoview-cold fcp android-hw-g5-7-0-arm7-api-16 pgo 7,610.58 -> 6,224.92

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=24302