Skip to content

Commit dc8377b

Browse files
Ensure OverlayPortal.overlayChild's renderObject is reachable via treewalk (flutter#134497)
Fixes flutter#133545 ` child._layoutSurrogate.markNeedsLayout();` was called when `_skipMarkNeedsLayout` is set true so when there's no relayout boundary between the layout surrogate and the RenderTheater, no dirty render objects will be added to the PipelineOwner's dirty list. It's ok to mark the RenderTheater dirty when there's no layout boundary between it and the layout surrogate.
1 parent fcba7b3 commit dc8377b

File tree

3 files changed

+101
-17
lines changed

3 files changed

+101
-17
lines changed

packages/flutter/lib/src/rendering/object.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1960,7 +1960,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
19601960
final bool mutationsToDirtySubtreesAllowed = activeLayoutRoot.owner?._debugAllowMutationsToDirtySubtrees ?? false;
19611961
final bool doingLayoutWithCallback = activeLayoutRoot._doingThisLayoutWithCallback;
19621962
// Mutations on this subtree is allowed when:
1963-
// - the subtree is being mutated in a layout callback.
1963+
// - the "activeLayoutRoot" subtree is being mutated in a layout callback.
19641964
// - a different part of the render tree is doing a layout callback,
19651965
// and this subtree is being reparented to that subtree, as a result
19661966
// of global key reparenting.

packages/flutter/lib/src/widgets/overlay.dart

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,12 +1031,14 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin<RenderBox
10311031
void _addDeferredChild(_RenderDeferredLayoutBox child) {
10321032
assert(!_skipMarkNeedsLayout);
10331033
_skipMarkNeedsLayout = true;
1034-
10351034
adoptChild(child);
1036-
// When child has never been laid out before, mark its layout surrogate as
1037-
// needing layout so it's reachable via tree walk.
1038-
child._layoutSurrogate.markNeedsLayout();
10391035
_skipMarkNeedsLayout = false;
1036+
1037+
// After adding `child` to the render tree, we want to make sure it will be
1038+
// laid out in the same frame. This is done by calling markNeedsLayout on the
1039+
// layout surrgate. This ensures `child` is reachable via tree walk (see
1040+
// _RenderLayoutSurrogateProxyBox.performLayout).
1041+
child._layoutSurrogate.markNeedsLayout();
10401042
}
10411043

