commit f6529c7990744370869e4ab2794caae6c46ba044
author Morten Stenshorne <mstensho@chromium.org> Thu Jun 22 06:53:00 2023
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jun 22 06:53:00 2023
tree 716413cc35ee364bf55bd43d2508573d4a79c678
parent 286d6c89c8507267a45170035b951d83af0619d6

Printing: Avoid points<->pixels roundtrips, untangle magical scaling.

Use CSS pixels where we can, most prominently, in WebPrintParams.
Convert directly from device pixels to CSS pixels. We used to convert
from device pixels to points, then pass this to Blink, and Blink would
convert it to CSS pixels. The fact that this conversion was
taking place wasn't exactly obvious. The values were multiplied by
PrintContext::kPrintingMinimumShrinkFactor, which was documented as a
somewhat mysterious factor, that, according to the documentation, used
to be 1.25, but more recent research had shown that 1.33333 was a better
factor. But there's really nothing magical or heuristic about this.
There are 96 CSS pixels per inch, and there are 72 points per inch.
96/72 = 4/3 ~= 1.33333

Don't expose internal Blink scaling (remove WebLocalFrame::
GetPrintPageShrink()). Blink sometimes scales down content to fit more
content in the inline direction (as an attempt to avoid clipping and
thus losing content). Take care of that internally, which is more
consistent. Blink used to do it internally for web tests
(SpoolPagesWithBoundariesForTesting() in WebLocalFrameImpl), whereas for
printing in actual Chrome, it was done on the outside, by
PrintRenderFrameHelper::PrintPageInternal().

This also improves media query evaluation "for free". We used to perform
some strange conversions in PrepareFrameAndViewForPrint::
ResizeForPrinting(). The input size used to be in points, and then we'd
*divide* the height (but not the width!) by 1.333, and then pretend that
the result was in pixels. Now that "everything" is in pixels, things
will improve automatically, unless we make an effort to prevent it. It's
still not quite right, and this CL isn't about that anyway (added TODO).

Also update the header/footer template. PrintHeaderAndFooter() used to
cancel out scaling, including the internal 1/1.333 (3/4) scaling in
Blink, so that content in headers and footers would appear 1.333 (4/3)
times as large as it should be. This has now been untangled and cleaned
up, so we need to ask for the sizes we actually want to see, rather than
3/4 of that. This includes the options passed to
setupHeaderFooterTemplate(). setupHeaderFooterTemplate() specifies the
page size in pixels, although it was actually passed in points. But that
looked just fine, since everything was accidentally (?) zoomed in by
4/3.

There was some confusion on the web tests side of things.
PrintFrameToBitmap() thinks that it takes a page size in pixels, but
this value was multiplied with 1.333 inside Blink afterwards, so it was
effectively points, not pixels. We need to rebaseline or rewrite quite a
few of the tests before we can remove it, so, for now, keep the magical
1.333 scaling thing for tests. Will follow up with a CL that removes
this and rebaselines / rewrites the tests that need it.

Four tests still need to be rebaselined in this CL, though. The
down-scaling of content inside Blink produces slightly different results
now. Since we're using pixels instead of points, and they are passed as
integer values, we end up with a slightly different aspect ratio, so
that LocalFrame::ResizePageRectsKeepingRatio() doesn't behave exactly
like before. Since this CL fixes some rounding errors, two layout tree
dump tests also need to be rebaselined.

Added a few browser tests, both for crbug.com/1444579 (which was the
main motivation for making this change), and also for crbug.com/1117050
since media query evaluation has improved, because we're now using CSS
pixels.

Two of the new tests are disabled. Since we don't support fractional
page sizes, and round down everything to the nearest integer (e.g.
123.9px -> 123px), content that the author would expect to fit on one
page will fragment. Will deal with this in a follow-up.

Bug: 1444579, 1117050
Change-Id: I076cc9f84591b74925465e364ef2f5f4908bb794
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4567866
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1161059}