Fix OverlayPortal layout in shrink-wrapped overlays by aayushkedawat · Pull Request #193079 · flutter/flutter

aayushkedawat · 2026-09-21T04:02:14Z

When an Overlay sizes itself to its content, _RenderLayoutSurrogateProxyBox.performLayout re-derived the constraints for its deferred child instead of using the ones the theater actually uses:

final BoxConstraints theaterConstraints = theater.constraints;
final Size boxSize = theaterConstraints.biggest.isFinite
    ? theaterConstraints.biggest
    : theater.size;

This is wrong in two ways:

In the unbounded case it reads theater.size from a descendant mid-layout, which trips RenderBox.size accessed beyond the scope of resize, layout, or permitted parent access (sizeAccessAllowed). This is the crash in OverlayPortal child throws sizeAccessAllowed when enclosing Overlay has unbounded constraints #192861.
When the overlay has alwaysSizeToContent: true and bounded incoming constraints, it falls into the first branch and uses constraints.biggest — but the theater sized itself to its content, so the overlay child is laid out against constraints the theater never used. There is no crash here, just a silently mis-sized overlay child.

_RenderTheater.performLayout already computes the correct value for its non-positioned children. This PR caches it in _nonPositionedChildConstraints and has the surrogate reuse it, so there is a single source of truth and no ancestor size read.

Theater state	Before	After
Bounded, `alwaysSizeToContent: false`	`constraints.biggest`	unchanged
Unbounded	`theater.size` (illegal read)	same value, no illegal read
Bounded, `alwaysSizeToContent: true`	`constraints.biggest` (wrong)	content size

The field is assigned before any non-size-determining child is laid out. It cannot be read before that assignment: the theater is always an ancestor of its layout surrogates, so a surrogate's first layout is only reachable from _RenderTheater.performLayout, and the only subtree laid out before the assignment is the size-determining one, which the existing _layingOutSizeDeterminingChild guard already skips. I added a comment recording this, since the invariant spans two classes.

This also restores the assert(size == theater.size) invariant in _RenderDeferredLayoutBox, which case 2 above could violate.

Fixes #192861

Tests

Added a regression test to overlay_portal_test.dart, run over both unbounded and alwaysSizeToContent overlays, covering open → resize while open → close → reopen, with verifyTreeIsClean() at each step. Without the source change, the unbounded variant throws the assertion from the issue and the bounded variant sizes the overlay child to 800x600 instead of 100x100.

Local results: all 7331 tests in packages/flutter/test/widgets/ pass, plus dropdown_menu_test.dart, dropdown_test.dart, menu_anchor_test.dart and tooltip_test.dart. The only failures I saw (widget_inspector screenshot, two dropdown goldens) reproduce identically on an unmodified checkout.

Pre-launch Checklist

google-cla · 2026-09-21T04:02:26Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request caches non-positioned child constraints in _RenderTheater to allow layout surrogates to lay out deferred children without reading the theater's size during layout, and adds regression tests for OverlayPortal opening in a shrink-wrapped Overlay. Feedback suggests refactoring the test to initialize overlayEntry before passing it to the Overlay constructor, avoiding a potential LateInitializationError in addTearDown if an exception occurs during setup.

aayushkedawat · 2026-09-24T04:01:08Z

Hi @victorsanni, my merge from master removed the CICD label. Could you re-add it when you get a chance? I'll hold off on further branch updates unless there's a conflict. Thanks!

aayushkedawat · 2026-09-24T06:44:54Z

Hi @victorsanni, thanks for adding CICD. ci.yaml validation failed because my branch was behind master's .ci.yaml (a newly added builder), so I've merged in master, which removed the label again. Could you re-add it? Sorry for the extra round trip.

hannah-hyj

This looks great to me! thank you for the clean fix!

aayushkedawat · 2026-10-05T11:16:19Z

Thanks @hannah-hyj!

I noticed that Google Testing is still failing with an internal test failure that I can't access. Would you be able to take a look when you get a chance?

Happy to make any changes if the failure is related to this PR. Thanks!

auto-submit · 2026-10-05T19:10:09Z

autosubmit label was removed for flutter/flutter/193079, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

LongCatIsLooong · 2026-10-05T19:22:05Z

Please don't merge yet. IIRC we've accounted for size determining child in the deferred child's layout algorithm, I'll take a closer look.

victorsanni · 2026-10-05T19:28:42Z

Please don't merge yet. IIRC we've accounted for size determining child in the deferred child's layout algorithm, I'll take a closer look.

autosubmit label was a mistake on my end, meant to slap on CICD. I also will review as well.

