1660166 - Clean up determination of page numbers

Status: RESOLVED FIXED

(Core :: Printing: Output, task)

Description

[Daniel Holbert [:dholbert]]

We use a somewhat clumsy mechanism for computing & handling page numbers and the page-count.

In support of bug 1631452 and bug 1659005, let's clean that up a bit.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: Bug 1660166 part 1: In PrintedSheetFrame's reflow loop, cast the child frame to nsPageFrame (which is the concrete type that we know it has). r?hiro

We already do this static_cast'ing before this patch, for a single
nsPageFrame-specific method call. This patch just captures the result of that
static_cast in a reusable variable, so that I can easily add another
nsPageFrame-specific function call in the next patch of this patch series.

(We can be confident this casting is valid, because we only ever give
PrintedSheetFrame this one type of child frame.)

Comment 2

[Daniel Holbert [:dholbert]]

(Two more patches coming, in phabricator processing limbo right now)

These patches don't represent any functional changes, given the current state of our tree. They should not impact behavior at this point.

They are meant to simplify code & prevent bugs that would crop up once we have more than one nsPageFrame on a given PrintedSheetFrame, which I intend to support in bug 1631452 and bug 1659005. Once we have multiple nsPageFrames per PrintedSheetFrame, we cannot simply count the pages by counting the number of nsPageSequenceFrame child-frames anymore, since strictly speaking that gives us the number of sheets.

In particular, they prevent us from reporting e.g. "Page 7 of 3" on page footers (where "3" is mistakenly a sheet count rather than a page count).

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1660166 part 2: Let nsPageFrame determine its own page number, and store total number of pages in one central spot. r?hiro

Depends on D87695

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: Bug 1660166 part 3: Remove a redundant member-variable that represents the total number of pages. r?hiro

Before this patch stack, nsPageSequenceFrame and every nsPageFrame each
individually tracked the number of pages.

As of the previous patch, we've coalesced the nsPageFrame variables into a
single variable that lives on nsSharedPageData. This patch here replaces
nsPageSequenceFrame's member-var with that nsSharedPageData variable, too.

Depends on D87696

Comment 5

[Hiroyuki Ikezoe (:hiro)]

I am going to land these patches once after I've confirmed it's ok on a try run.

Comment 6

[Daniel Holbert [:dholbert]]

Thank you!!

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Okay, the result looks good. https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bcb792a68bcdb53a7779140ace670801c69aa20

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5fe1dd9f3bad
part 1: In PrintedSheetFrame's reflow loop, cast the child frame to nsPageFrame (which is the concrete type that we know it has). r=hiro
https://hg.mozilla.org/integration/autoland/rev/d64d22e30483
part 2: Let nsPageFrame determine its own page number, and store total number of pages in one central spot. r=hiro
https://hg.mozilla.org/integration/autoland/rev/c3fe601df6e1
part 3: Remove a redundant member-variable that represents the total number of pages. r=hiro

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: Bug 1660166 followup: Mark nsPageFrame::GetPageNum as 'const'. r?hiro

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00693a176876
followup: Mark nsPageFrame::GetPageNum as 'const'. r=TYLin

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5fe1dd9f3bad
https://hg.mozilla.org/mozilla-central/rev/d64d22e30483
https://hg.mozilla.org/mozilla-central/rev/c3fe601df6e1

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/00693a176876