diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 9e560d4532..b1c20fb5e4 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -2049,7 +2049,9 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge assert(child._parent == this); assert(child.attached == attached); assert(child.parentData != null); - _cleanChildRelayoutBoundary(child); + if (!(_isRelayoutBoundary ?? true)) { + child._isRelayoutBoundary = null; + } child.parentData!.detach(); child.parentData = null; child._parent = null; @@ -2337,7 +2339,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge _owner = owner; // If the node was dirtied in some way while unattached, make sure to add // it to the appropriate dirty list now that an owner is available - if (_needsLayout && _relayoutBoundary != null) { + if (_needsLayout && _isRelayoutBoundary != null) { // Don't enter this block if we've never laid out at all; // scheduleInitialLayout() will handle it _needsLayout = false; @@ -2393,33 +2395,35 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge bool _needsLayout = true; - /// The nearest relayout boundary enclosing this render object, if known. + /// Whether this [RenderObject] is a known relayout boundary. /// - /// When a render object is marked as needing layout, its parent may - /// as a result also need to be marked as needing layout. - /// For details, see [markNeedsLayout]. - /// A render object where relayout does not require relayout of the parent - /// (because its size cannot change on relayout, or because - /// its parent does not use the child's size for its own layout) - /// is a "relayout boundary". + /// A relayout boundary is a [RenderObject] whose parent does not rely on the + /// child [RenderObject]'s size in its own layout algorithm. In other words, + /// if a [RenderObject]'s [performLayout] implementation does not ask the child + /// for its size at all, **the child** is a relayout boundary. /// - /// This property is set in [layout], and consulted by [markNeedsLayout] in - /// deciding whether to recursively mark the parent as also needing layout. + /// The type of "size" is typically defined by the coordinate system a + /// [RenderObject] subclass uses. For instance, [RenderSliver]s produce + /// [SliverGeometry] and [RenderBox]es produce [Size]. A parent [RenderObject] + /// may not read the child's size but still depend on the child's layout (using + /// a [RenderBox] child's baseline location, for example), this flag does not + /// reflect such dependencies and the [RenderObject] subclass must handle those + /// cases in its own implementation. See [RenderBox.markNeedsLayout] for an + /// example. /// - /// This property is initially null, and becomes null again if this - /// render object is removed from the tree (with [dropChild]); - /// it remains null until the first layout of this render object - /// after it was most recently added to the tree. - /// This property can also be null while an ancestor in the tree is - /// currently doing layout, until this render object itself does layout. + /// Relayout boundaries enable an important layout optimization: the parent not + /// depending on the size of a child means the child changing size does not + /// affect the layout of the parent. When a relayout boundary is marked as + /// needing layout, its parent does not have to be marked as dirty, hence the + /// name. For details, see [markNeedsLayout]. /// - /// When not null, the relayout boundary is either this render object itself - /// or one of its ancestors, and all the render objects in the ancestry chain - /// up through that ancestor have the same [_relayoutBoundary]. - /// Equivalently: when not null, the relayout boundary is either this render - /// object itself or the same as that of its parent. (So [_relayoutBoundary] - /// is one of `null`, `this`, or `parent!._relayoutBoundary!`.) - RenderObject? _relayoutBoundary; + /// This flag is typically set in [RenderObject.layout], and consulted by + /// [markNeedsLayout] in deciding whether to recursively mark the parent as + /// also needing layout. + /// + /// The flag is initially set to `null` when [layout] has yet been called, and + /// reset to `null` when the parent drops this child via [dropChild]. + bool? _isRelayoutBoundary; /// Whether [invokeLayoutCallback] for this render object is currently running. bool get debugDoingThisLayoutWithCallback => _doingThisLayoutWithCallback; @@ -2458,20 +2462,19 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge static bool debugCheckingIntrinsics = false; bool _debugRelayoutBoundaryAlreadyMarkedNeedsLayout() { - if (_relayoutBoundary == null) { - // We don't know where our relayout boundary is yet. - return true; - } - RenderObject node = this; - while (node != _relayoutBoundary) { - assert(node._relayoutBoundary == _relayoutBoundary); - assert(node.parent != null); - node = node.parent!; - if ((!node._needsLayout) && (!node._debugDoingThisLayout)) { + for ( + RenderObject? node = this; + node != null && node._isRelayoutBoundary != null; + node = node.parent + ) { + final bool alreadyMarkedNeedsLayout = node._needsLayout || node._debugDoingThisLayout; + if (!alreadyMarkedNeedsLayout) { return false; } + if (node._isRelayoutBoundary!) { + return true; + } } - assert(node._relayoutBoundary == node); return true; } @@ -2519,30 +2522,18 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge assert(_debugRelayoutBoundaryAlreadyMarkedNeedsLayout()); return; } - if (_relayoutBoundary == null) { - _needsLayout = true; - if (parent != null) { - // _relayoutBoundary is cleaned by an ancestor in RenderObject.layout. - // Conservatively mark everything dirty until it reaches the closest - // known relayout boundary. - markParentNeedsLayout(); - } - return; - } - if (_relayoutBoundary != this) { + _needsLayout = true; + if (owner case final PipelineOwner owner? when (_isRelayoutBoundary ?? false)) { + assert(() { + if (debugPrintMarkNeedsLayoutStacks) { + debugPrintStack(label: 'markNeedsLayout() called for $this'); + } + return true; + }()); + owner._nodesNeedingLayout.add(this); + owner.requestVisualUpdate(); + } else if (parent != null) { markParentNeedsLayout(); - } else { - _needsLayout = true; - if (owner != null) { - assert(() { - if (debugPrintMarkNeedsLayoutStacks) { - debugPrintStack(label: 'markNeedsLayout() called for $this'); - } - return true; - }()); - owner!._nodesNeedingLayout.add(this); - owner!.requestVisualUpdate(); - } } } @@ -2581,38 +2572,6 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge markParentNeedsLayout(); } - /// Set [_relayoutBoundary] to null throughout this render object's subtree, - /// stopping at relayout boundaries. - // This is a static method to reduce closure allocation with visitChildren. - static void _cleanChildRelayoutBoundary(RenderObject child) { - if (child._relayoutBoundary != child) { - child.visitChildren(_cleanChildRelayoutBoundary); - child._relayoutBoundary = null; - } - } - - // This is a static method to reduce closure allocation with visitChildren. - static void _propagateRelayoutBoundaryToChild(RenderObject child) { - if (child._relayoutBoundary == child) { - return; - } - final RenderObject? parentRelayoutBoundary = child.parent?._relayoutBoundary; - assert(parentRelayoutBoundary != null); - assert(parentRelayoutBoundary != child._relayoutBoundary); - child._setRelayoutBoundary(parentRelayoutBoundary!); - } - - /// Set [_relayoutBoundary] to [value] throughout this render object's - /// subtree, including this render object but stopping at relayout boundaries - /// thereafter. - void _setRelayoutBoundary(RenderObject value) { - assert(value != _relayoutBoundary); - // This may temporarily break the _relayoutBoundary invariant at children; - // the visitChildren restores the invariant. - _relayoutBoundary = value; - visitChildren(_propagateRelayoutBoundaryToChild); - } - /// Bootstrap the rendering pipeline by scheduling the very first layout. /// /// Requires this render object to be attached and that this render object @@ -2624,8 +2583,8 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge assert(attached); assert(parent == null); assert(!owner!._debugDoingLayout); - assert(_relayoutBoundary == null); - _relayoutBoundary = this; + assert(_isRelayoutBoundary == null); + _isRelayoutBoundary = true; assert(() { _debugCanParentUseSize = false; return true; @@ -2636,7 +2595,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge @pragma('vm:notify-debugger-on-exception') void _layoutWithoutResize() { assert(_needsLayout); - assert(_relayoutBoundary == this || this is RenderObjectWithLayoutCallbackMixin); + assert((_isRelayoutBoundary ?? false) || this is RenderObjectWithLayoutCallbackMixin); RenderObject? debugPreviousActiveLayout; assert(!_debugMutationsLocked); assert(!_doingThisLayoutWithCallback); @@ -2739,14 +2698,12 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge ); assert(!_debugDoingThisResize); assert(!_debugDoingThisLayout); - final bool isRelayoutBoundary = - !parentUsesSize || sizedByParent || constraints.isTight || parent == null; - final RenderObject relayoutBoundary = isRelayoutBoundary ? this : parent!._relayoutBoundary!; assert(() { _debugCanParentUseSize = parentUsesSize; return true; }()); + _isRelayoutBoundary = !parentUsesSize || sizedByParent || constraints.isTight || parent == null; if (!_needsLayout && constraints == _constraints) { assert(() { // in case parentUsesSize changed since the last invocation, set size @@ -2761,11 +2718,6 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge _debugDoingThisResize = false; return true; }()); - - if (relayoutBoundary != _relayoutBoundary) { - _setRelayoutBoundary(relayoutBoundary); - } - if (!kReleaseMode && debugProfileLayoutsEnabled) { FlutterTimeline.finishSync(); } @@ -2773,14 +2725,6 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge } _constraints = constraints; - if (_relayoutBoundary != null && relayoutBoundary != _relayoutBoundary) { - // The local relayout boundary has changed, must notify children in case - // they also need updating. Otherwise, they will be confused about what - // their actual relayout boundary is later. - visitChildren(_cleanChildRelayoutBoundary); - } - _relayoutBoundary = relayoutBoundary; - assert(!_debugMutationsLocked); assert(!_doingThisLayoutWithCallback); assert(() { @@ -3901,13 +3845,20 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge header += ' DISPOSED'; return header; } - if (_relayoutBoundary != null && _relayoutBoundary != this) { - int count = 1; - RenderObject? target = parent; - while (target != null && target != _relayoutBoundary) { - target = target.parent; - count += 1; + + int count = 0; + for ( + RenderObject? node = this; + node != null && !(node._isRelayoutBoundary ?? false); + node = node.parent + ) { + if (node._isRelayoutBoundary == null) { + count = -1; + break; } + count += 1; + } + if (count > 0) { header += ' relayoutBoundary=up$count'; } if (_needsLayout) { diff --git a/packages/flutter/test/widgets/layout_builder_test.dart b/packages/flutter/test/widgets/layout_builder_test.dart index 2133ab111f..e9d267c992 100644 --- a/packages/flutter/test/widgets/layout_builder_test.dart +++ b/packages/flutter/test/widgets/layout_builder_test.dart @@ -984,6 +984,60 @@ void main() { await tester.pump(); expect(rebuilt, isTrue); }); + + testWidgets('LayoutBuilder does not crash when it becomes kept-alive', ( + WidgetTester tester, + ) async { + final FocusNode focusNode = FocusNode(); + final TextEditingController controller = TextEditingController(); + addTearDown(focusNode.dispose); + addTearDown(controller.dispose); + final Widget layoutBuilderWithParent = SizedBox( + key: GlobalKey(), + child: LayoutBuilder( + builder: (BuildContext _, BoxConstraints _) { + // The text field keeps the widget alive in the SliverList. + return EditableText( + focusNode: focusNode, + backgroundCursorColor: const Color(0xFFFFFFFF), + cursorColor: const Color(0xFFFFFFFF), + style: const TextStyle(), + controller: controller, + ); + }, + ), + ); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: CustomScrollView( + slivers: [ + SliverList.list( + addRepaintBoundaries: false, + addSemanticIndexes: false, + children: [const SizedBox(height: 60), layoutBuilderWithParent], + ), + ], + ), + ), + ); + focusNode.requestFocus(); + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: CustomScrollView( + slivers: [ + SliverList.list( + addRepaintBoundaries: false, + addSemanticIndexes: false, + children: [const SizedBox(height: 6000), layoutBuilderWithParent], + ), + ], + ), + ), + ); + }); } class _SmartLayoutBuilder extends ConstrainedLayoutBuilder {