LongCatIsLooong

Sorry for the hold up, I think you're right on both accounts, that size access wasn't necessary and now it's incorrect with the introduction of alwaysSizeToContent. One nit regarding the code comment on the new instance variable, since the size access was never necessary, bringing up "reading the theater's size" could be a little confusing.

victorsanni

LGTM with some testing suggestions.

victorsanni · 2026-10-06T00:35:36Z

Hi can you fix the CI failures as well?

Reuse the theater child constraints when laying out deferred portal children to avoid reading an ancestor size during layout. Cover opening, resizing, and reopening portals with unbounded constraints and alwaysSizeToContent. Fixes flutter#192861

Assigning the entry inside the Overlay constructor required a late variable that addTearDown captured before it was assigned. Build the entry first and pass it in, so the teardown always sees an initialized value.

Reword the comment on _nonPositionedChildConstraints to explain why the value has to be cached: adding a _DeferredLayoutBox does not trigger a _RenderTheater relayout, so _RenderLayoutSurrogateProxyBox.performLayout needs the constraints from the last theater layout. The previous wording framed this around avoiding a size read, which was never necessary in the first place. Rename the local in computeDryBaseline so the dry layout value does not read as the new field. Extend the regression test with two more axes: the OverlayPortal.overlayChildLayoutBuilder variant, and a tightly constrained anchor that makes the layout surrogate its own relayout boundary instead of a shallower ancestor. All eight combinations fail without the fix.

aayushkedawat · 2026-10-10T14:50:06Z

@victorsanni - the CICD label came off with the branch update on the 8th, so the gated shards haven't run since.

Could you re-add CICD and autosubmit? Everything else is passing (7/7), and both your and @LongCatIsLooong 's approvals are still in place.

Thanks for keeping the branch up to date.

