Closed Bug 1577258 Opened 3 years ago Closed 3 years ago

Simplify PresShell::ResizeReflowIgnoreOverride.


(Core :: Layout, task)

Not set



Tracking Status
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- fixed


(Reporter: emilio, Assigned: emilio)



(Whiteboard: [geckoview:m1909])


(4 files)

I want this to:

  • Thread all the resizes via that (this includes allowing it to not flush).
  • Making it less scary to touch in general.

This avoids doing wasted work and sending spurious resize
events if this case would be hit.

In practice, it cannot be hit yet, I think, because
callers do check for this and bail out earlier. But
there's no assertion to that respect so this shouldn't

This is much easier than the existing ResizeReflowIgnoreOverride function, and
this will allow me to avoid flushing if needed (we already kinda do that
already with the "suppressResizeReflow" thing), which in turn allows me to
consolidate a bunch of the logic for resizes.

The function should be much easier to follow:

  • Set the new viewport (async, doesn't do any work).
  • Invalidate layout as needed due to the viewport change (that is, resize hint
    for the root frame, and invalidate isizes if needed). Also async and doesn't
    do any reflowing itself.
  • Flush layout / do the reflowing all at once. I think we can stop doing this
    much more often now, but that's follow-up work.

Now that this code path is on its own, we can write more straight-forward code.

Going to try landing these as incrementally as possible since this code is non-trivial.

Keywords: leave-open
Pushed by
early-return for noop resizes. r=botond

We have an optimization to avoid an expensive reflow from SetFullZoom, see

That was done because we used to do a full synchronous reflow right after. We no
longer do that, but due to that member we also don't invalidate!

My second patch in this bug changes the behavior of that flag so that we don't
synchronously reflow, but we do invalidate. So in turn this test before the
change wasn't really testing the zoomed code-path since it was using the clean
layout from before the zoom operation.

a11y getBounds and co. don't flush layout (they probably should), but since with
my patch we dirty the frame tree, and dirty frames return bogus offsets, the
test starts failing.

Flush layout explicitly to ensure we're testing the zoomed code path.

Regressions: 1560378
No longer regressions: 1560378
Assignee: nobody → emilio
Pushed by
Explicitly flush layout in an a11y test. r=eeejay

Adding this bug to GV's September sprint because it blocks Fenix image bug 1551659.

Whiteboard: [geckoview:m1909]
Flags: needinfo?(emilio)

Yeah, sorry, I'll be on PTO until Thursday. I replied, but I'll update the patch following your suggestions before re-requesting review.

Flags: needinfo?(emilio)

Yep, no problem. Thank you for following up!

Pushed by
Add a simpler resize reflow function for when we're not shrink-wrapping. r=bzbarsky
Pushed by
Streamline the shrink-wrapping resize reflow code. r=botond
Keywords: leave-open
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Regressions: 1585778
You need to log in before you can comment on or make changes to this bug.