Unverified Commit dc8377b1 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Ensure OverlayPortal.overlayChild's renderObject is reachable via treewalk (#134497)

Fixes https://github.com/flutter/flutter/issues/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.
parent fcba7b3d
...@@ -1960,7 +1960,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge ...@@ -1960,7 +1960,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge
final bool mutationsToDirtySubtreesAllowed = activeLayoutRoot.owner?._debugAllowMutationsToDirtySubtrees ?? false; final bool mutationsToDirtySubtreesAllowed = activeLayoutRoot.owner?._debugAllowMutationsToDirtySubtrees ?? false;
final bool doingLayoutWithCallback = activeLayoutRoot._doingThisLayoutWithCallback; final bool doingLayoutWithCallback = activeLayoutRoot._doingThisLayoutWithCallback;
// Mutations on this subtree is allowed when: // Mutations on this subtree is allowed when:
// - the subtree is being mutated in a layout callback. // - the "activeLayoutRoot" subtree is being mutated in a layout callback.
// - a different part of the render tree is doing a layout callback, // - a different part of the render tree is doing a layout callback,
// and this subtree is being reparented to that subtree, as a result // and this subtree is being reparented to that subtree, as a result
// of global key reparenting. // of global key reparenting.
......
...@@ -1031,12 +1031,14 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin<RenderBox ...@@ -1031,12 +1031,14 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin<RenderBox
void _addDeferredChild(_RenderDeferredLayoutBox child) { void _addDeferredChild(_RenderDeferredLayoutBox child) {
assert(!_skipMarkNeedsLayout); assert(!_skipMarkNeedsLayout);
_skipMarkNeedsLayout = true; _skipMarkNeedsLayout = true;
adoptChild(child); adoptChild(child);
// When child has never been laid out before, mark its layout surrogate as
// needing layout so it's reachable via tree walk.
child._layoutSurrogate.markNeedsLayout();
_skipMarkNeedsLayout = false; _skipMarkNeedsLayout = false;
// After adding `child` to the render tree, we want to make sure it will be
// laid out in the same frame. This is done by calling markNeedsLayout on the
// layout surrgate. This ensures `child` is reachable via tree walk (see
// _RenderLayoutSurrogateProxyBox.performLayout).
child._layoutSurrogate.markNeedsLayout();
} }
void _removeDeferredChild(_RenderDeferredLayoutBox child) { void _removeDeferredChild(_RenderDeferredLayoutBox child) {
...@@ -1048,11 +1050,10 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin<RenderBox ...@@ -1048,11 +1050,10 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin<RenderBox
@override @override
void markNeedsLayout() { void markNeedsLayout() {
if (_skipMarkNeedsLayout) { if (!_skipMarkNeedsLayout) {
return;
}
super.markNeedsLayout(); super.markNeedsLayout();
} }
}
RenderBox? get _firstOnstageChild { RenderBox? get _firstOnstageChild {
if (skipCount == super.childCount) { if (skipCount == super.childCount) {
...@@ -2088,7 +2089,7 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM ...@@ -2088,7 +2089,7 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM
RenderObject? get debugLayoutParent => _layoutSurrogate; RenderObject? get debugLayoutParent => _layoutSurrogate;
void layoutByLayoutSurrogate() { void layoutByLayoutSurrogate() {
assert(!_parentDoingLayout); assert(!_theaterDoingThisLayout);
final _RenderTheater? theater = parent as _RenderTheater?; final _RenderTheater? theater = parent as _RenderTheater?;
if (theater == null || !attached) { if (theater == null || !attached) {
assert(false, '$this is not attached to parent'); assert(false, '$this is not attached to parent');
...@@ -2097,25 +2098,26 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM ...@@ -2097,25 +2098,26 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM
super.layout(BoxConstraints.tight(theater.constraints.biggest)); super.layout(BoxConstraints.tight(theater.constraints.biggest));
} }
bool _parentDoingLayout = false; bool _theaterDoingThisLayout = false;
@override @override
void layout(Constraints constraints, { bool parentUsesSize = false }) { void layout(Constraints constraints, { bool parentUsesSize = false }) {
assert(_needsLayout == debugNeedsLayout); assert(_needsLayout == debugNeedsLayout);
// Only _RenderTheater calls this implementation. // Only _RenderTheater calls this implementation.
assert(parent != null); assert(parent != null);
final bool scheduleDeferredLayout = _needsLayout || this.constraints != constraints; final bool scheduleDeferredLayout = _needsLayout || this.constraints != constraints;
assert(!_parentDoingLayout); assert(!_theaterDoingThisLayout);
_parentDoingLayout = true; _theaterDoingThisLayout = true;
super.layout(constraints, parentUsesSize: parentUsesSize); super.layout(constraints, parentUsesSize: parentUsesSize);
assert(_parentDoingLayout); assert(_theaterDoingThisLayout);
_parentDoingLayout = false; _theaterDoingThisLayout = false;
_needsLayout = false; _needsLayout = false;
assert(!debugNeedsLayout); assert(!debugNeedsLayout);
if (scheduleDeferredLayout) { if (scheduleDeferredLayout) {
final _RenderTheater parent = this.parent! as _RenderTheater; final _RenderTheater parent = this.parent! as _RenderTheater;
// Invoking markNeedsLayout as a layout callback allows this node to be // Invoking markNeedsLayout as a layout callback allows this node to be
// merged back to the `PipelineOwner` if it's not already dirty. Otherwise // merged back to the `PipelineOwner`'s dirty list in the right order, if
// this may cause some dirty descendants to performLayout a second time. // it's not already dirty. Otherwise this may cause some dirty descendants
// to performLayout a second time.
parent.invokeLayoutCallback((BoxConstraints constraints) { markNeedsLayout(); }); parent.invokeLayoutCallback((BoxConstraints constraints) { markNeedsLayout(); });
} }
} }
...@@ -2129,7 +2131,7 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM ...@@ -2129,7 +2131,7 @@ final class _RenderDeferredLayoutBox extends RenderProxyBox with _RenderTheaterM
@override @override
void performLayout() { void performLayout() {
assert(!_debugMutationsLocked); assert(!_debugMutationsLocked);
if (_parentDoingLayout) { if (_theaterDoingThisLayout) {
_needsLayout = false; _needsLayout = false;
return; return;
} }
......
...@@ -255,6 +255,42 @@ void main() { ...@@ -255,6 +255,42 @@ void main() {
expect(tester.takeException(), isNull); expect(tester.takeException(), isNull);
}); });
testWidgets('No relayout boundary between OverlayPortal and Overlay', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/133545.
final GlobalKey key = GlobalKey(debugLabel: 'key');
final Widget widget = Directionality(
textDirection: TextDirection.ltr,
child: Overlay(
initialEntries: <OverlayEntry>[
OverlayEntry(
builder: (BuildContext context) {
// The Positioned widget prevents a relayout boundary from being
// introduced between the Overlay and OverlayPortal.
return Positioned(
top: 0,
left: 0,
child: OverlayPortal(
controller: controller1,
overlayChildBuilder: (BuildContext context) => SizedBox(key: key),
child: const SizedBox(),
),
);
},
),
],
),
);
controller1.hide();
await tester.pumpWidget(widget);
controller1.show();
await tester.pump();
expect(find.byKey(key), findsOneWidget);
expect(tester.takeException(), isNull);
verifyTreeIsClean();
});
testWidgets('Throws when the same controller is attached to multiple OverlayPortal', (WidgetTester tester) async { testWidgets('Throws when the same controller is attached to multiple OverlayPortal', (WidgetTester tester) async {
final OverlayPortalController controller = OverlayPortalController(debugLabel: 'local controller'); final OverlayPortalController controller = OverlayPortalController(debugLabel: 'local controller');
final Widget widget = Directionality( final Widget widget = Directionality(
...@@ -516,6 +552,52 @@ void main() { ...@@ -516,6 +552,52 @@ void main() {
expect(tester.takeException(), isNull); expect(tester.takeException(), isNull);
}); });
testWidgets('works in a LayoutBuilder 3', (WidgetTester tester) async {
late StateSetter setState;
bool shouldShowChild = false;
Widget layoutBuilder(BuildContext context, BoxConstraints constraints) {
return OverlayPortal(
controller: controller2,
overlayChildBuilder: (BuildContext context) => const SizedBox(),
child: const SizedBox(),
);
}
controller1.hide();
controller2.hide();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Overlay(
initialEntries: <OverlayEntry>[
OverlayStatefulEntry(builder: (BuildContext context, StateSetter setter) {
setState = setter;
// The Positioned widget ensures there's no relayout boundary
// between the Overlay and the OverlayPortal.
return Positioned(
top: 0,
left: 0,
child: OverlayPortal(
controller: controller1,
overlayChildBuilder: (BuildContext context) => const SizedBox(),
child: shouldShowChild ? LayoutBuilder(builder: layoutBuilder) : null,
),
);
}),
],
),
),
);
controller1.show();
controller2.show();
setState(() { shouldShowChild = true; });
await tester.pump();
expect(tester.takeException(), isNull);
});
testWidgets('throws when no Overlay', (WidgetTester tester) async { testWidgets('throws when no Overlay', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
Directionality( Directionality(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment