Add error messages to `_debugCanPerformMutations` (#105638)
diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart
index 3eb9b2b..fd32cbf 100644
--- a/packages/flutter/lib/src/rendering/object.dart
+++ b/packages/flutter/lib/src/rendering/object.dart
@@ -1579,34 +1579,115 @@
late bool result;
assert(() {
if (_debugDisposed) {
- result = false;
- return true;
+ throw FlutterError.fromParts(<DiagnosticsNode>[
+ ErrorSummary('A disposed RenderObject was mutated.'),
+ DiagnosticsProperty<RenderObject>(
+ 'The disposed RenderObject was',
+ this,
+ style: DiagnosticsTreeStyle.errorProperty,
+ ),
+ ]);
}
- if (owner != null && !owner!.debugDoingLayout) {
+
+ final PipelineOwner? owner = this.owner;
+ // Detached nodes are allowed to mutate and the "can perform mutations"
+ // check will be performed when they re-attach. This assert is only useful
+ // during layout.
+ if (owner == null || !owner.debugDoingLayout) {
result = true;
return true;
}
- RenderObject node = this;
- while (true) {
- if (node._doingThisLayoutWithCallback) {
+
+ RenderObject? activeLayoutRoot = this;
+ while (activeLayoutRoot != null) {
+ final bool mutationsToDirtySubtreesAllowed = activeLayoutRoot.owner?._debugAllowMutationsToDirtySubtrees ?? false;
+ final bool doingLayoutWithCallback = activeLayoutRoot._doingThisLayoutWithCallback;
+ // Mutations on this subtree is allowed when:
+ // - the subtree is being mutated in 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
+ // of global key reparenting.
+ if (doingLayoutWithCallback || mutationsToDirtySubtreesAllowed && activeLayoutRoot._needsLayout) {
result = true;
+ return true;
+ }
+
+ if (!activeLayoutRoot._debugMutationsLocked) {
+ final AbstractNode? p = activeLayoutRoot.parent;
+ activeLayoutRoot = p is RenderObject ? p : null;
+ } else {
+ // activeLayoutRoot found.
break;
}
- if (owner != null && owner!._debugAllowMutationsToDirtySubtrees && node._needsLayout) {
- result = true;
- break;
- }
- if (node._debugMutationsLocked) {
- result = false;
- break;
- }
- if (node.parent is! RenderObject) {
- result = true;
- break;
- }
- node = node.parent! as RenderObject;
}
- return true;
+
+ final RenderObject debugActiveLayout = RenderObject.debugActiveLayout!;
+ final String culpritMethodName = debugActiveLayout.debugDoingThisLayout ? 'performLayout' : 'performResize';
+ final String culpritFullMethodName = '${debugActiveLayout.runtimeType}.$culpritMethodName';
+ result = false;
+
+ if (activeLayoutRoot == null) {
+ throw FlutterError.fromParts(<DiagnosticsNode>[
+ ErrorSummary('A $runtimeType was mutated in $culpritFullMethodName.'),
+ ErrorDescription(
+ 'The RenderObject was mutated when none of its ancestors is actively performing layout.',
+ ),
+ DiagnosticsProperty<RenderObject>(
+ 'The RenderObject being mutated was',
+ this,
+ style: DiagnosticsTreeStyle.errorProperty,
+ ),
+ DiagnosticsProperty<RenderObject>(
+ 'The RenderObject that was mutating the said $runtimeType was',
+ debugActiveLayout,
+ style: DiagnosticsTreeStyle.errorProperty,
+ ),
+ ]);
+ }
+
+ if (activeLayoutRoot == this) {
+ throw FlutterError.fromParts(<DiagnosticsNode>[
+ ErrorSummary('A $runtimeType was mutated in its own $culpritMethodName implementation.'),
+ ErrorDescription('A RenderObject must not re-dirty itself while still being laid out.'),
+ DiagnosticsProperty<RenderObject>(
+ 'The RenderObject being mutated was',
+ this,
+ style: DiagnosticsTreeStyle.errorProperty,
+ ),
+ ErrorHint('Consider using the LayoutBuilder widget to dynamically change a subtree during layout.'),
+ ]);
+ }
+
+ final ErrorSummary summary = ErrorSummary('A $runtimeType was mutated in $culpritFullMethodName.');
+ final bool isMutatedByAncestor = activeLayoutRoot == debugActiveLayout;
+ final String description = isMutatedByAncestor
+ ? 'A RenderObject must not mutate its descendants in its $culpritMethodName method.'
+ : 'A RenderObject must not mutate another RenderObject from a different render subtree '
+ 'in its $culpritMethodName method.';
+
+ throw FlutterError.fromParts(<DiagnosticsNode>[
+ summary,
+ ErrorDescription(description),
+ DiagnosticsProperty<RenderObject>(
+ 'The RenderObject being mutated was',
+ this,
+ style: DiagnosticsTreeStyle.errorProperty,
+ ),
+ DiagnosticsProperty<RenderObject>(
+ 'The ${isMutatedByAncestor ? 'ancestor ' : ''}RenderObject that was mutating the said $runtimeType was',
+ debugActiveLayout,
+ style: DiagnosticsTreeStyle.errorProperty,
+ ),
+ if (!isMutatedByAncestor) DiagnosticsProperty<RenderObject>(
+ 'Their common ancestor was',
+ activeLayoutRoot,
+ style: DiagnosticsTreeStyle.errorProperty,
+ ),
+ ErrorHint(
+ 'Mutating the layout of another RenderObject may cause some RenderObjects in its subtree to be laid out more than once. '
+ 'Consider using the LayoutBuilder widget to dynamically mutate a subtree during layout.'
+ ),
+ ]);
}());
return result;
}
@@ -1799,6 +1880,7 @@
/// Only call this if [parent] is not null.
@protected
void markParentNeedsLayout() {
+ assert(_debugCanPerformMutations);
_needsLayout = true;
assert(this.parent != null);
final RenderObject parent = this.parent! as RenderObject;
diff --git a/packages/flutter/test/rendering/mutations_test.dart b/packages/flutter/test/rendering/mutations_test.dart
index 3e987b6..7f04768 100644
--- a/packages/flutter/test/rendering/mutations_test.dart
+++ b/packages/flutter/test/rendering/mutations_test.dart
@@ -8,9 +8,12 @@
import 'rendering_tester.dart';
class RenderLayoutTestBox extends RenderProxyBox {
- RenderLayoutTestBox(this.onLayout);
+ RenderLayoutTestBox(this.onLayout, {
+ this.onPerformLayout,
+ });
final VoidCallback onLayout;
+ final VoidCallback? onPerformLayout;
@override
void layout(Constraints constraints, { bool parentUsesSize = false }) {
@@ -27,7 +30,10 @@
bool get sizedByParent => true;
@override
- void performLayout() { }
+ void performLayout() {
+ child?.layout(constraints, parentUsesSize: true);
+ onPerformLayout?.call();
+ }
}
void main() {
@@ -72,4 +78,130 @@
expect(movedChild1, isFalse);
expect(movedChild2, isFalse);
});
+
+ group('Throws when illegal mutations are attempted: ', () {
+ FlutterError catchLayoutError(RenderBox box) {
+ Object? error;
+ layout(box, onErrors: () {
+ error = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails()!.exception;
+ });
+ expect(error, isFlutterError);
+ return error! as FlutterError;
+ }
+
+ test('on disposed render objects', () {
+ final RenderBox box = RenderLayoutTestBox(() {});
+ box.dispose();
+
+ Object? error;
+ try {
+ box.markNeedsLayout();
+ } catch (e) {
+ error = e;
+ }
+
+ expect(error, isFlutterError);
+ expect(
+ (error! as FlutterError).message,
+ equalsIgnoringWhitespace(
+ 'A disposed RenderObject was mutated.\n'
+ 'The disposed RenderObject was:\n'
+ '${box.toStringShort()}'
+ )
+ );
+ });
+
+ test('marking itself dirty in performLayout', () {
+ late RenderBox child1;
+ final RenderFlex block = RenderFlex(textDirection: TextDirection.ltr);
+ block.add(child1 = RenderLayoutTestBox(() {}, onPerformLayout: () { child1.markNeedsLayout(); }));
+
+ expect(
+ catchLayoutError(block).message,
+ equalsIgnoringWhitespace(
+ 'A RenderLayoutTestBox was mutated in its own performLayout implementation.\n'
+ 'A RenderObject must not re-dirty itself while still being laid out.\n'
+ 'The RenderObject being mutated was:\n'
+ '${child1.toStringShort()}\n'
+ 'Consider using the LayoutBuilder widget to dynamically change a subtree during layout.'
+ )
+ );
+ });
+
+ test('marking a sibling dirty in performLayout', () {
+ late RenderBox child1, child2;
+ final RenderFlex block = RenderFlex(textDirection: TextDirection.ltr);
+ block.add(child1 = RenderLayoutTestBox(() {}));
+ block.add(child2 = RenderLayoutTestBox(() {}, onPerformLayout: () { child1.markNeedsLayout(); }));
+
+ expect(
+ catchLayoutError(block).message,
+ equalsIgnoringWhitespace(
+ 'A RenderLayoutTestBox was mutated in RenderLayoutTestBox.performLayout.\n'
+ 'A RenderObject must not mutate another RenderObject from a different render subtree in its performLayout method.\n'
+ 'The RenderObject being mutated was:\n'
+ '${child1.toStringShort()}\n'
+ 'The RenderObject that was mutating the said RenderLayoutTestBox was:\n'
+ '${child2.toStringShort()}\n'
+ 'Their common ancestor was:\n'
+ '${block.toStringShort()}\n'
+ 'Mutating the layout of another RenderObject may cause some RenderObjects in its subtree to be laid out more than once. Consider using the LayoutBuilder widget to dynamically mutate a subtree during layout.'
+ )
+ );
+ });
+
+ test('marking a descendant dirty in performLayout', () {
+ late RenderBox child1;
+ final RenderFlex block = RenderFlex(textDirection: TextDirection.ltr);
+ block.add(child1 = RenderLayoutTestBox(() {}));
+ block.add(RenderLayoutTestBox(child1.markNeedsLayout));
+
+ expect(
+ catchLayoutError(block).message,
+ equalsIgnoringWhitespace(
+ 'A RenderLayoutTestBox was mutated in RenderFlex.performLayout.\n'
+ 'A RenderObject must not mutate its descendants in its performLayout method.\n'
+ 'The RenderObject being mutated was:\n'
+ '${child1.toStringShort()}\n'
+ 'The ancestor RenderObject that was mutating the said RenderLayoutTestBox was:\n'
+ '${block.toStringShort()}\n'
+ 'Mutating the layout of another RenderObject may cause some RenderObjects in its subtree to be laid out more than once. Consider using the LayoutBuilder widget to dynamically mutate a subtree during layout.'
+ ),
+ );
+ });
+
+ test('marking an out-of-band mutation in performLayout', () {
+ late RenderProxyBox child1, child11, child2, child21;
+ final RenderFlex block = RenderFlex(textDirection: TextDirection.ltr);
+ block.add(child1 = RenderLayoutTestBox(() {}));
+ block.add(child2 = RenderLayoutTestBox(() {}));
+ child1.child = child11 = RenderLayoutTestBox(() {});
+ layout(block);
+
+ expect(block.debugNeedsLayout, false);
+ expect(child1.debugNeedsLayout, false);
+ expect(child11.debugNeedsLayout, false);
+ expect(child2.debugNeedsLayout, false);
+
+ // Add a new child to child2 which is a relayout boundary.
+ child2.child = child21 = RenderLayoutTestBox(() {}, onPerformLayout: child11.markNeedsLayout);
+
+ FlutterError? error;
+ pumpFrame(onErrors: () {
+ error = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails()!.exception as FlutterError;
+ });
+
+ expect(
+ error?.message,
+ equalsIgnoringWhitespace(
+ 'A RenderLayoutTestBox was mutated in RenderLayoutTestBox.performLayout.\n'
+ 'The RenderObject was mutated when none of its ancestors is actively performing layout.\n'
+ 'The RenderObject being mutated was:\n'
+ '${child11.toStringShort()}\n'
+ 'The RenderObject that was mutating the said RenderLayoutTestBox was:\n'
+ '${child21.toStringShort()}'
+ ),
+ );
+ });
+ });
}