10421044
void _removeDeferredChild(_RenderDeferredLayoutBox child) {
@@ -1048,10 +1050,9 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin<RenderBox
10481050

10491051
@override
10501052
void markNeedsLayout() {
1051-
if (_skipMarkNeedsLayout) {
1052-
return;
1053+
if (!_skipMarkNeedsLayout) {
1054+
super.markNeedsLayout();
10531055
}
1054-
super.markNeedsLayout();
10551056
}
10561057

10571058
RenderBox? get _firstOnstageChild {
@@ -2088,7 +2089,7 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM
20882089
RenderObject? get debugLayoutParent => _layoutSurrogate;
20892090

20902091
void layoutByLayoutSurrogate() {
2091-
assert(!_parentDoingLayout);
2092+
assert(!_theaterDoingThisLayout);
20922093
final _RenderTheater? theater = parent as _RenderTheater?;
20932094
if (theater == null || !attached) {
20942095
assert(false, '$this is not attached to parent');
@@ -2097,25 +2098,26 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM
20972098
super.layout(BoxConstraints.tight(theater.constraints.biggest));
20982099
}
20992100

2100-
bool _parentDoingLayout = false;
2101+
bool _theaterDoingThisLayout = false;
21012102
@override
21022103
void layout(Constraints constraints, { bool parentUsesSize = false }) {
21032104
assert(_needsLayout == debugNeedsLayout);
21042105
// Only _RenderTheater calls this implementation.
21052106
assert(parent != null);
21062107
final bool scheduleDeferredLayout = _needsLayout || this.constraints != constraints;
2107-
assert(!_parentDoingLayout);
2108-
_parentDoingLayout = true;
2108+
assert(!_theaterDoingThisLayout);
2109+
_theaterDoingThisLayout = true;
21092110
super.layout(constraints, parentUsesSize: parentUsesSize);
2110-
assert(_parentDoingLayout);
2111-
_parentDoingLayout = false;
2111+
assert(_theaterDoingThisLayout);
2112+
_theaterDoingThisLayout = false;
21122113
_needsLayout = false;
21132114
assert(!debugNeedsLayout);
21142115
if (scheduleDeferredLayout) {
21152116
final _RenderTheater parent = this.parent! as _RenderTheater;
21162117
// Invoking markNeedsLayout as a layout callback allows this node to be
2117-
// merged back to the `PipelineOwner` if it's not already dirty. Otherwise
2118-
// this may cause some dirty descendants to performLayout a second time.
2118+
// merged back to the `PipelineOwner`'s dirty list in the right order, if
2119+
// it's not already dirty. Otherwise this may cause some dirty descendants
2120+
// to performLayout a second time.
21192121
parent.invokeLayoutCallback((BoxConstraints constraints) { markNeedsLayout(); });
21202122
}
21212123
}
@@ -2129,7 +2131,7 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM
21292131
@override
21302132
void performLayout() {
21312133
assert(!_debugMutationsLocked);
2132-
if (_parentDoingLayout) {
2134+
if (_theaterDoingThisLayout) {
21332135
_needsLayout = false;
21342136
return;
21352137
}

packages/flutter/test/widgets/overlay_portal_test.dart

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,42 @@ void main() {
255255
expect(tester.takeException(), isNull);
256256
});
257257

258+
testWidgets('No relayout boundary between OverlayPortal and Overlay', (WidgetTester tester) async {
259+
// Regression test for https://github.com/flutter/flutter/issues/133545.
260+
final GlobalKey key = GlobalKey(debugLabel: 'key');
261+
final Widget widget = Directionality(
262+
textDirection: TextDirection.ltr,
263+
child: Overlay(
264+
initialEntries: <OverlayEntry>[
265+
OverlayEntry(
266+
builder: (BuildContext context) {
267+
// The Positioned widget prevents a relayout boundary from being
268+
// introduced between the Overlay and OverlayPortal.
269+
return Positioned(
270+
top: 0,
271+
left: 0,
272+
child: OverlayPortal(
273+
controller: controller1,
274+
overlayChildBuilder: (BuildContext context) => SizedBox(key: key),
275+
child: const SizedBox(),
276+
),
277+
);
278+
},
279+
),
280+
],
281+
),
282+
);
283+
284+
controller1.hide();
285+
await tester.pumpWidget(widget);
286+
287+
controller1.show();
288+
await tester.pump();
289+
expect(find.byKey(key), findsOneWidget);
290+
expect(tester.takeException(), isNull);
291+
verifyTreeIsClean();
292+
});
293+
258294
testWidgets('Throws when the same controller is attached to multiple OverlayPortal', (WidgetTester tester) async {
259295
final OverlayPortalController controller = OverlayPortalController(debugLabel: 'local controller');
260296
final Widget widget = Directionality(
@@ -516,6 +552,52 @@ void main() {
516552
expect(tester.takeException(), isNull);
517553
});
518554

555+
testWidgets('works in a LayoutBuilder 3', (WidgetTester tester) async {
556+
late StateSetter setState;
557+
bool shouldShowChild = false;
558+
559+
Widget layoutBuilder(BuildContext context, BoxConstraints constraints) {
560+
return OverlayPortal(
561+
controller: controller2,
562+
overlayChildBuilder: (BuildContext context) => const SizedBox(),
563+
child: const SizedBox(),
564+
);
565+
}
566+
controller1.hide();
567+
controller2.hide();
568+
569+
await tester.pumpWidget(
570+
Directionality(
571+
textDirection: TextDirection.ltr,
572+
child: Overlay(
573+
initialEntries: <OverlayEntry>[
574+
OverlayStatefulEntry(builder: (BuildContext context, StateSetter setter) {
575+
setState = setter;
576+
// The Positioned widget ensures there's no relayout boundary
577+
// between the Overlay and the OverlayPortal.
578+
return Positioned(
579+
top: 0,
580+
left: 0,
581+
child: OverlayPortal(
582+
controller: controller1,
583+
overlayChildBuilder: (BuildContext context) => const SizedBox(),
584+
child: shouldShowChild ? LayoutBuilder(builder: layoutBuilder) : null,
585+
),
586+
);
587+
}),
588+
],
589+
),
590+
),
591+
);
592+
593+
controller1.show();
594+
controller2.show();
595+
setState(() { shouldShowChild = true; });
596+
597+
await tester.pump();
598+
expect(tester.takeException(), isNull);
599+
});
600+
519601
testWidgets('throws when no Overlay', (WidgetTester tester) async {
520602
await tester.pumpWidget(
521603
Directionality(

0 commit comments

Comments
 (0)