Unverified Commit 67d4a831 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Allow multiple ParentDataWidgets to write to ParentData (#133581)

Fixes https://github.com/flutter/flutter/issues/133089

This allows more than one ParentDataWidget to write to the ParentData of a child render object. Previously only one was allowed. There are some rules though:
1. Only one of a given type of `ParentDataWidget` can write to the `ParentData` of a given child.
  a. For example, 2 `Positioned` widgets wrapping a child of a `Stack` would not be allowed, as only one of type `Positioned` can contribute data.

2. The type of `ParentData` **must** be compatible with all of the `ParentDataWidget`s that want to contribute data.
  a. For example, `TwoDimensionalViewportParentData` mixes in the `KeepAliveParentDataMixin`. So the `ParentData` of a given child would be compatible with the `KeepAlive` `ParentDataWidget`, as well as another `ParentDataWidget` that writes `TwoDimensionalViewportParentData` (or a subclass of `TwoDimensionalViewportParentData` - This was the motivation for this change, where a `ParentDataWidget` is being used in `TableView` with the parent data type being a subclass of `TwoDimensionalViewportParentData`.)
parent d81c8aa8
......@@ -5824,12 +5824,28 @@ class ParentDataElement<T extends ParentData> extends ProxyElement {
/// Creates an element that uses the given widget as its configuration.
ParentDataElement(ParentDataWidget<T> super.widget);
/// Returns the [Type] of [ParentData] that this element has been configured
/// for.
///
/// This is only available in debug mode. It will throw in profile and
/// release modes.
Type get debugParentDataType {
Type? type;
assert(() {
type = T;
return true;
}());
if (type != null) {
return type!;
}
throw UnsupportedError('debugParentDataType is only supported in debug builds');
}
void _applyParentData(ParentDataWidget<T> widget) {
void applyParentDataToChild(Element child) {
if (child is RenderObjectElement) {
child._updateParentData(widget);
} else {
assert(child is! ParentDataElement<ParentData>);
child.visitChildren(applyParentDataToChild);
}
}
......@@ -6278,50 +6294,124 @@ abstract class RenderObjectElement extends Element {
return ancestor as RenderObjectElement?;
}
ParentDataElement<ParentData>? _findAncestorParentDataElement() {
Element? ancestor = _parent;
ParentDataElement<ParentData>? result;
while (ancestor != null && ancestor is! RenderObjectElement) {
if (ancestor is ParentDataElement<ParentData>) {
result = ancestor;
break;
}
ancestor = ancestor._parent;
}
void _debugCheckCompetingAncestors(
List<ParentDataElement<ParentData>> result,
Set<Type> debugAncestorTypes,
Set<Type> debugParentDataTypes,
List<Type> debugAncestorCulprits,
) {
assert(() {
if (result == null || ancestor == null) {
return true;
}
// Check that no other ParentDataWidgets want to provide parent data.
final List<ParentDataElement<ParentData>> badAncestors = <ParentDataElement<ParentData>>[];
ancestor = ancestor!._parent;
while (ancestor != null && ancestor is! RenderObjectElement) {
if (ancestor is ParentDataElement<ParentData>) {
badAncestors.add(ancestor! as ParentDataElement<ParentData>);
}
ancestor = ancestor!._parent;
}
if (badAncestors.isNotEmpty) {
badAncestors.insert(0, result);
// Check that no other ParentDataWidgets of the same
// type want to provide parent data.
if (debugAncestorTypes.length != result.length || debugParentDataTypes.length != result.length) {
// This can only occur if the Sets of ancestors and parent data types was
// provided a dupe and did not add it.
assert(debugAncestorTypes.length < result.length || debugParentDataTypes.length < result.length);
try {
// We explicitly throw here (even though we immediately redirect the
// exception elsewhere) so that debuggers will notice it when they
// have "break on exception" enabled.
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Incorrect use of ParentDataWidget.'),
ErrorDescription('The following ParentDataWidgets are providing parent data to the same RenderObject:'),
for (final ParentDataElement<ParentData> ancestor in badAncestors)
ErrorDescription('- ${ancestor.widget} (typically placed directly inside a ${(ancestor.widget as ParentDataWidget<ParentData>).debugTypicalAncestorWidgetDescription} widget)'),
ErrorDescription('However, a RenderObject can only receive parent data from at most one ParentDataWidget.'),
ErrorHint('Usually, this indicates that at least one of the offending ParentDataWidgets listed above is not placed directly inside a compatible ancestor widget.'),
ErrorDescription('The ownership chain for the RenderObject that received the parent data was:\n ${debugGetCreatorChain(10)}'),
ErrorDescription(
'Competing ParentDataWidgets are providing parent data to the '
'same RenderObject:'
),
for (final ParentDataElement<ParentData> ancestor in result.where((ParentDataElement<ParentData> ancestor) {
return debugAncestorCulprits.contains(ancestor.runtimeType);
}))
ErrorDescription(
'- ${ancestor.widget}, which writes ParentData of type '
'${ancestor.debugParentDataType}, (typically placed directly '
'inside a '
'${(ancestor.widget as ParentDataWidget<ParentData>).debugTypicalAncestorWidgetClass} '
'widget)'
),
ErrorDescription(
'A RenderObject can receive parent data from multiple '
'ParentDataWidgets, but the Type of ParentData must be unique to '
'prevent one overwriting another.'
),
ErrorHint(
'Usually, this indicates that one or more of the offending '
"ParentDataWidgets listed above isn't placed inside a dedicated "
"compatible ancestor widget that it isn't sharing with another "
'ParentDataWidget of the same type.'
),
ErrorHint(
'Otherwise, separating aspects of ParentData to prevent '
'conflicts can be done using mixins, mixing them all in on the '
'full ParentData Object, such as KeepAlive does with '
'KeepAliveParentDataMixin.'
),
ErrorDescription(
'The ownership chain for the RenderObject that received the '
'parent data was:\n ${debugGetCreatorChain(10)}'
),
]);
} on FlutterError catch (e) {
_reportException(ErrorSummary('while looking for parent data.'), e, e.stackTrace);
} on FlutterError catch (error) {
_reportException(
ErrorSummary('while looking for parent data.'),
error,
error.stackTrace,
);
}
}
return true;
}());
}
List<ParentDataElement<ParentData>> _findAncestorParentDataElements() {
Element? ancestor = _parent;
final List<ParentDataElement<ParentData>> result = <ParentDataElement<ParentData>>[];
final Set<Type> debugAncestorTypes = <Type>{};
final Set<Type> debugParentDataTypes = <Type>{};
final List<Type> debugAncestorCulprits = <Type>[];
// More than one ParentDataWidget can contribute ParentData, but there are
// some constraints.
// 1. ParentData can only be written by unique ParentDataWidget types.
// For example, two KeepAlive ParentDataWidgets trying to write to the
// same child is not allowed.
// 2. Each contributing ParentDataWidget must contribute to a unique
// ParentData type, less ParentData be overwritten.
// For example, there cannot be two ParentDataWidgets that both write
// ParentData of type KeepAliveParentDataMixin, if the first check was
// subverted by a subclassing of the KeepAlive ParentDataWidget.
// 3. The ParentData itself must be compatible with all ParentDataWidgets
// writing to it.
// For example, TwoDimensionalViewportParentData uses the
// KeepAliveParentDataMixin, so it could be compatible with both
// KeepAlive, and another ParentDataWidget with ParentData type
// TwoDimensionalViewportParentData or a subclass thereof.
// The first and second cases are verified here. The third is verified in
// debugIsValidRenderObject.
while (ancestor != null && ancestor is! RenderObjectElement) {
if (ancestor is ParentDataElement<ParentData>) {
assert((ParentDataElement<ParentData> ancestor) {
if (!debugAncestorTypes.add(ancestor.runtimeType) || !debugParentDataTypes.add(ancestor.debugParentDataType)) {
debugAncestorCulprits.add(ancestor.runtimeType);
}
return true;
}(ancestor));
result.add(ancestor);
}
ancestor = ancestor._parent;
}
assert(() {
if (result.isEmpty || ancestor == null) {
return true;
}
// Validate points 1 and 2 from above.
_debugCheckCompetingAncestors(
result,
debugAncestorTypes,
debugParentDataTypes,
debugAncestorCulprits,
);
return true;
}());
return result;
}
......@@ -6477,8 +6567,8 @@ abstract class RenderObjectElement extends Element {
return true;
}());
_ancestorRenderObjectElement?.insertRenderObjectChild(renderObject, newSlot);
final ParentDataElement<ParentData>? parentDataElement = _findAncestorParentDataElement();
if (parentDataElement != null) {
final List<ParentDataElement<ParentData>> parentDataElements = _findAncestorParentDataElements();
for (final ParentDataElement<ParentData> parentDataElement in parentDataElements) {
_updateParentData(parentDataElement.widget as ParentDataWidget<ParentData>);
}
}
......
......@@ -250,12 +250,99 @@ void main() {
checkTree(tester, <TestParentData>[]);
});
testWidgetsWithLeakTracking('ParentData overwrite with custom ParentDataWidget subclasses', (WidgetTester tester) async {
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: Stack(
children: <Widget>[
CustomPositionedWidget(
bottom: 8.0,
child: Positioned(
top: 6.0,
left: 7.0,
child: DecoratedBox(decoration: kBoxDecorationB),
),
),
],
),
),
);
dynamic exception = tester.takeException();
expect(exception, isFlutterError);
expect(
exception.toString(),
startsWith(
'Incorrect use of ParentDataWidget.\n'
'Competing ParentDataWidgets are providing parent data to the same RenderObject:\n'
'- Positioned(left: 7.0, top: 6.0), which writes ParentData of type '
'StackParentData, (typically placed directly inside a Stack widget)\n'
'- CustomPositionedWidget, which writes ParentData of type '
'StackParentData, (typically placed directly inside a Stack widget)\n'
'A RenderObject can receive parent data from multiple '
'ParentDataWidgets, but the Type of ParentData must be unique to '
'prevent one overwriting another.\n'
'Usually, this indicates that one or more of the offending ParentDataWidgets listed '
"above isn't placed inside a dedicated compatible ancestor widget that it isn't "
'sharing with another ParentDataWidget of the same type.\n'
'Otherwise, separating aspects of ParentData to prevent conflicts can '
'be done using mixins, mixing them all in on the full ParentData '
'Object, such as KeepAlive does with KeepAliveParentDataMixin.\n'
'The ownership chain for the RenderObject that received the parent data was:\n'
' DecoratedBox ← Positioned ← CustomPositionedWidget ← Stack ← Directionality ← ', // End of chain omitted, not relevant for test.
),
);
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: Stack(
children: <Widget>[
SubclassPositioned(
bottom: 8.0,
child: Positioned(
top: 6.0,
left: 7.0,
child: DecoratedBox(decoration: kBoxDecorationB),
),
),
],
),
),
);
exception = tester.takeException();
expect(exception, isFlutterError);
expect(
exception.toString(),
startsWith(
'Incorrect use of ParentDataWidget.\n'
'Competing ParentDataWidgets are providing parent data to the same RenderObject:\n'
'- Positioned(left: 7.0, top: 6.0), which writes ParentData of type '
'StackParentData, (typically placed directly inside a Stack widget)\n'
'- SubclassPositioned(bottom: 8.0), which writes ParentData of type '
'StackParentData, (typically placed directly inside a Stack widget)\n'
'A RenderObject can receive parent data from multiple '
'ParentDataWidgets, but the Type of ParentData must be unique to '
'prevent one overwriting another.\n'
'Usually, this indicates that one or more of the offending ParentDataWidgets listed '
"above isn't placed inside a dedicated compatible ancestor widget that it isn't "
'sharing with another ParentDataWidget of the same type.\n'
'Otherwise, separating aspects of ParentData to prevent conflicts can '
'be done using mixins, mixing them all in on the full ParentData '
'Object, such as KeepAlive does with KeepAliveParentDataMixin.\n'
'The ownership chain for the RenderObject that received the parent data was:\n'
' DecoratedBox ← Positioned ← SubclassPositioned ← Stack ← Directionality ← ', // End of chain omitted, not relevant for test.
),
);
});
testWidgetsWithLeakTracking('ParentDataWidget conflicting data', (WidgetTester tester) async {
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: Stack(
textDirection: TextDirection.ltr,
children: <Widget>[
Positioned(
top: 5.0,
......@@ -277,19 +364,26 @@ void main() {
exception.toString(),
startsWith(
'Incorrect use of ParentDataWidget.\n'
'The following ParentDataWidgets are providing parent data to the same RenderObject:\n'
'- Positioned(left: 7.0, top: 6.0) (typically placed directly inside a Stack widget)\n'
'- Positioned(top: 5.0, bottom: 8.0) (typically placed directly inside a Stack widget)\n'
'However, a RenderObject can only receive parent data from at most one ParentDataWidget.\n'
'Usually, this indicates that at least one of the offending ParentDataWidgets listed '
'above is not placed directly inside a compatible ancestor widget.\n'
'Competing ParentDataWidgets are providing parent data to the same RenderObject:\n'
'- Positioned(left: 7.0, top: 6.0), which writes ParentData of type '
'StackParentData, (typically placed directly inside a Stack widget)\n'
'- Positioned(top: 5.0, bottom: 8.0), which writes ParentData of type '
'StackParentData, (typically placed directly inside a Stack widget)\n'
'A RenderObject can receive parent data from multiple '
'ParentDataWidgets, but the Type of ParentData must be unique to '
'prevent one overwriting another.\n'
'Usually, this indicates that one or more of the offending ParentDataWidgets listed '
"above isn't placed inside a dedicated compatible ancestor widget that it isn't "
'sharing with another ParentDataWidget of the same type.\n'
'Otherwise, separating aspects of ParentData to prevent conflicts can '
'be done using mixins, mixing them all in on the full ParentData '
'Object, such as KeepAlive does with KeepAliveParentDataMixin.\n'
'The ownership chain for the RenderObject that received the parent data was:\n'
' DecoratedBox ← Positioned ← Positioned ← Stack ← Directionality ← ', // End of chain omitted, not relevant for test.
),
);
await tester.pumpWidget(const Stack(textDirection: TextDirection.ltr));
checkTree(tester, <TestParentData>[]);
await tester.pumpWidget(
......@@ -308,6 +402,7 @@ void main() {
),
),
);
exception = tester.takeException();
expect(exception, isFlutterError);
expect(
......@@ -328,7 +423,6 @@ void main() {
await tester.pumpWidget(
const Stack(textDirection: TextDirection.ltr),
);
checkTree(tester, <TestParentData>[]);
});
......@@ -458,6 +552,46 @@ void main() {
});
}
class SubclassPositioned extends Positioned {
const SubclassPositioned({
super.key,
super.left,
super.top,
super.right,
super.bottom,
super.width,
super.height,
required super.child,
});
@override
void applyParentData(RenderObject renderObject) {
assert(renderObject.parentData is StackParentData);
final StackParentData parentData = renderObject.parentData! as StackParentData;
parentData.bottom = bottom;
}
}
class CustomPositionedWidget extends ParentDataWidget<StackParentData> {
const CustomPositionedWidget({
super.key,
required this.bottom,
required super.child,
});
final double bottom;
@override
void applyParentData(RenderObject renderObject) {
assert(renderObject.parentData is StackParentData);
final StackParentData parentData = renderObject.parentData! as StackParentData;
parentData.bottom = bottom;
}
@override
Type get debugTypicalAncestorWidgetClass => Stack;
}
class TestParentDataWidget extends ParentDataWidget<DummyParentData> {
const TestParentDataWidget({
super.key,
......
......@@ -192,6 +192,18 @@ class RenderSimpleBuilderTableViewport extends RenderTwoDimensionalViewport {
RenderBox? testGetChildFor(ChildVicinity vicinity) => getChildFor(vicinity);
@override
TestExtendedParentData parentDataOf(RenderBox child) {
return child.parentData! as TestExtendedParentData;
}
@override
void setupParentData(RenderBox child) {
if (child.parentData is! TestExtendedParentData) {
child.parentData = TestExtendedParentData();
}
}
@override
void layoutChildSequence() {
// Really simple table implementation for testing.
......@@ -468,3 +480,32 @@ class KeepAliveCheckBoxState extends State<KeepAliveCheckBox> with AutomaticKeep
);
}
}
// TwoDimensionalViewportParentData already mixes in KeepAliveParentDataMixin,
// and so should be compatible with both the KeepAlive and
// TestParentDataWidget ParentDataWidgets.
// This ParentData is set up above as part of the
// RenderSimpleBuilderTableViewport for testing.
class TestExtendedParentData extends TwoDimensionalViewportParentData {
int? testValue;
}
class TestParentDataWidget extends ParentDataWidget<TestExtendedParentData> {
const TestParentDataWidget({
super.key,
required super.child,
this.testValue,
});
final int? testValue;
@override
void applyParentData(RenderObject renderObject) {
assert(renderObject.parentData is TestExtendedParentData);
final TestExtendedParentData parentData = renderObject.parentData! as TestExtendedParentData;
parentData.testValue = testValue;
}
@override
Type get debugTypicalAncestorWidgetClass => SimpleBuilderTableViewport;
}
......@@ -312,6 +312,120 @@ void main() {
);
});
testWidgets('Keep alive works with additional parent data widgets', (WidgetTester tester) async {
const ChildVicinity firstCell = ChildVicinity(xIndex: 0, yIndex: 0);
final ScrollController verticalController = ScrollController();
final UniqueKey checkBoxKey = UniqueKey();
final TwoDimensionalChildBuilderDelegate builderDelegate = TwoDimensionalChildBuilderDelegate(
maxXIndex: 5,
maxYIndex: 5,
addRepaintBoundaries: false,
builder: (BuildContext context, ChildVicinity vicinity) {
// The delegate will add a KeepAlive ParentDataWidget, this add an
// additional ParentDataWidget.
return TestParentDataWidget(
testValue: 20,
child: SizedBox.square(
dimension: 200,
child: Center(child: vicinity == firstCell
? KeepAliveCheckBox(key: checkBoxKey)
: Text('R${vicinity.xIndex}:C${vicinity.yIndex}')
),
),
);
}
);
await tester.pumpWidget(simpleBuilderTest(
delegate: builderDelegate,
verticalDetails: ScrollableDetails.vertical(controller: verticalController),
));
await tester.pumpAndSettle();
expect(verticalController.hasClients, isTrue);
expect(verticalController.position.pixels, 0.0);
expect(find.byKey(checkBoxKey), findsOneWidget);
expect(
tester.state<KeepAliveCheckBoxState>(find.byKey(checkBoxKey)).checkValue,
isFalse,
);
expect(
tester.state<KeepAliveCheckBoxState>(find.byKey(checkBoxKey)).wantKeepAlive,
isFalse,
);
RenderSimpleBuilderTableViewport viewport = getViewport(tester, checkBoxKey) as RenderSimpleBuilderTableViewport;
TestExtendedParentData parentData = viewport.parentDataOf(viewport.testGetChildFor(firstCell)!);
// Check parent data from both ParentDataWidgets
expect(parentData.testValue, 20);
expect(parentData.keepAlive, isFalse);
// Scroll away, disposing of the checkbox.
verticalController.jumpTo(verticalController.position.maxScrollExtent);
await tester.pump();
expect(verticalController.position.pixels, 600.0);
expect(find.byKey(checkBoxKey), findsNothing);
// Bring back into view, still unchecked, not kept alive.
verticalController.jumpTo(0.0);
await tester.pump();
expect(verticalController.position.pixels, 0.0);
expect(find.byKey(checkBoxKey), findsOneWidget);
// Check the box to set keep alive to true.
await tester.tap(find.byKey(checkBoxKey));
await tester.pumpAndSettle();
expect(
tester.state<KeepAliveCheckBoxState>(find.byKey(checkBoxKey)).checkValue,
isTrue,
);
expect(
tester.state<KeepAliveCheckBoxState>(find.byKey(checkBoxKey)).wantKeepAlive,
isTrue,
);
viewport = getViewport(tester, checkBoxKey) as RenderSimpleBuilderTableViewport;
parentData = viewport.parentDataOf(viewport.testGetChildFor(firstCell)!);
// Check parent data from both ParentDataWidgets
expect(parentData.testValue, 20);
expect(parentData.keepAlive, isTrue);
// Scroll away again, checkbox should be kept alive now.
verticalController.jumpTo(verticalController.position.maxScrollExtent);
await tester.pump();
expect(verticalController.position.pixels, 600.0);
expect(find.byKey(checkBoxKey), findsOneWidget);
expect(
tester.state<KeepAliveCheckBoxState>(find.byKey(checkBoxKey)).checkValue,
isTrue,
);
expect(
tester.state<KeepAliveCheckBoxState>(find.byKey(checkBoxKey)).wantKeepAlive,
isTrue,
);
viewport = getViewport(tester, checkBoxKey) as RenderSimpleBuilderTableViewport;
parentData = viewport.parentDataOf(viewport.testGetChildFor(firstCell)!);
// Check parent data from both ParentDataWidgets
expect(parentData.testValue, 20);
expect(parentData.keepAlive, isTrue);
// Bring back into view, still checked, after being kept alive.
verticalController.jumpTo(0.0);
await tester.pump();
expect(verticalController.position.pixels, 0.0);
expect(find.byKey(checkBoxKey), findsOneWidget);
expect(
tester.state<KeepAliveCheckBoxState>(find.byKey(checkBoxKey)).checkValue,
isTrue,
);
expect(
tester.state<KeepAliveCheckBoxState>(find.byKey(checkBoxKey)).wantKeepAlive,
isTrue,
);
viewport = getViewport(tester, checkBoxKey) as RenderSimpleBuilderTableViewport;
parentData = viewport.parentDataOf(viewport.testGetChildFor(firstCell)!);
// Check parent data from both ParentDataWidgets
expect(parentData.testValue, 20);
expect(parentData.keepAlive, isTrue);
});
testWidgetsWithLeakTracking('builder delegate will not add automatic keep alives', (WidgetTester tester) async {
const ChildVicinity firstCell = ChildVicinity(xIndex: 0, yIndex: 0);
final ScrollController verticalController = ScrollController();
......@@ -787,7 +901,7 @@ void main() {
expect(horizontal.widget.controller, isNotNull);
}, variant: TargetPlatformVariant.all());
testWidgets('asserts the axis directions do not conflict with one another', (WidgetTester tester) async {
testWidgetsWithLeakTracking('asserts the axis directions do not conflict with one another', (WidgetTester tester) async {
final List<Object> exceptions = <Object>[];
final FlutterExceptionHandler? oldHandler = FlutterError.onError;
FlutterError.onError = (FlutterErrorDetails details) {
......
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