From 8e15e56c817aacd8f58b266c33b696b49c57d6c1 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Mon, 3 Jun 2024 09:45:23 -0700 Subject: [PATCH] Reland "Prevent LayoutBuilder from rebuilding more than once (#147856)" (#149303) Diff commit: https://github.com/flutter/flutter/commit/a3f7acab0c324cb6517322280150e21bd13e362d In the failing tests the debugger treated the exception thrown by `Element.rebuild` as a caught exception in the absence of the vm pragma. --- .../flutter/lib/src/rendering/object.dart | 9 +- .../flutter/lib/src/widgets/framework.dart | 462 ++++++++++++------ .../lib/src/widgets/layout_builder.dart | 77 +-- .../flutter/test/widgets/framework_test.dart | 275 +++++++++-- .../layout_builder_and_global_keys_test.dart | 42 ++ .../layout_builder_and_state_test.dart | 3 +- .../test/widgets/layout_builder_test.dart | 81 +++ .../test/widgets/overlay_portal_test.dart | 13 +- 8 files changed, 714 insertions(+), 248 deletions(-) diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index e699bf691fe6..5b3c4b579579 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -1999,8 +1999,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge } if (!activeLayoutRoot._debugMutationsLocked) { - final RenderObject? p = activeLayoutRoot.debugLayoutParent; - activeLayoutRoot = p is RenderObject ? p : null; + activeLayoutRoot = activeLayoutRoot.debugLayoutParent; } else { // activeLayoutRoot found. break; @@ -3004,7 +3003,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge owner!._nodesNeedingPaint.add(this); owner!.requestVisualUpdate(); } - } else if (parent is RenderObject) { + } else if (parent != null) { parent!.markNeedsPaint(); } else { assert(() { @@ -3020,9 +3019,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge // // Trees rooted at a RenderView do not go through this // code path because RenderViews are repaint boundaries. - if (owner != null) { - owner!.requestVisualUpdate(); - } + owner?.requestVisualUpdate(); } } diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index f45862549b4b..a84557d4071c 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -2610,6 +2610,207 @@ abstract class BuildContext { DiagnosticsNode describeOwnershipChain(String name); } +/// A class that determines the scope of a [BuildOwner.buildScope] operation. +/// +/// The [BuildOwner.buildScope] method rebuilds all dirty [Element]s who share +/// the same [Element.buildScope] as its `context` argument, and skips those +/// with a different [Element.buildScope]. +/// +/// [Element]s by default have the same `buildScope` as their parents. Special +/// [Element]s may override [Element.buildScope] to create an isolated build scope +/// for its descendants. The [LayoutBuilder] widget, for example, establishes its +/// own [BuildScope] such that no descendant [Element]s may rebuild prematurely +/// until the incoming constraints are known. +final class BuildScope { + /// Creates a [BuildScope] with an optional [scheduleRebuild] callback. + BuildScope({ this.scheduleRebuild }); + + // Whether `scheduleRebuild` is called. + bool _buildScheduled = false; + // Whether [BuildOwner.buildScope] is actively running in this [BuildScope]. + bool _building = false; + + /// An optional [VoidCallback] that will be called when [Element]s in this + /// [BuildScope] are marked as dirty for the first time. + /// + /// This callback usually signifies that the [BuildOwner.buildScope] method + /// must be called at a later time in this frame to rebuild dirty elements in + /// this [BuildScope]. It will **not** be called if this scope is actively being + /// built by [BuildOwner.buildScope], since the [BuildScope] will be clean when + /// [BuildOwner.buildScope] returns. + final VoidCallback? scheduleRebuild; + + /// Whether [_dirtyElements] need to be sorted again as a result of more + /// elements becoming dirty during the build. + /// + /// This is necessary to preserve the sort order defined by [Element._sort]. + /// + /// This field is set to null when [buildScope] is not actively rebuilding + /// the widget tree. + bool? _dirtyElementsNeedsResorting; + final List _dirtyElements = []; + + @pragma('dart2js:tryInline') + @pragma('vm:prefer-inline') + @pragma('wasm:prefer-inline') + void _scheduleBuildFor(Element element) { + assert(identical(element.buildScope, this)); + if (!element._inDirtyList) { + _dirtyElements.add(element); + element._inDirtyList = true; + } + if (!_buildScheduled && !_building) { + _buildScheduled = true; + scheduleRebuild?.call(); + } + if (_dirtyElementsNeedsResorting != null) { + _dirtyElementsNeedsResorting = true; + } + } + + @pragma('dart2js:tryInline') + @pragma('vm:prefer-inline') + @pragma('wasm:prefer-inline') + @pragma('vm:notify-debugger-on-exception') + void _tryRebuild(Element element) { + assert(element._inDirtyList); + assert(identical(element.buildScope, this)); + final bool isTimelineTracked = !kReleaseMode && _isProfileBuildsEnabledFor(element.widget); + if (isTimelineTracked) { + Map? debugTimelineArguments; + assert(() { + if (kDebugMode && debugEnhanceBuildTimelineArguments) { + debugTimelineArguments = element.widget.toDiagnosticsNode().toTimelineArguments(); + } + return true; + }()); + FlutterTimeline.startSync( + '${element.widget.runtimeType}', + arguments: debugTimelineArguments, + ); + } + try { + element.rebuild(); + } catch (e, stack) { + _reportException( + ErrorDescription('while rebuilding dirty elements'), + e, + stack, + informationCollector: () => [ + if (kDebugMode) + DiagnosticsDebugCreator(DebugCreator(element)), + element.describeElement('The element being rebuilt at the time was') + ], + ); + } + if (isTimelineTracked) { + FlutterTimeline.finishSync(); + } + } + + bool _debugAssertElementInScope(Element element, Element debugBuildRoot) { + final bool isInScope = element._debugIsDescsendantOf(debugBuildRoot) + || !element.debugIsActive; + if (isInScope) { + return true; + } + throw FlutterError.fromParts([ + ErrorSummary('Tried to build dirty widget in the wrong build scope.'), + ErrorDescription( + 'A widget which was marked as dirty and is still active was scheduled to be built, ' + 'but the current build scope unexpectedly does not contain that widget.', + ), + ErrorHint( + 'Sometimes this is detected when an element is removed from the widget tree, but the ' + 'element somehow did not get marked as inactive. In that case, it might be caused by ' + 'an ancestor element failing to implement visitChildren correctly, thus preventing ' + 'some or all of its descendants from being correctly deactivated.', + ), + DiagnosticsProperty( + 'The root of the build scope was', + debugBuildRoot, + style: DiagnosticsTreeStyle.errorProperty, + ), + DiagnosticsProperty( + 'The offending element (which does not appear to be a descendant of the root of the build scope) was', + element, + style: DiagnosticsTreeStyle.errorProperty, + ), + ]); + } + + @pragma('vm:notify-debugger-on-exception') + void _flushDirtyElements({ required Element debugBuildRoot }) { + assert(_dirtyElementsNeedsResorting == null, '_flushDirtyElements must be non-reentrant'); + _dirtyElements.sort(Element._sort); + _dirtyElementsNeedsResorting = false; + try { + for (int index = 0; index < _dirtyElements.length; index = _dirtyElementIndexAfter(index)) { + final Element element = _dirtyElements[index]; + if (identical(element.buildScope, this)) { + assert(_debugAssertElementInScope(element, debugBuildRoot)); + _tryRebuild(element); + } + } + assert(() { + final Iterable missedElements = _dirtyElements.where((Element element) => element.debugIsActive && element.dirty && identical(element.buildScope, this)); + if (missedElements.isNotEmpty) { + throw FlutterError.fromParts([ + ErrorSummary('buildScope missed some dirty elements.'), + ErrorHint('This probably indicates that the dirty list should have been resorted but was not.'), + DiagnosticsProperty( + 'The context argument of the buildScope call was', + debugBuildRoot, + style: DiagnosticsTreeStyle.errorProperty, + ), + Element.describeElements('The list of missed elements at the end of the buildScope call was', missedElements), + ]); + } + return true; + }()); + } finally { + for (final Element element in _dirtyElements) { + if (identical(element.buildScope, this)) { + element._inDirtyList = false; + } + } + _dirtyElements.clear(); + _dirtyElementsNeedsResorting = null; + _buildScheduled = false; + } + } + + @pragma('dart2js:tryInline') + @pragma('vm:prefer-inline') + @pragma('wasm:prefer-inline') + int _dirtyElementIndexAfter(int index) { + if (!_dirtyElementsNeedsResorting!) { + return index + 1; + } + index += 1; + _dirtyElements.sort(Element._sort); + _dirtyElementsNeedsResorting = false; + while (index > 0 && _dirtyElements[index - 1].dirty) { + // It is possible for previously dirty but inactive widgets to move right in the list. + // We therefore have to move the index left in the list to account for this. + // We don't know how many could have moved. However, we do know that the only possible + // change to the list is that nodes that were previously to the left of the index have + // now moved to be to the right of the right-most cleaned node, and we do know that + // all the clean nodes were to the left of the index. So we move the index left + // until just after the right-most clean node. + index -= 1; + } + assert(() { + for (int i = index - 1; i >= 0; i -= 1) { + final Element element = _dirtyElements[i]; + assert(!element.dirty || element._lifecycleState != _ElementLifecycle.active); + } + return true; + }()); + return index; + } +} + /// Manager class for the widgets framework. /// /// This class tracks which widgets need rebuilding, and handles other tasks @@ -2651,23 +2852,8 @@ class BuildOwner { final _InactiveElements _inactiveElements = _InactiveElements(); - final List _dirtyElements = []; bool _scheduledFlushDirtyElements = false; - /// Whether [_dirtyElements] need to be sorted again as a result of more - /// elements becoming dirty during the build. - /// - /// This is necessary to preserve the sort order defined by [Element._sort]. - /// - /// This field is set to null when [buildScope] is not actively rebuilding - /// the widget tree. - bool? _dirtyElementsNeedsResorting; - - /// Whether [buildScope] is actively rebuilding the widget tree. - /// - /// [scheduleBuildFor] should only be called when this value is true. - bool get _debugIsInBuildScope => _dirtyElementsNeedsResorting != null; - /// The object in charge of the focus tree. /// /// Rarely used directly. Instead, consider using [FocusScope.of] to obtain @@ -2686,9 +2872,10 @@ class BuildOwner { /// when [WidgetsBinding.drawFrame] calls [buildScope]. void scheduleBuildFor(Element element) { assert(element.owner == this); + assert(element._parentBuildScope != null); assert(() { if (debugPrintScheduleBuildForStacks) { - debugPrintStack(label: 'scheduleBuildFor() called for $element${_dirtyElements.contains(element) ? " (ALREADY IN LIST)" : ""}'); + debugPrintStack(label: 'scheduleBuildFor() called for $element${element.buildScope._dirtyElements.contains(element) ? " (ALREADY IN LIST)" : ""}'); } if (!element.dirty) { throw FlutterError.fromParts([ @@ -2707,34 +2894,36 @@ class BuildOwner { } return true; }()); - if (element._inDirtyList) { - assert(() { - if (debugPrintScheduleBuildForStacks) { - debugPrintStack(label: 'BuildOwner.scheduleBuildFor() called; _dirtyElementsNeedsResorting was $_dirtyElementsNeedsResorting (now true); dirty list is: $_dirtyElements'); - } - if (!_debugIsInBuildScope) { - throw FlutterError.fromParts([ - ErrorSummary('BuildOwner.scheduleBuildFor() called inappropriately.'), - ErrorHint( - 'The BuildOwner.scheduleBuildFor() method should only be called while the ' - 'buildScope() method is actively rebuilding the widget tree.', - ), - ]); - } - return true; - }()); - _dirtyElementsNeedsResorting = true; - return; - } + final BuildScope buildScope = element.buildScope; + assert(() { + if (debugPrintScheduleBuildForStacks && element._inDirtyList) { + debugPrintStack( + label: 'BuildOwner.scheduleBuildFor() called; ' + '_dirtyElementsNeedsResorting was ${buildScope._dirtyElementsNeedsResorting} (now true); ' + 'The dirty list for the current build scope is: ${buildScope._dirtyElements}', + ); + } + // When reactivating an inactivate Element, _scheduleBuildFor should only be + // called within _flushDirtyElements. + if (!_debugBuilding && element._inDirtyList) { + throw FlutterError.fromParts([ + ErrorSummary('BuildOwner.scheduleBuildFor() called inappropriately.'), + ErrorHint( + 'The BuildOwner.scheduleBuildFor() method should only be called while the ' + 'buildScope() method is actively rebuilding the widget tree.', + ), + ]); + } + return true; + }()); if (!_scheduledFlushDirtyElements && onBuildScheduled != null) { _scheduledFlushDirtyElements = true; onBuildScheduled!(); } - _dirtyElements.add(element); - element._inDirtyList = true; + buildScope._scheduleBuildFor(element); assert(() { if (debugPrintScheduleBuildForStacks) { - debugPrint('...dirty list is now: $_dirtyElements'); + debugPrint("...the build scope's dirty list is now: $buildScope._dirtyElements"); } return true; }()); @@ -2799,14 +2988,18 @@ class BuildOwner { /// often. @pragma('vm:notify-debugger-on-exception') void buildScope(Element context, [ VoidCallback? callback ]) { - if (callback == null && _dirtyElements.isEmpty) { + final BuildScope buildScope = context.buildScope; + if (callback == null && buildScope._dirtyElements.isEmpty) { return; } assert(_debugStateLockLevel >= 0); assert(!_debugBuilding); assert(() { if (debugPrintBuildScope) { - debugPrint('buildScope called with context $context; dirty list is: $_dirtyElements'); + debugPrint( + 'buildScope called with context $context; ' + "its build scope's dirty list is: ${buildScope._dirtyElements}", + ); } _debugStateLockLevel += 1; _debugBuilding = true; @@ -2817,8 +3010,8 @@ class BuildOwner { assert(() { if (debugEnhanceBuildTimelineArguments) { debugTimelineArguments = { - 'dirty count': '${_dirtyElements.length}', - 'dirty list': '$_dirtyElements', + 'build scope dirty count': '${buildScope._dirtyElements.length}', + 'build scope dirty list': '${buildScope._dirtyElements}', 'lock level': '$_debugStateLockLevel', 'scope context': '$context', }; @@ -2832,6 +3025,7 @@ class BuildOwner { } try { _scheduledFlushDirtyElements = true; + buildScope._building = true; if (callback != null) { assert(_debugStateLocked); Element? debugPreviousBuildTarget; @@ -2840,7 +3034,6 @@ class BuildOwner { _debugCurrentBuildTarget = context; return true; }()); - _dirtyElementsNeedsResorting = false; try { callback(); } finally { @@ -2852,110 +3045,10 @@ class BuildOwner { }()); } } - _dirtyElements.sort(Element._sort); - _dirtyElementsNeedsResorting = false; - int dirtyCount = _dirtyElements.length; - int index = 0; - while (index < dirtyCount) { - final Element element = _dirtyElements[index]; - assert(element._inDirtyList); - assert(() { - if (element._lifecycleState == _ElementLifecycle.active && !element._debugIsInScope(context)) { - throw FlutterError.fromParts([ - ErrorSummary('Tried to build dirty widget in the wrong build scope.'), - ErrorDescription( - 'A widget which was marked as dirty and is still active was scheduled to be built, ' - 'but the current build scope unexpectedly does not contain that widget.', - ), - ErrorHint( - 'Sometimes this is detected when an element is removed from the widget tree, but the ' - 'element somehow did not get marked as inactive. In that case, it might be caused by ' - 'an ancestor element failing to implement visitChildren correctly, thus preventing ' - 'some or all of its descendants from being correctly deactivated.', - ), - DiagnosticsProperty( - 'The root of the build scope was', - context, - style: DiagnosticsTreeStyle.errorProperty, - ), - DiagnosticsProperty( - 'The offending element (which does not appear to be a descendant of the root of the build scope) was', - element, - style: DiagnosticsTreeStyle.errorProperty, - ), - ]); - } - return true; - }()); - final bool isTimelineTracked = !kReleaseMode && _isProfileBuildsEnabledFor(element.widget); - if (isTimelineTracked) { - Map? debugTimelineArguments; - assert(() { - if (kDebugMode && debugEnhanceBuildTimelineArguments) { - debugTimelineArguments = element.widget.toDiagnosticsNode().toTimelineArguments(); - } - return true; - }()); - FlutterTimeline.startSync( - '${element.widget.runtimeType}', - arguments: debugTimelineArguments, - ); - } - try { - element.rebuild(); - } catch (e, stack) { - _reportException( - ErrorDescription('while rebuilding dirty elements'), - e, - stack, - informationCollector: () => [ - if (kDebugMode && index < _dirtyElements.length) - DiagnosticsDebugCreator(DebugCreator(element)), - if (index < _dirtyElements.length) - element.describeElement('The element being rebuilt at the time was index $index of $dirtyCount') - else - ErrorHint('The element being rebuilt at the time was index $index of $dirtyCount, but _dirtyElements only had ${_dirtyElements.length} entries. This suggests some confusion in the framework internals.'), - ], - ); - } - if (isTimelineTracked) { - FlutterTimeline.finishSync(); - } - index += 1; - if (dirtyCount < _dirtyElements.length || _dirtyElementsNeedsResorting!) { - _dirtyElements.sort(Element._sort); - _dirtyElementsNeedsResorting = false; - dirtyCount = _dirtyElements.length; - while (index > 0 && _dirtyElements[index - 1].dirty) { - // It is possible for previously dirty but inactive widgets to move right in the list. - // We therefore have to move the index left in the list to account for this. - // We don't know how many could have moved. However, we do know that the only possible - // change to the list is that nodes that were previously to the left of the index have - // now moved to be to the right of the right-most cleaned node, and we do know that - // all the clean nodes were to the left of the index. So we move the index left - // until just after the right-most clean node. - index -= 1; - } - } - } - assert(() { - if (_dirtyElements.any((Element element) => element._lifecycleState == _ElementLifecycle.active && element.dirty)) { - throw FlutterError.fromParts([ - ErrorSummary('buildScope missed some dirty elements.'), - ErrorHint('This probably indicates that the dirty list should have been resorted but was not.'), - Element.describeElements('The list of dirty elements at the end of the buildScope call was', _dirtyElements), - ]); - } - return true; - }()); + buildScope._flushDirtyElements(debugBuildRoot: context); } finally { - for (final Element element in _dirtyElements) { - assert(element._inDirtyList); - element._inDirtyList = false; - } - _dirtyElements.clear(); + buildScope._building = false; _scheduledFlushDirtyElements = false; - _dirtyElementsNeedsResorting = null; if (!kReleaseMode) { FlutterTimeline.finishSync(); } @@ -2975,9 +3068,8 @@ class BuildOwner { Map>? _debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans; void _debugTrackElementThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans(Element node, GlobalKey key) { - _debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans ??= HashMap>(); - final Set keys = _debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans! - .putIfAbsent(node, () => HashSet()); + final Map> map = _debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans ??= HashMap>(); + final Set keys = map.putIfAbsent(node, () => HashSet()); keys.add(key); } @@ -3177,8 +3269,7 @@ class BuildOwner { try { _debugVerifyGlobalKeyReservation(); _debugVerifyIllFatedPopulation(); - if (_debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans != null && - _debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans!.isNotEmpty) { + if (_debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans?.isNotEmpty ?? false) { final Set keys = HashSet(); for (final Element element in _debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans!.keys) { if (element._lifecycleState != _ElementLifecycle.defunct) { @@ -3512,6 +3603,41 @@ abstract class Element extends DiagnosticableTree implements BuildContext { BuildOwner? get owner => _owner; BuildOwner? _owner; + /// A [BuildScope] whose dirty [Element]s can only be rebuilt by + /// [BuildOwner.buildScope] calls whose `context` argument is an [Element] + /// within this [BuildScope]. + /// + /// The getter typically is only safe to access when this [Element] is [mounted]. + /// + /// The default implementation returns the parent [Element]'s [buildScope], + /// as in most cases an [Element] is ready to rebuild as soon as its ancestors + /// are no longer dirty. One notable exception is [LayoutBuilder]'s + /// descendants, which must not rebuild until the incoming constraints become + /// available. [LayoutBuilder]'s [Element] overrides [buildScope] to make none + /// of its descendants can rebuild until the incoming constraints are known. + /// + /// If you choose to override this getter to establish your own [BuildScope], + /// to flush the dirty [Element]s in the [BuildScope] you need to manually call + /// [BuildOwner.buildScope] with the root [Element] of your [BuildScope] when + /// appropriate, as the Flutter framework does not try to register or manage + /// custom [BuildScope]s. + /// + /// Always return the same [BuildScope] instance if you override this getter. + /// Changing the value returned by this getter at runtime is not + /// supported. + /// + /// The [updateChild] method ignores [buildScope]: if the parent [Element] + /// calls [updateChild] on a child with a different [BuildScope], the child may + /// still rebuild. + /// + /// See also: + /// + /// * [LayoutBuilder], a widget that establishes a custom [BuildScope]. + BuildScope get buildScope => _parentBuildScope!; + // The cached value of the parent Element's build scope. The cache is updated + // when this Element mounts or reparents. + BuildScope? _parentBuildScope; + /// {@template flutter.widgets.Element.reassemble} /// Called whenever the application is reassembled during debugging, for /// example during hot reload. @@ -3548,15 +3674,12 @@ abstract class Element extends DiagnosticableTree implements BuildContext { }); } - bool _debugIsInScope(Element target) { - Element? current = this; - while (current != null) { - if (target == current) { - return true; - } - current = current._parent; + bool _debugIsDescsendantOf(Element target) { + Element? element = this; + while (element != null && element.depth > target.depth) { + element = element._parent; } - return false; + return element == target; } /// The render object at (or below) this location in the tree. @@ -4102,6 +4225,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { // (the root node), the owner should have already been assigned. // See RootRenderObjectElement.assignOwner(). _owner = parent.owner; + _parentBuildScope = parent.buildScope; } assert(owner != null); final Key? key = widget.key; @@ -4186,6 +4310,19 @@ abstract class Element extends DiagnosticableTree implements BuildContext { } } + void _updateBuildScopeRecursively() { + if (identical(buildScope, _parent?.buildScope)) { + return; + } + // Unset the _inDirtyList flag so this Element can be added to the dirty list + // of the new build scope if it's dirty. + _inDirtyList = false; + _parentBuildScope = _parent?.buildScope; + visitChildren((Element child) { + child._updateBuildScopeRecursively(); + }); + } + /// Remove [renderObject] from the render tree. /// /// The default implementation of this function calls @@ -4425,6 +4562,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { void _activateWithParent(Element parent, Object? newSlot) { assert(_lifecycleState == _ElementLifecycle.inactive); _parent = parent; + _owner = parent.owner; assert(() { if (debugPrintGlobalKeyedWidgetLifecycle) { debugPrint('Reactivating $this (now child of $_parent).'); @@ -4432,6 +4570,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { return true; }()); _updateDepth(_parent!.depth); + _updateBuildScopeRecursively(); _activateRecursively(this); attachRenderObject(newSlot); assert(_lifecycleState == _ElementLifecycle.active); @@ -4994,8 +5133,8 @@ abstract class Element extends DiagnosticableTree implements BuildContext { bool get dirty => _dirty; bool _dirty = true; - // Whether this is in owner._dirtyElements. This is used to know whether we - // should be adding the element back into the list when it's reactivated. + // Whether this is in _buildScope._dirtyElements. This is used to know whether + // we should be adding the element back into the list when it's reactivated. bool _inDirtyList = false; // Whether we've already built or not. Set in [rebuild]. @@ -5019,7 +5158,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { if (owner!._debugBuilding) { assert(owner!._debugCurrentBuildTarget != null); assert(owner!._debugStateLocked); - if (_debugIsInScope(owner!._debugCurrentBuildTarget!)) { + if (_debugIsDescsendantOf(owner!._debugCurrentBuildTarget!)) { return true; } final List information = [ @@ -6687,6 +6826,7 @@ mixin RootElementMixin on Element { // ignore: use_setters_to_change_properties, (API predates enforcing the lint) void assignOwner(BuildOwner owner) { _owner = owner; + _parentBuildScope = BuildScope(); } @override diff --git a/packages/flutter/lib/src/widgets/layout_builder.dart b/packages/flutter/lib/src/widgets/layout_builder.dart index 766b4bede1a2..69efb2dfaa79 100644 --- a/packages/flutter/lib/src/widgets/layout_builder.dart +++ b/packages/flutter/lib/src/widgets/layout_builder.dart @@ -80,6 +80,11 @@ class _LayoutBuilderElement extends RenderOb Element? _child; + @override + BuildScope get buildScope => LayoutBuilder.applyDoubleRebuildFix ? _buildScope : super.buildScope; + + late final BuildScope _buildScope = BuildScope(scheduleRebuild: renderObject.markNeedsLayout); + @override void visitChildren(ElementVisitor visitor) { if (_child != null) { @@ -97,7 +102,7 @@ class _LayoutBuilderElement extends RenderOb @override void mount(Element? parent, Object? newSlot) { super.mount(parent, newSlot); // Creates the renderObject. - renderObject.updateCallback(_layout); + renderObject.updateCallback(_rebuildWithConstraints); } @override @@ -107,12 +112,20 @@ class _LayoutBuilderElement extends RenderOb super.update(newWidget); assert(widget == newWidget); - renderObject.updateCallback(_layout); + renderObject.updateCallback(_rebuildWithConstraints); if (newWidget.updateShouldRebuild(oldWidget)) { - renderObject.markNeedsBuild(); + _needsBuild = true; + renderObject.markNeedsLayout(); } } + @override + void markNeedsBuild() { + super.markNeedsBuild(); + renderObject.markNeedsLayout(); + _needsBuild = true; + } + @override void performRebuild() { // This gets called if markNeedsBuild() is called on us. @@ -121,7 +134,8 @@ class _LayoutBuilderElement extends RenderOb // Force the callback to be called, even if the layout constraints are the // same. This is because that callback may depend on the updated widget // configuration, or an inherited widget. - renderObject.markNeedsBuild(); + renderObject.markNeedsLayout(); + _needsBuild = true; super.performRebuild(); // Calls widget.updateRenderObject (a no-op in this case). } @@ -131,9 +145,15 @@ class _LayoutBuilderElement extends RenderOb super.unmount(); } - void _layout(ConstraintType constraints) { + // The constraints that were passed to this class last time it was laid out. + // These constraints are compared to the new constraints to determine whether + // [ConstrainedLayoutBuilder.builder] needs to be called. + ConstraintType? _previousConstraints; + bool _needsBuild = true; + + void _rebuildWithConstraints(ConstraintType constraints) { @pragma('vm:notify-debugger-on-exception') - void layoutCallback() { + void updateChildCallback() { Widget built; try { built = (widget as ConstrainedLayoutBuilder).builder(this, constraints); @@ -167,10 +187,16 @@ class _LayoutBuilderElement extends RenderOb ), ); _child = updateChild(null, built, slot); + } finally { + _needsBuild = false; + _previousConstraints = constraints; } } - owner!.buildScope(this, layoutCallback); + final VoidCallback? callback = _needsBuild || (constraints != _previousConstraints) + ? updateChildCallback + : null; + owner!.buildScope(this, callback); } @override @@ -211,42 +237,13 @@ mixin RenderConstrainedLayoutBuilder { required super.builder, }); + /// Temporary flag that controls whether [LayoutBuilder]s and + /// [SliverLayoutBuilder]s should apply the double rebuild fix. This flag is + /// for migration only and **SHOULD NOT BE USED**. + @Deprecated('This is a temporary migration flag. DO NOT USE THIS.') // flutter_ignore: deprecation_syntax (see analyze.dart) + static bool applyDoubleRebuildFix = false; + @override RenderObject createRenderObject(BuildContext context) => _RenderLayoutBuilder(); } diff --git a/packages/flutter/test/widgets/framework_test.dart b/packages/flutter/test/widgets/framework_test.dart index f3a4014bdf1c..2313e352c85e 100644 --- a/packages/flutter/test/widgets/framework_test.dart +++ b/packages/flutter/test/widgets/framework_test.dart @@ -23,6 +23,9 @@ class _MyGlobalObjectKey> extends GlobalObjectKe } void main() { + setUp(() { LayoutBuilder.applyDoubleRebuildFix = true; }); + tearDown(() { LayoutBuilder.applyDoubleRebuildFix = false; }); + testWidgets('UniqueKey control test', (WidgetTester tester) async { final Key key = UniqueKey(); expect(key, hasOneLineDescription); @@ -137,8 +140,8 @@ void main() { }); testWidgets('GlobalKey correct case 3 - can deal with early rebuild in layoutbuilder - move backward', (WidgetTester tester) async { - const Key key1 = GlobalObjectKey('Text1'); - const Key key2 = GlobalObjectKey('Text2'); + final Key key1 = GlobalKey(debugLabel: 'Text1'); + final Key key2 = GlobalKey(debugLabel: 'Text2'); Key? rebuiltKeyOfSecondChildBeforeLayout; Key? rebuiltKeyOfFirstChildAfterLayout; Key? rebuiltKeyOfSecondChildAfterLayout; @@ -147,7 +150,7 @@ void main() { builder: (BuildContext context, BoxConstraints constraints) { return Column( children: [ - const _Stateful( + _Stateful( child: Text( 'Text1', textDirection: TextDirection.ltr, @@ -155,7 +158,7 @@ void main() { ), ), _Stateful( - child: const Text( + child: Text( 'Text2', textDirection: TextDirection.ltr, key: key2, @@ -186,14 +189,14 @@ void main() { return Column( children: [ _Stateful( - child: const Text( + child: Text( 'Text2', textDirection: TextDirection.ltr, key: key2, ), onElementRebuild: (StatefulElement element) { - // Verifies the early rebuild happens before layout. - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + // The widget is only built once. + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); // We don't want noise to override the result; expect(rebuiltKeyOfFirstChildAfterLayout, isNull); final _Stateful statefulWidget = element.widget as _Stateful; @@ -201,14 +204,14 @@ void main() { }, ), _Stateful( - child: const Text( + child: Text( 'Text1', textDirection: TextDirection.ltr, key: key1, ), onElementRebuild: (StatefulElement element) { - // Verifies the early rebuild happens before layout. - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + // The widget is only built once. + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); // We don't want noise to override the result; expect(rebuiltKeyOfSecondChildAfterLayout, isNull); final _Stateful statefulWidget = element.widget as _Stateful; @@ -220,7 +223,7 @@ void main() { }, ), ); - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); expect(rebuiltKeyOfFirstChildAfterLayout, key2); expect(rebuiltKeyOfSecondChildAfterLayout, key1); }); @@ -295,8 +298,8 @@ void main() { key: key3, ), onElementRebuild: (StatefulElement element) { - // Verifies the early rebuild happens before layout. - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + // The widget is only built once. + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); // We don't want noise to override the result; expect(rebuiltKeyOfSecondChildAfterLayout, isNull); final _Stateful statefulWidget = element.widget as _Stateful; @@ -310,8 +313,8 @@ void main() { key: key2, ), onElementRebuild: (StatefulElement element) { - // Verifies the early rebuild happens before layout. - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + // The widget is only built once. + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); // We don't want noise to override the result; expect(rebuiltKeyOfThirdChildAfterLayout, isNull); final _Stateful statefulWidget = element.widget as _Stateful; @@ -323,7 +326,7 @@ void main() { }, ), ); - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); expect(rebuiltKeyOfSecondChildAfterLayout, key3); expect(rebuiltKeyOfThirdChildAfterLayout, key2); }); @@ -391,8 +394,8 @@ void main() { textDirection: TextDirection.ltr, ), onElementRebuild: (StatefulElement element) { - // Verifies the early rebuild happens before layout. - expect(rebuiltKeyOfSecondChildBeforeLayout, key1); + // The widget is only built once. + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); }, ), _Stateful( @@ -402,8 +405,8 @@ void main() { key: key1, ), onElementRebuild: (StatefulElement element) { - // Verifies the early rebuild happens before layout. - expect(rebuiltKeyOfSecondChildBeforeLayout, key1); + // The widget is only built once. + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); // We don't want noise to override the result; expect(rebuiltKeyOfThirdChildAfterLayout, isNull); final _Stateful statefulWidget = element.widget as _Stateful; @@ -415,7 +418,7 @@ void main() { }, ), ); - expect(rebuiltKeyOfSecondChildBeforeLayout, key1); + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); expect(rebuiltKeyOfThirdChildAfterLayout, key1); }); @@ -990,8 +993,8 @@ void main() { key: key2, ), onElementRebuild: (StatefulElement element) { - // Verifies the early rebuild happens before layout. - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + // The widget is only rebuilt once. + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); // We don't want noise to override the result; expect(rebuiltKeyOfFirstChildAfterLayout, isNull); final _Stateful statefulWidget = element.widget as _Stateful; @@ -1005,8 +1008,8 @@ void main() { key: key2, ), onElementRebuild: (StatefulElement element) { - // Verifies the early rebuild happens before layout. - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + // The widget is only rebuilt once. + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); // We don't want noise to override the result; expect(rebuiltKeyOfSecondChildAfterLayout, isNull); final _Stateful statefulWidget = element.widget as _Stateful; @@ -1018,7 +1021,7 @@ void main() { }, ), ); - expect(rebuiltKeyOfSecondChildBeforeLayout, key2); + expect(rebuiltKeyOfSecondChildBeforeLayout, isNull); expect(rebuiltKeyOfFirstChildAfterLayout, key2); expect(rebuiltKeyOfSecondChildAfterLayout, key2); final dynamic exception = tester.takeException(); @@ -1291,7 +1294,7 @@ void main() { late FlutterError error; try { tester.binding.buildOwner!.scheduleBuildFor( - DirtyElementWithCustomBuildOwner(tester.binding.buildOwner!, Container()), + DirtyElementWithCustomBuildOwner(tester.binding.buildOwner!, Container()) ); } on FlutterError catch (e) { error = e; @@ -1852,6 +1855,95 @@ The findRenderObject() method was called for the following element: )); expect(tester.takeException(), isNull); }); + + testWidgets('BuildScope segregates dirty elements', (WidgetTester tester) async { + final BuildScope buildScope = BuildScope(); + await tester.pumpWidget( + StatefulBuilder( + builder: (BuildContext context, StateSetter stateSetter) { + return _CustomBuildScopeWidget( + buildScope: buildScope, + child: const _NullLeaf(), + ); + }, + ), + ); + final Element rootElement = tester.element(find.byType(StatefulBuilder)); + final Element scopeElement = tester.element(find.byType(_CustomBuildScopeWidget)); + final Element leafElement = tester.element(find.byType(_NullLeaf)); + expect(rootElement.dirty, isFalse); + expect(scopeElement.dirty, isFalse); + expect(leafElement.dirty, isFalse); + + rootElement.markNeedsBuild(); + await tester.pump(); + expect(rootElement.dirty, isFalse); + expect(scopeElement.dirty, isFalse); + expect(leafElement.dirty, isFalse); + + scopeElement.markNeedsBuild(); + await tester.pump(); + expect(rootElement.dirty, isFalse); + expect(scopeElement.dirty, isTrue); + expect(leafElement.dirty, isFalse); + + scopeElement.owner!.buildScope(scopeElement); + await tester.pump(); + expect(rootElement.dirty, isFalse); + expect(scopeElement.dirty, isFalse); + expect(leafElement.dirty, isFalse); + + leafElement.markNeedsBuild(); + await tester.pump(); + expect(rootElement.dirty, isFalse); + expect(scopeElement.dirty, isFalse); + expect(leafElement.dirty, isTrue); + + scopeElement.owner!.buildScope(scopeElement); + await tester.pump(); + expect(rootElement.dirty, isFalse); + expect(scopeElement.dirty, isFalse); + expect(leafElement.dirty, isFalse); + }); + + testWidgets('reparenting Element to another BuildScope', (WidgetTester tester) async { + final BuildScope buildScope = BuildScope(); + final GlobalKey key = GlobalKey(debugLabel: 'key'); + await tester.pumpWidget( + _DummyMultiChildWidget( + [ + _CustomBuildScopeWidget( // This widget does not call updateChild when rebuild. + buildScope: buildScope, + child: const _NullLeaf(), + ), + _NullLeaf(key: key), + ], + ), + ); + final Element scopeElement = tester.element(find.byType(_CustomBuildScopeWidget)); + final Element keyedWidget = tester.element(find.byKey(key)); + + expect(scopeElement.dirty, isFalse); + expect(keyedWidget.dirty, isFalse); + + // Mark the Element with a GlobalKey dirty and reparent it to another BuildScope. + // the Element should not rebuild. + keyedWidget.markNeedsBuild(); + await tester.pumpWidget( + _DummyMultiChildWidget( + [ + _CustomBuildScopeWidget( // This widget does not call updateChild when rebuild. + buildScope: buildScope, + child: _NullLeaf(key: key), + ), + const _NullLeaf(), + ], + ), + ); + + expect(scopeElement.dirty, isFalse); + expect(keyedWidget.dirty, isTrue); + }); } class _TestInheritedElement extends InheritedElement { @@ -1951,14 +2043,10 @@ class _DecorateState extends State { } } -class DirtyElementWithCustomBuildOwner extends Element { - DirtyElementWithCustomBuildOwner(BuildOwner buildOwner, super.widget) - : _owner = buildOwner; - - final BuildOwner _owner; - - @override - BuildOwner get owner => _owner; +class DirtyElementWithCustomBuildOwner extends Element with RootElementMixin { + DirtyElementWithCustomBuildOwner(BuildOwner buildOwner, super.widget) { + assignOwner(buildOwner); + } @override bool get dirty => true; @@ -2082,9 +2170,9 @@ class StatefulElementSpy extends StatefulElement { _Stateful get _statefulWidget => widget as _Stateful; @override - void rebuild({bool force = false}) { + void performRebuild() { _statefulWidget.onElementRebuild?.call(this); - super.rebuild(force: force); + super.performRebuild(); } } @@ -2288,3 +2376,116 @@ class _RenderTestLeaderLayerWidget extends RenderProxyBox { } } } + +// This widget does not call updateChild when it rebuilds. +class _CustomBuildScopeWidget extends ProxyWidget { + const _CustomBuildScopeWidget({this.buildScope, required super.child}); + + final BuildScope? buildScope; + @override + Element createElement() => _CustomBuildScopeElement(this); +} + +class _CustomBuildScopeElement extends Element { + _CustomBuildScopeElement(super.widget); + + @override + BuildScope get buildScope => (widget as _CustomBuildScopeWidget).buildScope ?? super.buildScope; + @override + bool get debugDoingBuild => throw UnimplementedError(); + + Element? _child; + + @override + void mount(Element? parent, Object? newSlot) { + super.mount(parent, newSlot); + rebuild(force: true); + // Only does tree walk on mount. + _child = updateChild(_child, (widget as _CustomBuildScopeWidget).child, slot); + } + + @override + void visitChildren(ElementVisitor visitor) { + final Element? child = _child; + if (child != null) { + visitor(child); + } + } +} + +class _DummyMultiChildWidget extends Widget { + const _DummyMultiChildWidget(this.children); + + final List children; + @override + Element createElement() => _DummyMuitiChildElement(this); +} + +class _DummyMuitiChildElement extends Element { + _DummyMuitiChildElement(super.widget); + + @override + bool get debugDoingBuild => throw UnimplementedError(); + + late List _children; + final Set _forgottenChildren = {}; + + @override + void mount(Element? parent, Object? newSlot) { + super.mount(parent, newSlot); + final List childWidgets = (widget as _DummyMultiChildWidget).children; + + Element? previousChild; + final List children = List.generate(childWidgets.length, (int i) { + final Element child = previousChild = inflateWidget(childWidgets[i], IndexedSlot(i, previousChild)); + return child; + }); + _children = children; + } + + @override + void update(_DummyMultiChildWidget newWidget) { + super.update(newWidget); + _children = updateChildren(_children, newWidget.children, forgottenChildren: _forgottenChildren); + _forgottenChildren.clear(); + } + + @override + Element? get renderObjectAttachingChild => null; + + @override + void visitChildren(ElementVisitor visitor) { + for (final Element child in _children) { + if (!_forgottenChildren.contains(child)) { + visitor(child); + } + } + } + + @override + void forgetChild(Element child) { + assert(_children.contains(child)); + assert(!_forgottenChildren.contains(child)); + _forgottenChildren.add(child); + super.forgetChild(child); + } +} + +class _NullLeaf extends Widget { + const _NullLeaf({super.key}); + @override + Element createElement() => _NullElement(this); +} + +class _NullElement extends Element { + _NullElement(super.widget); + + @override + void mount(Element? parent, Object? newSlot) { + super.mount(parent, newSlot); + rebuild(force: true); + } + + @override + bool get debugDoingBuild => throw UnimplementedError(); +} diff --git a/packages/flutter/test/widgets/layout_builder_and_global_keys_test.dart b/packages/flutter/test/widgets/layout_builder_and_global_keys_test.dart index 68e6f4ee27dd..4082255f5bd1 100644 --- a/packages/flutter/test/widgets/layout_builder_and_global_keys_test.dart +++ b/packages/flutter/test/widgets/layout_builder_and_global_keys_test.dart @@ -41,6 +41,9 @@ class StatefulWrapperState extends State { } void main() { + setUp(() { LayoutBuilder.applyDoubleRebuildFix = true; }); + tearDown(() { LayoutBuilder.applyDoubleRebuildFix = false; }); + testWidgets('Moving global key inside a LayoutBuilder', (WidgetTester tester) async { final GlobalKey key = GlobalKey(); await tester.pumpWidget( @@ -60,6 +63,45 @@ void main() { expect(tester.takeException(), null); }); + testWidgets('Moving GlobalKeys out of LayoutBuilder', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/146379. + final GlobalKey widgetKey = GlobalKey(debugLabel: 'widget key'); + final Widget widgetWithKey = Builder(builder: (BuildContext context) { + Directionality.of(context); + return SizedBox(key: widgetKey); + }); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Row( + children: [ + LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) => widgetWithKey, + ), + ], + ), + ), + ); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.rtl, + child: Row( + children: [ + LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) => const Placeholder(), + ), + widgetWithKey, + ], + ), + ), + ); + + expect(tester.takeException(), null); + expect(find.byKey(widgetKey), findsOneWidget); + }); + testWidgets('Moving global key inside a SliverLayoutBuilder', (WidgetTester tester) async { final GlobalKey key = GlobalKey(); diff --git a/packages/flutter/test/widgets/layout_builder_and_state_test.dart b/packages/flutter/test/widgets/layout_builder_and_state_test.dart index f5687492e714..a9e6fb419939 100644 --- a/packages/flutter/test/widgets/layout_builder_and_state_test.dart +++ b/packages/flutter/test/widgets/layout_builder_and_state_test.dart @@ -50,7 +50,6 @@ class Wrapper extends StatelessWidget { void main() { testWidgets('Calling setState on a widget that moves into a LayoutBuilder in the same frame', (WidgetTester tester) async { - StatefulWrapperState statefulWrapper; final Widget inner = Wrapper( child: StatefulWrapper( key: GlobalKey(), @@ -63,7 +62,7 @@ void main() { }), right: inner, )); - statefulWrapper = tester.state(find.byType(StatefulWrapper)); + final StatefulWrapperState statefulWrapper = tester.state(find.byType(StatefulWrapper)); expect(statefulWrapper.built, true); statefulWrapper.built = false; diff --git a/packages/flutter/test/widgets/layout_builder_test.dart b/packages/flutter/test/widgets/layout_builder_test.dart index bdadc5d82056..216957d2ed87 100644 --- a/packages/flutter/test/widgets/layout_builder_test.dart +++ b/packages/flutter/test/widgets/layout_builder_test.dart @@ -7,6 +7,9 @@ import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; void main() { + setUp(() { LayoutBuilder.applyDoubleRebuildFix = true; }); + tearDown(() { LayoutBuilder.applyDoubleRebuildFix = false; }); + testWidgets('LayoutBuilder parent size', (WidgetTester tester) async { late Size layoutBuilderSize; final Key childKey = UniqueKey(); @@ -294,6 +297,84 @@ void main() { expect(built, 2); }); + testWidgets('LayoutBuilder rebuilds once in the same frame', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/146379. + int built = 0; + final Widget target = LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) { + return Builder(builder: (BuildContext context) { + built += 1; + MediaQuery.of(context); + return const Placeholder(); + }); + }, + ); + expect(built, 0); + + await tester.pumpWidget(MediaQuery( + data: const MediaQueryData(size: Size(400.0, 300.0)), + child: Center( + child: SizedBox( + width: 400.0, + child: target, + ), + ), + )); + expect(built, 1); + + await tester.pumpWidget(MediaQuery( + data: const MediaQueryData(size: Size(300.0, 400.0)), + child: Center( + child: SizedBox( + width: 300.0, + child: target, + ), + ), + )); + expect(built, 2); + }); + + testWidgets('LayoutBuilder can change size without rebuild', (WidgetTester tester) async { + int built = 0; + final Widget target = LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) { + return Builder(builder: (BuildContext context) { + built += 1; + return const Text('A'); + }); + }, + ); + expect(built, 0); + + await tester.pumpWidget( + Center( + child: Directionality( + textDirection: TextDirection.ltr, + child: DefaultTextStyle( + style: const TextStyle(fontSize: 10), + child: target, + ), + ), + ) + ); + expect(built, 1); + expect(tester.getSize(find.byWidget(target)), const Size(10, 10)); + + await tester.pumpWidget( + Center( + child: Directionality( + textDirection: TextDirection.ltr, + child: DefaultTextStyle( + style: const TextStyle(fontSize: 100), + child: target, + ), + ), + ) + ); + expect(built, 1); + expect(tester.getSize(find.byWidget(target)), const Size(100, 100)); + }); + testWidgets('SliverLayoutBuilder and Inherited -- do not rebuild when not using inherited', (WidgetTester tester) async { int built = 0; final Widget target = Directionality( diff --git a/packages/flutter/test/widgets/overlay_portal_test.dart b/packages/flutter/test/widgets/overlay_portal_test.dart index 7b6d919a4318..436bbf03687e 100644 --- a/packages/flutter/test/widgets/overlay_portal_test.dart +++ b/packages/flutter/test/widgets/overlay_portal_test.dart @@ -29,7 +29,7 @@ class _ManyRelayoutBoundaries extends StatelessWidget { } } -void rebuildLayoutBuilderSubtree(RenderBox descendant) { +void rebuildLayoutBuilderSubtree(RenderBox descendant, WidgetTester tester) { assert(descendant is! RenderConstrainedLayoutBuilder); RenderObject? node = descendant.parent; @@ -37,7 +37,10 @@ void rebuildLayoutBuilderSubtree(RenderBox descendant) { if (node is! RenderConstrainedLayoutBuilder) { node = node.parent; } else { - node.markNeedsBuild(); + final Element layoutBuilderElement = tester.element(find.byElementPredicate( + (Element element) => element.widget is LayoutBuilder && element.renderObject == node, + )); + layoutBuilderElement.markNeedsBuild(); return; } } @@ -711,7 +714,7 @@ void main() { renderChild1.markNeedsLayout(); // Dirty both render subtree branches. childBox.markNeedsLayout(); - rebuildLayoutBuilderSubtree(overlayChildBox); + rebuildLayoutBuilderSubtree(overlayChildBox, tester); // Make sure childBox's depth is greater than that of the overlay // child, and childBox's parent isn't dirty (childBox is a dirty relayout @@ -1110,7 +1113,7 @@ void main() { widgetKey.currentContext!.findRenderObject()!.markNeedsLayout(); childBox.markNeedsLayout(); - rebuildLayoutBuilderSubtree(overlayChildBox); + rebuildLayoutBuilderSubtree(overlayChildBox, tester); // Make sure childBox's depth is greater than that of the overlay child. expect( widgetKey.currentContext!.findRenderObject()!.depth, @@ -1190,7 +1193,7 @@ void main() { targetGlobalKey.currentContext!.findRenderObject()!.markNeedsLayout(); childBox.markNeedsLayout(); - rebuildLayoutBuilderSubtree(overlayChildBox); + rebuildLayoutBuilderSubtree(overlayChildBox, tester); setState1(() {}); setState2(() {}); targetMovedToOverlayEntry3 = true;