1648305 - Avoid flip-flopping the resolution and visual viewport size on first paint when shrink-to-fit is needed

Status: RESOLVED FIXED

(Core :: Panning and Zooming, task)

Description

[Kartikaya Gupta (email:[e-mail])]

Consider the scenario where meta-viewport support is enabled. In this scenario, when the load event or first-paint occurs, the MobileViewportManager (in VisualAndMetaViewport mode) runs through the RefreshViewportSize function with mIsFirstPaint = true. This means it runs UpdateResolutionForFirstPaint and then, at the end of the function, runs ShrinkToDisplaySizeIfNeeded. The former function computes a resolution using the viewport size and the latter re-computes it using the scrollable rect size.

This means that the resolution first gets set to 1.0 (and the visual viewport size is updated accordingly) and then it gets re-set to the new "shrink-to-fit" resolution, and again the visual viewport size is updated accordingly. If the resolution was already at the shrink-to-fit resolution before this ran, then this flip-flop of the visual viewport size means a bunch of things get marked dirty for reflow unnecessarily, because in the end the resolution didn't change and nothing needs to be reflowed. In bug 1647034 I'm making it so that the very first reflow (which can happen before the load event/first-paint) does set the shrink-to-fit resolution, and that means this flip-flop produces a spurious reflow.

I think we can get rid of this by avoid the flip-flopping, if we just make sure to use the scrollable rect size for computing the intrinsic scale in UpdateResolutionForFirstPaint. It should be a pretty safe change to do considering that in this scenario the ShrinkToDisplaySizeIfNeeded code runs unconditionally afterwards and clobbers the resolution anyway.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=51712908ba855e4b7aa9e790b9cc0b9eeb163a7f

Comment 2

[Kartikaya Gupta (email:[e-mail])]

I should also point out that the UpdateResolutionForViewportSizeChange function also uses the scrollable rect size, so in that sense UpdateResolutionForFirstPaint is the only one that doesn't. The patch for this bug corrects that inconsistency.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1648305 - Use the scrollable rect size for first-paint intrinsic scale calculation. r?hiro

When meta-viewport support is enabled, the call to UpdateResolutionForFirstPaint
from RefreshViewportSize is followed by a call to ShrinkToDispalySizeIfNeeded,
which calls UpdateResolutionForContentSizeChange and uses the scrollable rect
size for the intrinsic scale computation. So the intrinsic scale computation
in UpdateResolutionForFirstPaint causes a transient state where the resolution
and visual viewport size is wrong. It is corrected immediately after, but
changing the visual viewport size like that ends up marking frames dirty for
reflow. Avoiding the transient state avoids those reflows, which is a nice
optimization.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

... and I forgot to further clarify that the patch only affects the !viewportInfo.IsDefaultZoomValid() codepath which is only the case when meta-viewport handling is turned on. So it shouldn't affect the normal desktop case (the test I was investigating turns on the meta-viewport pref).

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93abfd457d35
Use the scrollable rect size for first-paint intrinsic scale calculation. r=hiro

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/93abfd457d35