…#13220) Manual roll Flutter from d03768eef055 to 4249ccbc18a3 (158 revisions) Manual roll requested by [email protected] flutter/flutter@d03768e...4249ccb 2026-10-12 [email protected] [AGP 9.1.0 Migration #8] Unify add-to-app module wiring on the variant API (flutter/flutter#193717) 2026-10-12 [email protected] [master] Update Flutter DEPS to Dart 5933cb486b4aef2f8cf6b9f89a5651c0d5b0955e (flutter/flutter#193982) 2026-10-12 [email protected] [Android] Fix use-after-free of SurfaceControl.Transaction in HCPP (flutter/flutter#192969) 2026-10-12 [email protected] [windows] Add OpenGL compositor tests against ANGLE at D3D11 feature level 10_0 (flutter/flutter#194114) 2026-10-12 [email protected] Fix HDR image packing for ios (flutter/flutter#192526) 2026-10-12 [email protected] Bump customer tests to pick up material_ui and cupertino_ui (flutter/flutter#194236) 2026-10-12 [email protected] Remove no-shuffle from text_field_test.dart (flutter/flutter#193550) 2026-10-12 [email protected] [web] Clip semantics DOM text labels to stop stealing taps (flutter/flutter#194011) 2026-10-12 [email protected] Roll Skia from 9b6ceba4728d to 17f6986c226c (6 revisions) (flutter/flutter#194232) 2026-10-12 [email protected] [web] Skip non-focusable containers in Focusable.focusAsRouteDefault (flutter/flutter#194138) 2026-10-12 [email protected] [web_paragraph] Honor kTextHeightNone in TextStyle and ParagraphStyle (flutter/flutter#193733) 2026-10-12 [email protected] Roll Skia from c88b49d1d6c7 to 9b6ceba4728d (4 revisions) (flutter/flutter#194226) 2026-10-12 [email protected] Migrate remaining test to macOS 26 (flutter/flutter#193825) 2026-10-12 [email protected] Roll Dart SDK from 047ca322aa9c to 221363557163 (1 revision) (flutter/flutter#194219) 2026-10-12 [email protected] [flutter_tools] Include dev_dependencies hooks for release integration tests (flutter/flutter#193436) 2026-10-12 [email protected] Roll Skia from d8afad1b1829 to c88b49d1d6c7 (1 revision) (flutter/flutter#194198) 2026-10-12 [email protected] Roll Skia from b23ddbff1265 to d8afad1b1829 (4 revisions) (flutter/flutter#194195) 2026-10-11 [email protected] Roll Skia from a168863e44ec to b23ddbff1265 (1 revision) (flutter/flutter#194185) 2026-10-11 [email protected] Roll Skia from 0c1d93abfa04 to a168863e44ec (1 revision) (flutter/flutter#194181) 2026-10-11 [email protected] iOS: Re-enable rendering on willEnterForeground (flutter/flutter#191350) 2026-10-11 [email protected] Roll Skia from 93c9d3ffda25 to 0c1d93abfa04 (1 revision) (flutter/flutter#194179) 2026-10-11 [email protected] Remove more instances of deprecated `withOpacity` (flutter/flutter#193559) 2026-10-11 [email protected] Fix another `parameter_assignments` warning (flutter/flutter#193669) 2026-10-11 [email protected] Roll Skia from 3136abef8f58 to 93c9d3ffda25 (1 revision) (flutter/flutter#194169) 2026-10-11 [email protected] [Impeller] Guard the sampler library caches with a mutex (flutter/flutter#193738) 2026-10-11 [email protected] Roll Dart SDK from e9c89888bcc1 to 047ca322aa9c (1 revision) (flutter/flutter#194157) 2026-10-10 [email protected] Roll Dart SDK from 787baa8aae78 to e9c89888bcc1 (2 revisions) (flutter/flutter#194141) 2026-10-10 [email protected] Fix OverlayPortal layout in shrink-wrapped overlays (flutter/flutter#193079) 2026-10-10 [email protected] Roll Skia from 353076e27b1c to 3136abef8f58 (1 revision) (flutter/flutter#194130) 2026-10-10 [email protected] Roll Dart SDK from 88a07589b1ff to 787baa8aae78 (1 revision) (flutter/flutter#194124) 2026-10-10 [email protected] Route windowing view lookups through WidgetsBinding (flutter/flutter#192666) 2026-10-10 [email protected] Roll Dart SDK from 2a06cab34715 to 88a07589b1ff (1 revision) (flutter/flutter#194117) 2026-10-10 [email protected] Roll Skia from f9043b7aa798 to 353076e27b1c (2 revisions) (flutter/flutter#194115) 2026-10-10 [email protected] Refactored PipelineLibraryGLES (flutter/flutter#193985) 2026-10-09 [email protected] add shorthands for scroll physics subclasses in framework (flutter/flutter#193082) 2026-10-09 [email protected] Roll Dart SDK from ba889e3cee8a to 2a06cab34715 (1 revision) (flutter/flutter#194105) 2026-10-09 [email protected] Roll Skia from 5ea03d8a53a6 to f9043b7aa798 (2 revisions) (flutter/flutter#194099) 2026-10-09 [email protected] Add docs for using bb and led with LUCI (flutter/flutter#193859) 2026-10-09 [email protected] Roll Skia from e329bb617820 to 5ea03d8a53a6 (5 revisions) (flutter/flutter#194087) 2026-10-09 [email protected] Roll Packages from 54706bd to d8d9e97 (5 revisions) (flutter/flutter#194080) 2026-10-09 [email protected] Move the Vulkan wrapped-texture helper out of the GPU surface (flutter/flutter#193001) 2026-10-09 [email protected] Roll Skia from 3e54bcf4c963 to e329bb617820 (1 revision) (flutter/flutter#194078) 2026-10-09 [email protected] Roll Dart SDK from 23f0308f9af8 to ba889e3cee8a (2 revisions) (flutter/flutter#194077) 2026-10-09 [email protected] Bump dartdoc to 9.0.11 (flutter/flutter#194076) ...

