Skip to content

Commit 09a5382

Browse files
LongCatIsLooongLongCatIsLooong
authored
Add error messages to _debugCanPerformMutations (#105638)
1 parent c4aaa39 commit 09a5382

File tree

2 files changed

+235
-21
lines changed

2 files changed

+235
-21
lines changed

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

Lines changed: 101 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,34 +1579,115 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
15791579
late bool result;
15801580
assert(() {
15811581
if (_debugDisposed) {
1582-
result = false;
1583-
return true;
1582+
throw FlutterError.fromParts(<DiagnosticsNode>[
1583+
ErrorSummary('A disposed RenderObject was mutated.'),
1584+
DiagnosticsProperty<RenderObject>(
1585+
'The disposed RenderObject was',
1586+
this,
1587+
style: DiagnosticsTreeStyle.errorProperty,
1588+
),
1589+
]);
15841590
}
1585-
if (owner != null && !owner!.debugDoingLayout) {
1591+
1592+
final PipelineOwner? owner = this.owner;
1593+
// Detached nodes are allowed to mutate and the "can perform mutations"
1594+
// check will be performed when they re-attach. This assert is only useful
1595+
// during layout.
1596+
if (owner == null || !owner.debugDoingLayout) {
15861597
result = true;
15871598
return true;
15881599
}
1589-
RenderObject node = this;
1590-
while (true) {
1591-
if (node._doingThisLayoutWithCallback) {
1592-
result = true;
1593-
break;
1594-
}
1595-
if (owner != null && owner!._debugAllowMutationsToDirtySubtrees && node._needsLayout) {
1600+
1601+
RenderObject? activeLayoutRoot = this;
1602+
while (activeLayoutRoot != null) {
1603+
final bool mutationsToDirtySubtreesAllowed = activeLayoutRoot.owner?._debugAllowMutationsToDirtySubtrees ?? false;
1604+
final bool doingLayoutWithCallback = activeLayoutRoot._doingThisLayoutWithCallback;
1605+
// Mutations on this subtree is allowed when:
1606+
// - the subtree is being mutated in a layout callback.
1607+
// - a different part of the render tree is doing a layout callback,
1608+
// and this subtree is being reparented to that subtree, as a result
1609+
// of global key reparenting.
1610+
if (doingLayoutWithCallback || mutationsToDirtySubtreesAllowed && activeLayoutRoot._needsLayout) {
15961611
result = true;
1597-
break;
1598-
}
1599-
if (node._debugMutationsLocked) {
1600-
result = false;
1601-
break;
1612+
return true;
16021613
}
1603-
if (node.parent is! RenderObject) {
1604-
result = true;
1614+
1615+
if (!activeLayoutRoot._debugMutationsLocked) {
1616+
final AbstractNode? p = activeLayoutRoot.parent;
1617+
activeLayoutRoot = p is RenderObject ? p : null;
1618+
} else {
1619+
// activeLayoutRoot found.
16051620
break;
16061621
}
1607-
node = node.parent! as RenderObject;
16081622
}
1609-
return true;
1623+
1624+
final RenderObject debugActiveLayout = RenderObject.debugActiveLayout!;
1625+
final String culpritMethodName = debugActiveLayout.debugDoingThisLayout ? 'performLayout' : 'performResize';
1626+
final String culpritFullMethodName = '${debugActiveLayout.runtimeType}.$culpritMethodName';
1627+
result = false;
1628+
1629+
if (activeLayoutRoot == null) {
1630+
throw FlutterError.fromParts(<DiagnosticsNode>[
1631+
ErrorSummary('A $runtimeType was mutated in $culpritFullMethodName.'),
1632+
ErrorDescription(
1633+
'The RenderObject was mutated when none of its ancestors is actively performing layout.',
1634+
),
1635+
DiagnosticsProperty<RenderObject>(
1636+
'The RenderObject being mutated was',
1637+
this,
1638+
style: DiagnosticsTreeStyle.errorProperty,
1639+
),
1640+
DiagnosticsProperty<RenderObject>(
1641+
'The RenderObject that was mutating the said $runtimeType was',
1642+
debugActiveLayout,
1643+
style: DiagnosticsTreeStyle.errorProperty,
1644+
),
1645+
]);
1646+
}
1647+
1648+
if (activeLayoutRoot == this) {
1649+
throw FlutterError.fromParts(<DiagnosticsNode>[
1650+
ErrorSummary('A $runtimeType was mutated in its own $culpritMethodName implementation.'),
1651+
ErrorDescription('A RenderObject must not re-dirty itself while still being laid out.'),
1652+
DiagnosticsProperty<RenderObject>(
1653+
'The RenderObject being mutated was',
1654+
this,
1655+
style: DiagnosticsTreeStyle.errorProperty,
1656+
),
1657+
ErrorHint('Consider using the LayoutBuilder widget to dynamically change a subtree during layout.'),
1658+
]);
1659+
}
1660+
1661+
final ErrorSummary summary = ErrorSummary('A $runtimeType was mutated in $culpritFullMethodName.');
1662+
final bool isMutatedByAncestor = activeLayoutRoot == debugActiveLayout;
1663+
final String description = isMutatedByAncestor
1664+
? 'A RenderObject must not mutate its descendants in its $culpritMethodName method.'
1665+
: 'A RenderObject must not mutate another RenderObject from a different render subtree '
1666+
'in its $culpritMethodName method.';
1667+
1668+
throw FlutterError.fromParts(<DiagnosticsNode>[
1669+
summary,
1670+
ErrorDescription(description),
1671+
DiagnosticsProperty<RenderObject>(
1672+
'The RenderObject being mutated was',
1673+
this,
1674+
style: DiagnosticsTreeStyle.errorProperty,
1675+
),
1676+
DiagnosticsProperty<RenderObject>(
1677+
'The ${isMutatedByAncestor ? 'ancestor ' : ''}RenderObject that was mutating the said $runtimeType was',
1678+
debugActiveLayout,
1679+
style: DiagnosticsTreeStyle.errorProperty,
1680+
),
1681+
if (!isMutatedByAncestor) DiagnosticsProperty<RenderObject>(
1682+
'Their common ancestor was',
1683+
activeLayoutRoot,
1684+
style: DiagnosticsTreeStyle.errorProperty,
1685+
),
1686+
ErrorHint(
1687+
'Mutating the layout of another RenderObject may cause some RenderObjects in its subtree to be laid out more than once. '
1688+
'Consider using the LayoutBuilder widget to dynamically mutate a subtree during layout.'
1689+
),
1690+
]);
16101691
}());
16111692
return result;
16121693
}
@@ -1799,6 +1880,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
17991880
/// Only call this if [parent] is not null.
18001881
@protected
18011882
void markParentNeedsLayout() {
1883+
assert(_debugCanPerformMutations);
18021884
_needsLayout = true;
18031885
assert(this.parent != null);
18041886
final RenderObject parent = this.parent! as RenderObject;

‎packages/flutter/test/rendering/mutations_test.dart

Lines changed: 134 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ import 'package:flutter_test/flutter_test.dart';
88
import 'rendering_tester.dart';
99

1010
class RenderLayoutTestBox extends RenderProxyBox {
11-
RenderLayoutTestBox(this.onLayout);
11+
RenderLayoutTestBox(this.onLayout, {
12+
this.onPerformLayout,
13+
});
1214

1315
final VoidCallback onLayout;
16+
final VoidCallback? onPerformLayout;
1417

1518
@override
1619
void layout(Constraints constraints, { bool parentUsesSize = false }) {
@@ -27,7 +30,10 @@ class RenderLayoutTestBox extends RenderProxyBox {
2730
bool get sizedByParent => true;
2831

2932
@override
30-
void performLayout() { }
33+
void performLayout() {
34+
child?.layout(constraints, parentUsesSize: true);
35+
onPerformLayout?.call();
36+
}
3137
}
3238

3339
void main() {
@@ -72,4 +78,130 @@ void main() {
7278
expect(movedChild1, isFalse);
7379
expect(movedChild2, isFalse);
7480
});
81+
82+
group('Throws when illegal mutations are attempted: ', () {
83+
FlutterError catchLayoutError(RenderBox box) {
84+
Object? error;
85+
layout(box, onErrors: () {
86+
error = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails()!.exception;
87+
});
88+
expect(error, isFlutterError);
89+
return error! as FlutterError;
90+
}
91+
92+
test('on disposed render objects', () {
93+
final RenderBox box = RenderLayoutTestBox(() {});
94+
box.dispose();
95+
96+
Object? error;
97+
try {
98+
box.markNeedsLayout();
99+
} catch (e) {
100+
error = e;
101+
}
102+
103+
expect(error, isFlutterError);
104+
expect(
105+
(error! as FlutterError).message,
106+
equalsIgnoringWhitespace(
107+
'A disposed RenderObject was mutated.\n'
108+
'The disposed RenderObject was:\n'
109+
'${box.toStringShort()}'
110+
)
111+
);
112+
});
113+
114+
test('marking itself dirty in performLayout', () {
115+
late RenderBox child1;
116+
final RenderFlex block = RenderFlex(textDirection: TextDirection.ltr);
117+
block.add(child1 = RenderLayoutTestBox(() {}, onPerformLayout: () { child1.markNeedsLayout(); }));
118+
119+
expect(
120+
catchLayoutError(block).message,
121+
equalsIgnoringWhitespace(
122+
'A RenderLayoutTestBox was mutated in its own performLayout implementation.\n'
123+
'A RenderObject must not re-dirty itself while still being laid out.\n'
124+
'The RenderObject being mutated was:\n'
125+
'${child1.toStringShort()}\n'
126+
'Consider using the LayoutBuilder widget to dynamically change a subtree during layout.'
127+
)
128+
);
129+
});
130+
131+
test('marking a sibling dirty in performLayout', () {
132+
late RenderBox child1, child2;
133+
final RenderFlex block = RenderFlex(textDirection: TextDirection.ltr);
134+
block.add(child1 = RenderLayoutTestBox(() {}));
135+
block.add(child2 = RenderLayoutTestBox(() {}, onPerformLayout: () { child1.markNeedsLayout(); }));
136+
137+
expect(
138+
catchLayoutError(block).message,
139+
equalsIgnoringWhitespace(
140+
'A RenderLayoutTestBox was mutated in RenderLayoutTestBox.performLayout.\n'
141+
'A RenderObject must not mutate another RenderObject from a different render subtree in its performLayout method.\n'
142+
'The RenderObject being mutated was:\n'
143+
'${child1.toStringShort()}\n'
144+
'The RenderObject that was mutating the said RenderLayoutTestBox was:\n'
145+
'${child2.toStringShort()}\n'
146+
'Their common ancestor was:\n'
147+
'${block.toStringShort()}\n'
148+
'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.'
149+
)
150+
);
151+
});
152+
153+
test('marking a descendant dirty in performLayout', () {
154+
late RenderBox child1;
155+
final RenderFlex block = RenderFlex(textDirection: TextDirection.ltr);
156+
block.add(child1 = RenderLayoutTestBox(() {}));
157+
block.add(RenderLayoutTestBox(child1.markNeedsLayout));
158+
159+
expect(
160+
catchLayoutError(block).message,
161+
equalsIgnoringWhitespace(
162+
'A RenderLayoutTestBox was mutated in RenderFlex.performLayout.\n'
163+
'A RenderObject must not mutate its descendants in its performLayout method.\n'
164+
'The RenderObject being mutated was:\n'
165+
'${child1.toStringShort()}\n'
166+
'The ancestor RenderObject that was mutating the said RenderLayoutTestBox was:\n'
167+
'${block.toStringShort()}\n'
168+
'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.'
169+
),
170+
);
171+
});
172+
173+
test('marking an out-of-band mutation in performLayout', () {
174+
late RenderProxyBox child1, child11, child2, child21;
175+
final RenderFlex block = RenderFlex(textDirection: TextDirection.ltr);
176+
block.add(child1 = RenderLayoutTestBox(() {}));
177+
block.add(child2 = RenderLayoutTestBox(() {}));
178+
child1.child = child11 = RenderLayoutTestBox(() {});
179+
layout(block);
180+
181+
expect(block.debugNeedsLayout, false);
182+
expect(child1.debugNeedsLayout, false);
183+
expect(child11.debugNeedsLayout, false);
184+
expect(child2.debugNeedsLayout, false);
185+
186+
// Add a new child to child2 which is a relayout boundary.
187+
child2.child = child21 = RenderLayoutTestBox(() {}, onPerformLayout: child11.markNeedsLayout);
188+
189+
FlutterError? error;
190+
pumpFrame(onErrors: () {
191+
error = TestRenderingFlutterBinding.instance.takeFlutterErrorDetails()!.exception as FlutterError;
192+
});
193+
194+
expect(
195+
error?.message,
196+
equalsIgnoringWhitespace(
197+
'A RenderLayoutTestBox was mutated in RenderLayoutTestBox.performLayout.\n'
198+
'The RenderObject was mutated when none of its ancestors is actively performing layout.\n'
199+
'The RenderObject being mutated was:\n'
200+
'${child11.toStringShort()}\n'
201+
'The RenderObject that was mutating the said RenderLayoutTestBox was:\n'
202+
'${child21.toStringShort()}'
203+
),
204+
);
205+
});
206+
});
75207
}

0 commit comments

Comments
 (0)