1686494 - Minutes of delay, when printing a page-range from a long PDF document

Status: VERIFIED FIXED

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

Description

[Emil Ghitta, QA [:emilghitta]]

Attachment: LongWaitTime.gif

Affected versions

• Firefox 86.0a1 (BuildId:20210113100240)
• Firefox 85.0b8 (BuildId:20210112185508)

Affected platforms

• Windows 10 64bit
• Ubuntu 20.04 64bit
• macOS 10.14

Steps to reproduce

1. Launch Firefox.
2. Open the following webpage
3. Hit Ctrl + P
4. Select from the page range to print the first page.
5. Click print.

Expected result

• The print job is successfully started.

Actual result

• The status remains blocked in “printing” but no actual print job is initiated.

Regression Range

• This seems to be a regression:
• Pushlog : https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4489f33bf6521cdc4ecc915063d8733f5daa0814&tochange=f831fe50e3baa7ec892cb7112ccff5b4cb30ae53
• Potential Regressor: Bug 1661868

Notes

• For further information regarding this issue please observe the attached screencast.
• Tentatively tracking this for [print2020_v86]

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Hi Daniel!

It seems that mozregression generated a pushlog containing both Bug 1669774 & Bug 1661868. One of them may have caused this regression.

Can you please take a look?

Thank you!

Comment 2

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Thank you! It does look related to the second of those bugs; I'll take a look.

From my local testing (using Microsoft print-to-pdf as my printer), it looks like we're busy for a few minutes before actually printing.

During that busy-time, we're spinning our wheels in PDF.js rendering code, probably rendering the whole document. I wonder if the old codepath (the one that I removed in bug 1661868) had some fast-path that bypassed rendering for the pages that we don't need...

Comment 3

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Profile from a Nightly before the regressing patch landed:
https://share.firefox.dev/2LodbN8

Profile from a Nightly after the regressing patch landed (captured after 1 minute had passed and the print operation was still going):
https://share.firefox.dev/39npDEw

Comment 4

[Daniel Holbert [:dholbert] (PTO until 2/24)]

In the first profile, it looks like we call the PDF.js function _next only once, whereas in the second profile, we call it hundreds of times. That function must be the "paint the next page" function, or something like that, and the old codepath must've had something to control how many times it was called...

Comment 5

[Daniel Holbert [:dholbert] (PTO until 2/24)]

bdahl pointed me to GetPrintCanvasElementsInFrame - we're likely mishandling something around that function now:
https://searchfox.org/mozilla-central/rev/2a24205479519e70c0574929f45730d285141584/layout/generic/nsPageSequenceFrame.cpp#471

Comment 6

[Daniel Holbert [:dholbert] (PTO until 2/24)]

(In particular, that function probably needs an adjustment to skip nsPageFrame descendants that are tagged with NS_PAGE_SKIPPED_BY_CUSTOM_RANGE.)

Comment 7

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Attachment: Bug 1686494 part 1: Change return type of nsPageSequence::GetCurrentSheetFrame to the actual concrete type, PrintedSheetFrame*, instead of nsIFrame*. r?TYLin

This patch doesn't change behavior; it just adds a static_cast to a
more-specific type that we know the variable has, and updates the signature and
callsites.

(The next patch will make use of the fact that we know this type.)

Comment 8

[Daniel Holbert [:dholbert] (PTO until 2/24)]

Attachment: Bug 1686494 part 2: When gathering printed canvas elements for PDF.js, skip canvases on pages that aren't in the user's custom print range. r?TYLin

This code currently does a recursive tree-traversal to get a list of all canvas
elements that might be used for PDF.js content, so that we can invoke the
appropriate callbacks on them to let PDF.js prepare for printing.

In this patch, I'm adjusting the outermost level of that traversal to make it
aware of the fact that PrintedSheetFrames may now contain many skipped
nsPageFrames whose content we don't want to render.

Depends on D101941

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a87bda8920b9
part 1: Change return type of nsPageSequence::GetCurrentSheetFrame to the actual concrete type, PrintedSheetFrame*, instead of nsIFrame*. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/946ca4dbc01e
part 2: When gathering printed canvas elements for PDF.js, skip canvases on pages that aren't in the user's custom print range. r=TYLin

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/a87bda8920b9
https://hg.mozilla.org/mozilla-central/rev/946ca4dbc01e

Comment 12

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello,

Confirming this issue as verified fixed using 86.0b8(20210209185733) and 87.0a1(20210209214921) on a Windows 10x64, Ubuntu 20 and macOS 10.15.7.