When an `Overlay` sizes itself to its content, `_RenderLayoutSurrogateProxyBox.performLayout` re-derived the constraints for its deferred child instead of using the ones the theater actually uses: ```dart final BoxConstraints theaterConstraints = theater.constraints; final Size boxSize = theaterConstraints.biggest.isFinite ? theaterConstraints.biggest : theater.size; ``` This is wrong in two ways: 1. In the unbounded case it reads `theater.size` from a descendant mid-layout, which trips `RenderBox.size accessed beyond the scope of resize, layout, or permitted parent access` (`sizeAccessAllowed`). This is the crash in flutter#192861. 2. When the overlay has `alwaysSizeToContent: true` *and* bounded incoming constraints, it falls into the first branch and uses `constraints.biggest` — but the theater sized itself to its content, so the overlay child is laid out against constraints the theater never used. There is no crash here, just a silently mis-sized overlay child. `_RenderTheater.performLayout` already computes the correct value for its non-positioned children. This PR caches it in `_nonPositionedChildConstraints` and has the surrogate reuse it, so there is a single source of truth and no ancestor size read. | Theater state | Before | After | | --- | --- | --- | | Bounded, `alwaysSizeToContent: false` | `constraints.biggest` | unchanged | | Unbounded | `theater.size` (illegal read) | same value, no illegal read | | Bounded, `alwaysSizeToContent: true` | `constraints.biggest` (wrong) | content size | The field is assigned before any non-size-determining child is laid out. It cannot be read before that assignment: the theater is always an ancestor of its layout surrogates, so a surrogate's first layout is only reachable from `_RenderTheater.performLayout`, and the only subtree laid out before the assignment is the size-determining one, which the existing `_layingOutSizeDeterminingChild` guard already skips. I added a comment recording this, since the invariant spans two classes. This also restores the `assert(size == theater.size)` invariant in `_RenderDeferredLayoutBox`, which case 2 above could violate. Fixes flutter#192861 ## Tests Added a regression test to `overlay_portal_test.dart`, run over both unbounded and `alwaysSizeToContent` overlays, covering open → resize while open → close → reopen, with `verifyTreeIsClean()` at each step. Without the source change, the unbounded variant throws the assertion from the issue and the bounded variant sizes the overlay child to 800x600 instead of 100x100. Local results: all 7331 tests in `packages/flutter/test/widgets/` pass, plus `dropdown_menu_test.dart`, `dropdown_test.dart`, `menu_anchor_test.dart` and `tooltip_test.dart`. The only failures I saw (`widget_inspector` screenshot, two dropdown goldens) reproduce identically on an unmodified checkout. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Victor Sanni <[email protected]>

github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 21, 2026

gemini-code-assist Bot reviewed Sep 21, 2026

View reviewed changes

Comment thread packages/flutter/test/widgets/overlay_portal_test.dart Resolved

aayushkedawat force-pushed the fix/overlay-portal-unbounded-constraints branch from 7507243 to e52fbd1 Compare September 21, 2026 04:15

victorsanni added the CICD Run CI/CD label Sep 21, 2026

victorsanni self-requested a review September 21, 2026 20:57

flutter-dashboard Bot removed the CICD Run CI/CD label Sep 22, 2026

victorsanni added the CICD Run CI/CD label Sep 24, 2026

flutter-dashboard Bot removed the CICD Run CI/CD label Sep 24, 2026

victorsanni added the CICD Run CI/CD label Sep 24, 2026

victorsanni requested a review from hannah-hyj September 30, 2026 19:19

hannah-hyj previously approved these changes Oct 5, 2026

View reviewed changes

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 5, 2026

victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 5, 2026

victorsanni requested a review from LongCatIsLooong October 5, 2026 19:10

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 5, 2026

victorsanni added the CICD Run CI/CD label Oct 5, 2026

LongCatIsLooong previously approved these changes Oct 5, 2026

View reviewed changes

Comment thread packages/flutter/lib/src/widgets/overlay.dart Resolved

victorsanni previously approved these changes Oct 5, 2026

View reviewed changes

Comment thread packages/flutter/test/widgets/overlay_portal_test.dart Resolved

Comment thread packages/flutter/test/widgets/overlay_portal_test.dart Resolved

aayushkedawat added 3 commits October 6, 2026 16:38

Initialize overlayEntry before constructing the Overlay in the new test

b499466

Assigning the entry inside the Overlay constructor required a late variable that addTearDown captured before it was assigned. Build the entry first and pass it in, so the teardown always sees an initialized value.

aayushkedawat dismissed victorsanni’s stale review via ad685a1 October 6, 2026 11:11

Merge branch 'master' into fix/overlay-portal-unbounded-constraints

983c25f

flutter-dashboard Bot removed the CICD Run CI/CD label Oct 8, 2026

victorsanni added autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD labels Oct 10, 2026

auto-submit Bot added this pull request to the merge queue Oct 10, 2026

Merged via the queue into flutter:master with commit a3ee5e3 Oct 10, 2026
27 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 10, 2026

Conversation

aayushkedawat commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests

Pre-launch Checklist

Uh oh!

google-cla Bot commented Sep 21, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

aayushkedawat commented Sep 24, 2026

Uh oh!

aayushkedawat commented Sep 24, 2026

Uh oh!

hannah-hyj left a comment

Choose a reason for hiding this comment

Uh oh!

aayushkedawat commented Oct 5, 2026

Uh oh!

auto-submit Bot commented Oct 5, 2026

Uh oh!

LongCatIsLooong commented Oct 5, 2026

Uh oh!

victorsanni commented Oct 5, 2026

Uh oh!

LongCatIsLooong left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

victorsanni commented Oct 6, 2026

Uh oh!

aayushkedawat commented Oct 10, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aayushkedawat commented Sep 21, 2026 •

edited

Loading