Skip to content

F
Front-End
Unverified Commit 2dc71a34 authored by chunhtai's avatar chunhtai Committed by GitHub
Browse files
Options

reland Refactors global key duplication detection (#49896) 

* reland "Refactors global key duplication detection (#46183)"

This reverts commit d2b66dbf.

* fix test
parent 2c51efef
Hide whitespace changes
Inline Side-by-side
Showing with 756 additions and 84 deletions
packages/flutter/lib/src/cupertino/action_sheet.dart
View file @ 2dc71a34
...@@ -469,6 +469,7 @@ class _CupertinoAlertRenderElement extends RenderObjectElement { ...@@ -469,6 +469,7 @@ class _CupertinoAlertRenderElement extends RenderObjectElement {
} else if (_actionsElement == child) { } else if (_actionsElement == child) {
_actionsElement = null; _actionsElement = null;
} }
super.forgetChild(child);
} }
@override @override
......
packages/flutter/lib/src/cupertino/dialog.dart
View file @ 2dc71a34
...@@ -464,6 +464,7 @@ class _CupertinoDialogRenderElement extends RenderObjectElement { ...@@ -464,6 +464,7 @@ class _CupertinoDialogRenderElement extends RenderObjectElement {
assert(_actionsElement == child); assert(_actionsElement == child);
_actionsElement = null; _actionsElement = null;
} }
super.forgetChild(child);
} }
@override @override
......
packages/flutter/lib/src/material/chip.dart
View file @ 2dc71a34
...@@ -2072,6 +2072,7 @@ class _RenderChipElement extends RenderObjectElement { ...@@ -2072,6 +2072,7 @@ class _RenderChipElement extends RenderObjectElement {
final _ChipSlot slot = childToSlot[child]; final _ChipSlot slot = childToSlot[child];
childToSlot.remove(child); childToSlot.remove(child);
slotToChild.remove(slot); slotToChild.remove(slot);
super.forgetChild(child);
} }
void _mountChild(Widget widget, _ChipSlot slot) { void _mountChild(Widget widget, _ChipSlot slot) {
......
packages/flutter/lib/src/material/input_decorator.dart
View file @ 2dc71a34
...@@ -1511,6 +1511,7 @@ class _RenderDecorationElement extends RenderObjectElement { ...@@ -1511,6 +1511,7 @@ class _RenderDecorationElement extends RenderObjectElement {
final _DecorationSlot slot = childToSlot[child]; final _DecorationSlot slot = childToSlot[child];
childToSlot.remove(child); childToSlot.remove(child);
slotToChild.remove(slot); slotToChild.remove(slot);
super.forgetChild(child);
} }
void _mountChild(Widget widget, _DecorationSlot slot) { void _mountChild(Widget widget, _DecorationSlot slot) {
......
packages/flutter/lib/src/material/list_tile.dart
View file @ 2dc71a34
...@@ -992,6 +992,7 @@ class _ListTileElement extends RenderObjectElement { ...@@ -992,6 +992,7 @@ class _ListTileElement extends RenderObjectElement {
final _ListTileSlot slot = childToSlot[child]; final _ListTileSlot slot = childToSlot[child];
childToSlot.remove(child); childToSlot.remove(child);
slotToChild.remove(slot); slotToChild.remove(slot);
super.forgetChild(child);
} }
void _mountChild(Widget widget, _ListTileSlot slot) { void _mountChild(Widget widget, _ListTileSlot slot) {
......
packages/flutter/lib/src/widgets/binding.dart
View file @ 2dc71a34
...@@ -1016,6 +1016,7 @@ class RenderObjectToWidgetElement<T extends RenderObject> extends RootRenderObje ...@@ -1016,6 +1016,7 @@ class RenderObjectToWidgetElement<T extends RenderObject> extends RootRenderObje
void forgetChild(Element child) { void forgetChild(Element child) {
assert(child == _child); assert(child == _child);
_child = null; _child = null;
super.forgetChild(child);
} }
@override @override
......
packages/flutter/lib/src/widgets/framework.dart
View file @ 2dc71a34
...@@ -132,7 +132,20 @@ abstract class GlobalKey<T extends State<StatefulWidget>> extends Key { ...@@ -132,7 +132,20 @@ abstract class GlobalKey<T extends State<StatefulWidget>> extends Key {
static final Map<GlobalKey, Element> _registry = <GlobalKey, Element>{}; static final Map<GlobalKey, Element> _registry = <GlobalKey, Element>{};
static final Set<Element> _debugIllFatedElements = HashSet<Element>(); static final Set<Element> _debugIllFatedElements = HashSet<Element>();
static final Map<GlobalKey, Element> _debugReservations = <GlobalKey, Element>{}; // This map keeps track which child reserve the global key with the parent.
// Parent, child -> global key.
// This provides us a way to remove old reservation while parent rebuilds the
// child in the same slot.
static final Map<Element, Map<Element, GlobalKey>> _debugReservations = <Element, Map<Element, GlobalKey>>{};
static void _debugRemoveReservationFor(Element parent, Element child) {
assert(() {
assert(parent != null);
assert(child != null);
_debugReservations[parent]?.remove(child);
return true;
}());
}
void _register(Element element) { void _register(Element element) {
assert(() { assert(() {
...@@ -160,46 +173,83 @@ abstract class GlobalKey<T extends State<StatefulWidget>> extends Key { ...@@ -160,46 +173,83 @@ abstract class GlobalKey<T extends State<StatefulWidget>> extends Key {
_registry.remove(this); _registry.remove(this);
} }
void _debugReserveFor(Element parent) { void _debugReserveFor(Element parent, Element child) {
assert(() { assert(() {
assert(parent != null); assert(parent != null);
if (_debugReservations.containsKey(this) && _debugReservations[this] != parent) { assert(child != null);
// Reserving a new parent while the old parent is not attached is ok. _debugReservations[parent] ??= <Element, GlobalKey>{};
// This can happen when a renderObject detaches and re-attaches to rendering _debugReservations[parent][child] = this;
// tree multiple times. return true;
if (_debugReservations[this].renderObject?.attached == false) { }());
_debugReservations[this] = parent; }
return true;
} static void _debugVerifyGlobalKeyReservation() {
// It's possible for an element to get built multiple times in one assert(() {
// frame, in which case it'll reserve the same child's key multiple final Map<GlobalKey, Element> keyToParent = <GlobalKey, Element>{};
// times. We catch multiple children of one widget having the same key _debugReservations.forEach((Element parent, Map<Element, GlobalKey> chidToKey) {
// by verifying that an element never steals elements from itself, so we // We ignore parent that are detached.
// don't care to verify that here as well. if (parent.renderObject?.attached == false)
final String older = _debugReservations[this].toString(); return;
final String newer = parent.toString(); chidToKey.forEach((Element child, GlobalKey key) {
if (older != newer) { // If parent = null, the node is deactivated by its parent and is
throw FlutterError.fromParts(<DiagnosticsNode>[ // not re-attached to other part of the tree. We should ignore this
ErrorSummary('Multiple widgets used the same GlobalKey.'), // node.
ErrorDescription( if (child._parent == null)
'The key $this was used by multiple widgets. The parents of those widgets were:\n' return;
'- $older\n' // It is possible the same key registers to the same parent twice
'- $newer\n' // with different children. That is illegal, but it is not in the
'A GlobalKey can only be specified on one widget at a time in the widget tree.' // scope of this check. Such error will be detected in
), // _debugVerifyIllFatedPopulation or
]); // _debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans.
} if (keyToParent.containsKey(key) && keyToParent[key] != parent) {
throw FlutterError.fromParts(<DiagnosticsNode>[ // We have duplication reservations for the same global key.
ErrorSummary('Multiple widgets used the same GlobalKey.'), final Element older = keyToParent[key];
ErrorDescription( final Element newer = parent;
'The key $this was used by multiple widgets. The parents of those widgets were ' FlutterError error;
'different widgets that both had the following description:\n' if (older.toString() != newer.toString()) {
' $parent\n' error = FlutterError.fromParts(<DiagnosticsNode>[
'A GlobalKey can only be specified on one widget at a time in the widget tree.' ErrorSummary('Multiple widgets used the same GlobalKey.'),
), ErrorDescription(
]); 'The key $key was used by multiple widgets. The parents of those widgets were:\n'
} '- ${older.toString()}\n'
_debugReservations[this] = parent; '- ${newer.toString()}\n'
'A GlobalKey can only be specified on one widget at a time in the widget tree.'
),
]);
} else {
error = FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Multiple widgets used the same GlobalKey.'),
ErrorDescription(
'The key $key was used by multiple widgets. The parents of those widgets were '
'different widgets that both had the following description:\n'
' ${parent.toString()}\n'
'A GlobalKey can only be specified on one widget at a time in the widget tree.'
),
]);
}
// Fix the tree by removing the duplicated child from one of its
// parents to resolve the duplicated key issue. This allows us to
// tear down the tree during testing without producing additional
// misleading exceptions.
if (child._parent != older) {
older.visitChildren((Element currentChild) {
if (currentChild == child)
older.forgetChild(child);
});
}
if (child._parent != newer) {
newer.visitChildren((Element currentChild) {
if (currentChild == child)
newer.forgetChild(child);
});
}
throw error;
} else {
keyToParent[key] = parent;
}
});
});
_debugReservations.clear();
return true; return true;
}()); }());
} }
...@@ -215,13 +265,13 @@ abstract class GlobalKey<T extends State<StatefulWidget>> extends Key { ...@@ -215,13 +265,13 @@ abstract class GlobalKey<T extends State<StatefulWidget>> extends Key {
final GlobalKey key = element.widget.key as GlobalKey; final GlobalKey key = element.widget.key as GlobalKey;
assert(_registry.containsKey(key)); assert(_registry.containsKey(key));
duplicates ??= <GlobalKey, Set<Element>>{}; duplicates ??= <GlobalKey, Set<Element>>{};
final Set<Element> elements = duplicates.putIfAbsent(key, () => HashSet<Element>()); // Uses ordered set to produce consistent error message.
final Set<Element> elements = duplicates.putIfAbsent(key, () => LinkedHashSet<Element>());
elements.add(element); elements.add(element);
elements.add(_registry[key]); elements.add(_registry[key]);
} }
} }
_debugIllFatedElements.clear(); _debugIllFatedElements.clear();
_debugReservations.clear();
if (duplicates != null) { if (duplicates != null) {
final List<DiagnosticsNode> information = <DiagnosticsNode>[]; final List<DiagnosticsNode> information = <DiagnosticsNode>[];
information.add(ErrorSummary('Multiple widgets used the same GlobalKey.')); information.add(ErrorSummary('Multiple widgets used the same GlobalKey.'));
...@@ -2640,6 +2690,7 @@ class BuildOwner { ...@@ -2640,6 +2690,7 @@ class BuildOwner {
}); });
assert(() { assert(() {
try { try {
GlobalKey._debugVerifyGlobalKeyReservation();
GlobalKey._debugVerifyIllFatedPopulation(); GlobalKey._debugVerifyIllFatedPopulation();
if (_debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans != null && if (_debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans != null &&
_debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans.isNotEmpty) { _debugElementsThatWillNeedToBeRebuiltDueToGlobalKeyShenanigans.isNotEmpty) {
...@@ -3094,18 +3145,12 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3094,18 +3145,12 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
/// | **child != null** | Old child is removed, returns null. | Old child updated if possible, returns child or new [Element]. | /// | **child != null** | Old child is removed, returns null. | Old child updated if possible, returns child or new [Element]. |
@protected @protected
Element updateChild(Element child, Widget newWidget, dynamic newSlot) { Element updateChild(Element child, Widget newWidget, dynamic newSlot) {
assert(() {
final Key key = newWidget?.key;
if (key is GlobalKey) {
key._debugReserveFor(this);
}
return true;
}());
if (newWidget == null) { if (newWidget == null) {
if (child != null) if (child != null)
deactivateChild(child); deactivateChild(child);
return null; return null;
} }
Element newChild;
if (child != null) { if (child != null) {
bool hasSameSuperclass = true; bool hasSameSuperclass = true;
// When the type of a widget is changed between Stateful and Stateless via // When the type of a widget is changed between Stateful and Stateless via
...@@ -3129,28 +3174,40 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3129,28 +3174,40 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
hasSameSuperclass = oldElementClass == newWidgetClass; hasSameSuperclass = oldElementClass == newWidgetClass;
return true; return true;
}()); }());
if (hasSameSuperclass) { if (hasSameSuperclass && child.widget == newWidget) {
if (child.widget == newWidget) { if (child.slot != newSlot)
if (child.slot != newSlot) updateSlotForChild(child, newSlot);
updateSlotForChild(child, newSlot); newChild = child;
return child; } else if (hasSameSuperclass && Widget.canUpdate(child.widget, newWidget)) {
} if (child.slot != newSlot)
if (Widget.canUpdate(child.widget, newWidget)) { updateSlotForChild(child, newSlot);
if (child.slot != newSlot) child.update(newWidget);
updateSlotForChild(child, newSlot); assert(child.widget == newWidget);
child.update(newWidget); assert(() {
assert(child.widget == newWidget); child.owner._debugElementWasRebuilt(child);
assert(() { return true;
child.owner._debugElementWasRebuilt(child); }());
return true; newChild = child;
}()); } else {
return child; deactivateChild(child);
} assert(child._parent == null);
newChild = inflateWidget(newWidget, newSlot);
} }
deactivateChild(child); } else {
assert(child._parent == null); newChild = inflateWidget(newWidget, newSlot);
} }
return inflateWidget(newWidget, newSlot);
assert(() {
if (child != null)
_debugRemoveGlobalKeyReservation(child);
final Key key = newWidget?.key;
if (key is GlobalKey) {
key._debugReserveFor(this, newChild);
}
return true;
}());
return newChild;
} }
/// Add this element to the tree in the given slot of the given parent. /// Add this element to the tree in the given slot of the given parent.
...@@ -3188,6 +3245,9 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3188,6 +3245,9 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
}()); }());
} }
void _debugRemoveGlobalKeyReservation(Element child) {
GlobalKey._debugRemoveReservationFor(this, child);
}
/// Change the widget used to configure this element. /// Change the widget used to configure this element.
/// ///
/// The framework calls this function when the parent wishes to use a /// The framework calls this function when the parent wishes to use a
...@@ -3206,6 +3266,16 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3206,6 +3266,16 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
&& depth != null && depth != null
&& _active && _active
&& Widget.canUpdate(widget, newWidget)); && Widget.canUpdate(widget, newWidget));
// This Element was told to update and we can now release all the global key
// reservations of forgotten children. We cannot do this earlier because the
// forgotten children still represent global key duplications if the element
// never updates (the forgotten children are not removed from the tree
// until the call to update happens)
assert(() {
_debugForgottenChildrenWithGlobalKey.forEach(_debugRemoveGlobalKeyReservation);
_debugForgottenChildrenWithGlobalKey.clear();
return true;
}());
_widget = newWidget; _widget = newWidget;
} }
...@@ -3402,6 +3472,10 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3402,6 +3472,10 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
}()); }());
} }
// The children that have been forgotten by forgetChild. This will be used in
// [update] to remove the global key reservations of forgotten children.
final Set<Element> _debugForgottenChildrenWithGlobalKey = HashSet<Element>();
/// Remove the given child from the element's child list, in preparation for /// Remove the given child from the element's child list, in preparation for
/// the child being reused elsewhere in the element tree. /// the child being reused elsewhere in the element tree.
/// ///
...@@ -3410,12 +3484,23 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -3410,12 +3484,23 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
/// ///
/// The element will still have a valid parent when this is called. After this /// The element will still have a valid parent when this is called. After this
/// is called, [deactivateChild] is called to sever the link to this object. /// is called, [deactivateChild] is called to sever the link to this object.
///
/// The [update] is responsible for updating or creating the new child that
/// will replace this [child].
@protected @protected
@mustCallSuper
void forgetChild(Element child) { void forgetChild(Element child) {
// TODO(chunhtai): Creates empty body for subclass to call super. This will // This method is called on the old parent when the given child (with a
// enable us to fix internal tests pro-actively for upcoming breaking // global key) is given a new parent. We cannot remove the global key
// change. // reservation directly in this method because the forgotten child is not
// https://github.com/flutter/flutter/issues/43780. // removed from the tree until this Element is updated in [update]. If
// [update] is never called, the forgotten child still represents a global
// key duplication that we need to catch.
assert(() {
if (child.widget.key is GlobalKey)
_debugForgottenChildrenWithGlobalKey.add(child);
return true;
}());
} }
void _activateWithParent(Element parent, dynamic newSlot) { void _activateWithParent(Element parent, dynamic newSlot) {
...@@ -4445,6 +4530,7 @@ abstract class ComponentElement extends Element { ...@@ -4445,6 +4530,7 @@ abstract class ComponentElement extends Element {
void forgetChild(Element child) { void forgetChild(Element child) {
assert(child == _child); assert(child == _child);
_child = null; _child = null;
super.forgetChild(child);
} }
} }
...@@ -5598,6 +5684,7 @@ class LeafRenderObjectElement extends RenderObjectElement { ...@@ -5598,6 +5684,7 @@ class LeafRenderObjectElement extends RenderObjectElement {
@override @override
void forgetChild(Element child) { void forgetChild(Element child) {
assert(false); assert(false);
super.forgetChild(child);
} }
@override @override
...@@ -5647,6 +5734,7 @@ class SingleChildRenderObjectElement extends RenderObjectElement { ...@@ -5647,6 +5734,7 @@ class SingleChildRenderObjectElement extends RenderObjectElement {
void forgetChild(Element child) { void forgetChild(Element child) {
assert(child == _child); assert(child == _child);
_child = null; _child = null;
super.forgetChild(child);
} }
@override @override
...@@ -5753,6 +5841,7 @@ class MultiChildRenderObjectElement extends RenderObjectElement { ...@@ -5753,6 +5841,7 @@ class MultiChildRenderObjectElement extends RenderObjectElement {
assert(_children.contains(child)); assert(_children.contains(child));
assert(!_forgottenChildren.contains(child)); assert(!_forgottenChildren.contains(child));
_forgottenChildren.add(child); _forgottenChildren.add(child);
super.forgetChild(child);
} }
@override @override
......
packages/flutter/lib/src/widgets/layout_builder.dart
View file @ 2dc71a34
...@@ -60,6 +60,7 @@ class _LayoutBuilderElement<ConstraintType extends Constraints> extends RenderOb ...@@ -60,6 +60,7 @@ class _LayoutBuilderElement<ConstraintType extends Constraints> extends RenderOb
void forgetChild(Element child) { void forgetChild(Element child) {
assert(child == _child); assert(child == _child);
_child = null; _child = null;
super.forgetChild(child);
} }
@override @override
......
packages/flutter/lib/src/widgets/list_wheel_scroll_view.dart
View file @ 2dc71a34
...@@ -924,6 +924,7 @@ class ListWheelElement extends RenderObjectElement implements ListWheelChildMana ...@@ -924,6 +924,7 @@ class ListWheelElement extends RenderObjectElement implements ListWheelChildMana
@override @override
void forgetChild(Element child) { void forgetChild(Element child) {
_childElements.remove(child.slot); _childElements.remove(child.slot);
super.forgetChild(child);
} }
} }
......
packages/flutter/lib/src/widgets/sliver.dart
View file @ 2dc71a34
...@@ -1160,6 +1160,7 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render ...@@ -1160,6 +1160,7 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
assert(child.slot != null); assert(child.slot != null);
assert(_childElements.containsKey(child.slot)); assert(_childElements.containsKey(child.slot));
_childElements.remove(child.slot); _childElements.remove(child.slot);
super.forgetChild(child);
} }
@override @override
......
packages/flutter/lib/src/widgets/sliver_persistent_header.dart
View file @ 2dc71a34
...@@ -230,6 +230,7 @@ class _SliverPersistentHeaderElement extends RenderObjectElement { ...@@ -230,6 +230,7 @@ class _SliverPersistentHeaderElement extends RenderObjectElement {
void forgetChild(Element child) { void forgetChild(Element child) {
assert(child == this.child); assert(child == this.child);
this.child = null; this.child = null;
super.forgetChild(child);
} }
@override @override
......
packages/flutter/lib/src/widgets/table.dart
View file @ 2dc71a34
...@@ -344,6 +344,7 @@ class _TableElement extends RenderObjectElement { ...@@ -344,6 +344,7 @@ class _TableElement extends RenderObjectElement {
@override @override
bool forgetChild(Element child) { bool forgetChild(Element child) {
_forgottenChildren.add(child); _forgottenChildren.add(child);
super.forgetChild(child);
return true; return true;
} }
} }
......
packages/flutter/test/foundation/diagnostics_json_test.dart
View file @ 2dc71a34
...@@ -221,11 +221,6 @@ void main() { ...@@ -221,11 +221,6 @@ void main() {
class _TestElement extends Element { class _TestElement extends Element {
_TestElement() : super(const Placeholder()); _TestElement() : super(const Placeholder());
@override
void forgetChild(Element child) {
// Intentionally left empty.
}
@override @override
void performRebuild() { void performRebuild() {
// Intentionally left empty. // Intentionally left empty.
......
packages/flutter/test/widgets/framework_test.dart
View file @ 2dc71a34
...@@ -6,6 +6,8 @@ import 'package:flutter_test/flutter_test.dart'; ...@@ -6,6 +6,8 @@ import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
typedef ElementRebuildCallback = void Function(StatefulElement element);
class TestState extends State<StatefulWidget> { class TestState extends State<StatefulWidget> {
@override @override
Widget build(BuildContext context) => null; Widget build(BuildContext context) => null;
...@@ -61,6 +63,358 @@ void main() { ...@@ -61,6 +63,358 @@ void main() {
expect(keyA, isNot(equals(keyB))); expect(keyA, isNot(equals(keyB)));
}); });
testWidgets('GlobalKey correct case 1 - can move global key from container widget to layoutbuilder', (WidgetTester tester) async {
final Key key = GlobalKey(debugLabel: 'correct');
await tester.pumpWidget(Stack(
textDirection: TextDirection.ltr,
children: <Widget>[
Container(
key: const ValueKey<int>(1),
child: SizedBox(key: key),
),
LayoutBuilder(
key: const ValueKey<int>(2),
builder: (BuildContext context, BoxConstraints constraints) {
return const Placeholder();
},
),
],
));
await tester.pumpWidget(Stack(
textDirection: TextDirection.ltr,
children: <Widget>[
Container(
key: const ValueKey<int>(1),
child: const Placeholder(),
),
LayoutBuilder(
key: const ValueKey<int>(2),
builder: (BuildContext context, BoxConstraints constraints) {
return SizedBox(key: key);
},
),
],
));
});
testWidgets('GlobalKey correct case 2 - can move global key from layoutbuilder to container widget', (WidgetTester tester) async {
final Key key = GlobalKey(debugLabel: 'correct');
await tester.pumpWidget(Stack(
textDirection: TextDirection.ltr,
children: <Widget>[
Container(
key: const ValueKey<int>(1),
child: const Placeholder(),
),
LayoutBuilder(
key: const ValueKey<int>(2),
builder: (BuildContext context, BoxConstraints constraints) {
return SizedBox(key: key);
},
),
],
));
await tester.pumpWidget(Stack(
textDirection: TextDirection.ltr,
children: <Widget>[
Container(
key: const ValueKey<int>(1),
child: SizedBox(key: key),
),
LayoutBuilder(
key: const ValueKey<int>(2),
builder: (BuildContext context, BoxConstraints constraints) {
return const Placeholder();
},
),
],
));
});
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');
Key rebuiltKeyOfSecondChildBeforeLayout;
Key rebuiltKeyOfFirstChildAfterLayout;
Key rebuiltKeyOfSecondChildAfterLayout;
await tester.pumpWidget(
LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return Column(
children: <Widget>[
const _Stateful(
child: Text(
'Text1',
textDirection: TextDirection.ltr,
key: key1,
),
),
_Stateful(
child: const Text(
'Text2',
textDirection: TextDirection.ltr,
key: key2,
),
onElementRebuild: (StatefulElement element) {
// We don't want noise to override the result;
expect(rebuiltKeyOfSecondChildBeforeLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfSecondChildBeforeLayout =
statefulWidget.child.key;
},
),
],
);
},
)
);
// Result will be written during first build and need to clear it to remove
// noise.
rebuiltKeyOfSecondChildBeforeLayout = null;
final _StatefulState state = tester.firstState(find.byType(_Stateful).at(1));
state.rebuild();
// Reorders the items
await tester.pumpWidget(
LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return Column(
children: <Widget>[
_Stateful(
child: const Text(
'Text2',
textDirection: TextDirection.ltr,
key: key2,
),
onElementRebuild: (StatefulElement element) {
// Verifies the early rebuild happens before layout.
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
// We don't want noise to override the result;
expect(rebuiltKeyOfFirstChildAfterLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfFirstChildAfterLayout = statefulWidget.child.key;
},
),
_Stateful(
child: const Text(
'Text1',
textDirection: TextDirection.ltr,
key: key1,
),
onElementRebuild: (StatefulElement element) {
// Verifies the early rebuild happens before layout.
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
// We don't want noise to override the result;
expect(rebuiltKeyOfSecondChildAfterLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfSecondChildAfterLayout = statefulWidget.child.key;
},
),
],
);
},
)
);
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
expect(rebuiltKeyOfFirstChildAfterLayout, key2);
expect(rebuiltKeyOfSecondChildAfterLayout, key1);
});
testWidgets('GlobalKey correct case 4 - can deal with early rebuild in layoutbuilder - move forward', (WidgetTester tester) async {
const Key key1 = GlobalObjectKey('Text1');
const Key key2 = GlobalObjectKey('Text2');
const Key key3 = GlobalObjectKey('Text3');
Key rebuiltKeyOfSecondChildBeforeLayout;
Key rebuiltKeyOfSecondChildAfterLayout;
Key rebuiltKeyOfThirdChildAfterLayout;
await tester.pumpWidget(
LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return Column(
children: <Widget>[
const _Stateful(
child: Text(
'Text1',
textDirection: TextDirection.ltr,
key: key1,
),
),
_Stateful(
child: const Text(
'Text2',
textDirection: TextDirection.ltr,
key: key2,
),
onElementRebuild: (StatefulElement element) {
// We don't want noise to override the result;
expect(rebuiltKeyOfSecondChildBeforeLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfSecondChildBeforeLayout = statefulWidget.child.key;
},
),
const _Stateful(
child: Text(
'Text3',
textDirection: TextDirection.ltr,
key: key3,
),
),
],
);
},
)
);
// Result will be written during first build and need to clear it to remove
// noise.
rebuiltKeyOfSecondChildBeforeLayout = null;
final _StatefulState state = tester.firstState(find.byType(_Stateful).at(1));
state.rebuild();
// Reorders the items
await tester.pumpWidget(
LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return Column(
children: <Widget>[
const _Stateful(
child: Text(
'Text1',
textDirection: TextDirection.ltr,
key: key1,
),
),
_Stateful(
child: const Text(
'Text3',
textDirection: TextDirection.ltr,
key: key3,
),
onElementRebuild: (StatefulElement element) {
// Verifies the early rebuild happens before layout.
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
// We don't want noise to override the result;
expect(rebuiltKeyOfSecondChildAfterLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfSecondChildAfterLayout = statefulWidget.child.key;
},
),
_Stateful(
child: const Text(
'Text2',
textDirection: TextDirection.ltr,
key: key2,
),
onElementRebuild: (StatefulElement element) {
// Verifies the early rebuild happens before layout.
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
// We don't want noise to override the result;
expect(rebuiltKeyOfThirdChildAfterLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfThirdChildAfterLayout = statefulWidget.child.key;
},
),
],
);
},
)
);
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
expect(rebuiltKeyOfSecondChildAfterLayout, key3);
expect(rebuiltKeyOfThirdChildAfterLayout, key2);
});
testWidgets('GlobalKey correct case 5 - can deal with early rebuild in layoutbuilder - only one global key', (WidgetTester tester) async {
const Key key1 = GlobalObjectKey('Text1');
Key rebuiltKeyOfSecondChildBeforeLayout;
Key rebuiltKeyOfThirdChildAfterLayout;
await tester.pumpWidget(
LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return Column(
children: <Widget>[
const _Stateful(
child: Text(
'Text1',
textDirection: TextDirection.ltr,
),
),
_Stateful(
child: const Text(
'Text2',
textDirection: TextDirection.ltr,
key: key1,
),
onElementRebuild: (StatefulElement element) {
// We don't want noise to override the result;
expect(rebuiltKeyOfSecondChildBeforeLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfSecondChildBeforeLayout = statefulWidget.child.key;
},
),
const _Stateful(
child: Text(
'Text3',
textDirection: TextDirection.ltr,
),
),
],
);
},
)
);
// Result will be written during first build and need to clear it to remove
// noise.
rebuiltKeyOfSecondChildBeforeLayout = null;
final _StatefulState state = tester.firstState(find.byType(_Stateful).at(1));
state.rebuild();
// Reorders the items
await tester.pumpWidget(
LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return Column(
children: <Widget>[
const _Stateful(
child: Text(
'Text1',
textDirection: TextDirection.ltr,
),
),
_Stateful(
child: const Text(
'Text3',
textDirection: TextDirection.ltr,
),
onElementRebuild: (StatefulElement element) {
// Verifies the early rebuild happens before layout.
expect(rebuiltKeyOfSecondChildBeforeLayout, key1);
},
),
_Stateful(
child: const Text(
'Text2',
textDirection: TextDirection.ltr,
key: key1,
),
onElementRebuild: (StatefulElement element) {
// Verifies the early rebuild happens before layout.
expect(rebuiltKeyOfSecondChildBeforeLayout, key1);
// We don't want noise to override the result;
expect(rebuiltKeyOfThirdChildAfterLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfThirdChildAfterLayout = statefulWidget.child.key;
},
),
],
);
},
)
);
expect(rebuiltKeyOfSecondChildBeforeLayout, key1);
expect(rebuiltKeyOfThirdChildAfterLayout, key1);
});
testWidgets('GlobalKey duplication 1 - double appearance', (WidgetTester tester) async { testWidgets('GlobalKey duplication 1 - double appearance', (WidgetTester tester) async {
final Key key = GlobalKey(debugLabel: 'problematic'); final Key key = GlobalKey(debugLabel: 'problematic');
await tester.pumpWidget(Stack( await tester.pumpWidget(Stack(
...@@ -501,7 +855,173 @@ void main() { ...@@ -501,7 +855,173 @@ void main() {
], ],
)); ));
FlutterError.onError = oldHandler; FlutterError.onError = oldHandler;
expect(count, 2); expect(count, 1);
});
testWidgets('GlobalKey duplication 18 - subtree build duplicate key with same type', (WidgetTester tester) async {
final Key key = GlobalKey(debugLabel: 'problematic');
final Stack stack = Stack(
textDirection: TextDirection.ltr,
children: <Widget>[
const SwapKeyWidget(childKey: ValueKey<int>(0)),
Container(key: const ValueKey<int>(1)),
Container(key: key),
],
);
await tester.pumpWidget(stack);
final SwapKeyWidgetState state = tester.state(find.byType(SwapKeyWidget));
state.swapKey(key);
await tester.pump();
final dynamic exception = tester.takeException();
expect(exception, isFlutterError);
expect(
exception.toString(),
equalsIgnoringHashCodes(
'Duplicate GlobalKey detected in widget tree.\n'
'The following GlobalKey was specified multiple times in the widget tree. This will lead '
'to parts of the widget tree being truncated unexpectedly, because the second time a key is seen, the '
'previous instance is moved to the new location. The key was:\n'
'- [GlobalKey#00000 problematic]\n'
'This was determined by noticing that after the widget with the above global key was '
'moved out of its previous parent, that previous parent never updated during this frame, meaning that '
'it either did not update at all or updated before the widget was moved, in either case implying that '
'it still thinks that it should have a child with that global key.\n'
'The specific parent that did not update after having one or more children forcibly '
'removed due to GlobalKey reparenting is:\n'
'- Stack(alignment: AlignmentDirectional.topStart, textDirection: ltr, fit: loose, '
'overflow: clip, renderObject: RenderStack#00000)\n'
'A GlobalKey can only be specified on one widget at a time in the widget tree.'
),
);
});
testWidgets('GlobalKey duplication 19 - subtree build duplicate key with different types', (WidgetTester tester) async {
final Key key = GlobalKey(debugLabel: 'problematic');
final Stack stack = Stack(
textDirection: TextDirection.ltr,
children: <Widget>[
const SwapKeyWidget(childKey: ValueKey<int>(0)),
Container(key: const ValueKey<int>(1)),
Container(child: SizedBox(key: key)),
],
);
await tester.pumpWidget(stack);
final SwapKeyWidgetState state = tester.state(find.byType(SwapKeyWidget));
state.swapKey(key);
await tester.pump();
final dynamic exception = tester.takeException();
expect(exception, isFlutterError);
expect(
exception.toString(),
equalsIgnoringHashCodes(
'Multiple widgets used the same GlobalKey.\n'
'The key [GlobalKey#95367 problematic] was used by 2 widgets:\n'
' SizedBox-[GlobalKey#00000 problematic]\n'
' Container-[GlobalKey#00000 problematic]\n'
'A GlobalKey can only be specified on one widget at a time in the widget tree.'
),
);
});
testWidgets('GlobalKey duplication 20 - real duplication with early rebuild in layoutbuilder will throw', (WidgetTester tester) async {
const Key key1 = GlobalObjectKey('Text1');
const Key key2 = GlobalObjectKey('Text2');
Key rebuiltKeyOfSecondChildBeforeLayout;
Key rebuiltKeyOfFirstChildAfterLayout;
Key rebuiltKeyOfSecondChildAfterLayout;
await tester.pumpWidget(
LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return Column(
children: <Widget>[
const _Stateful(
child: Text(
'Text1',
textDirection: TextDirection.ltr,
key: key1,
),
),
_Stateful(
child: const Text(
'Text2',
textDirection: TextDirection.ltr,
key: key2,
),
onElementRebuild: (StatefulElement element) {
// We don't want noise to override the result;
expect(rebuiltKeyOfSecondChildBeforeLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfSecondChildBeforeLayout = statefulWidget.child.key;
},
),
],
);
},
)
);
// Result will be written during first build and need to clear it to remove
// noise.
rebuiltKeyOfSecondChildBeforeLayout = null;
final _StatefulState state = tester.firstState(find.byType(_Stateful).at(1));
state.rebuild();
await tester.pumpWidget(
LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return Column(
children: <Widget>[
_Stateful(
child: const Text(
'Text2',
textDirection: TextDirection.ltr,
key: key2,
),
onElementRebuild: (StatefulElement element) {
// Verifies the early rebuild happens before layout.
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
// We don't want noise to override the result;
expect(rebuiltKeyOfFirstChildAfterLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfFirstChildAfterLayout = statefulWidget.child.key;
},
),
_Stateful(
child: const Text(
'Text1',
textDirection: TextDirection.ltr,
key: key2,
),
onElementRebuild: (StatefulElement element) {
// Verifies the early rebuild happens before layout.
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
// We don't want noise to override the result;
expect(rebuiltKeyOfSecondChildAfterLayout, isNull);
final _Stateful statefulWidget = element.widget as _Stateful;
rebuiltKeyOfSecondChildAfterLayout = statefulWidget.child.key;
},
),
],
);
},
)
);
expect(rebuiltKeyOfSecondChildBeforeLayout, key2);
expect(rebuiltKeyOfFirstChildAfterLayout, key2);
expect(rebuiltKeyOfSecondChildAfterLayout, key2);
final dynamic exception = tester.takeException();
expect(exception, isFlutterError);
expect(
exception.toString(),
equalsIgnoringHashCodes(
'Multiple widgets used the same GlobalKey.\n'
'The key [GlobalObjectKey String#00000] was used by multiple widgets. The '
'parents of those widgets were:\n'
'- _Stateful(state: _StatefulState#00000)\n'
'- _Stateful(state: _StatefulState#00000)\n'
'A GlobalKey can only be specified on one widget at a time in the widget tree.'
),
);
}); });
testWidgets('GlobalKey - dettach and re-attach child to different parents', (WidgetTester tester) async { testWidgets('GlobalKey - dettach and re-attach child to different parents', (WidgetTester tester) async {
...@@ -758,9 +1278,6 @@ class NullChildElement extends Element { ...@@ -758,9 +1278,6 @@ class NullChildElement extends Element {
visitor(null); visitor(null);
} }
@override
void forgetChild(Element child) { }
@override @override
void performRebuild() { } void performRebuild() { }
} }
...@@ -772,9 +1289,6 @@ class DirtyElementWithCustomBuildOwner extends Element { ...@@ -772,9 +1289,6 @@ class DirtyElementWithCustomBuildOwner extends Element {
final BuildOwner _owner; final BuildOwner _owner;
@override
void forgetChild(Element child) {}
@override @override
void performRebuild() {} void performRebuild() {}
...@@ -823,3 +1337,66 @@ class DependentState extends State<DependentStatefulWidget> { ...@@ -823,3 +1337,66 @@ class DependentState extends State<DependentStatefulWidget> {
deactivatedCount += 1; deactivatedCount += 1;
} }
} }
class SwapKeyWidget extends StatefulWidget {
const SwapKeyWidget({this.childKey}): super();
final Key childKey;
@override
SwapKeyWidgetState createState() => SwapKeyWidgetState();
}
class SwapKeyWidgetState extends State<SwapKeyWidget> {
Key key;
@override
void initState() {
super.initState();
key = widget.childKey;
}
void swapKey(Key newKey) {
setState(() {
key = newKey;
});
}
@override
Widget build(BuildContext context) {
return Container(key: key);
}
}
class _Stateful extends StatefulWidget {
const _Stateful({Key key, this.child, this.onElementRebuild}) : super(key: key);
final Text child;
final ElementRebuildCallback onElementRebuild;
@override
State<StatefulWidget> createState() => _StatefulState();
@override
StatefulElement createElement() => StatefulElementSpy(this);
}
class _StatefulState extends State<_Stateful> {
void rebuild() => setState(() {});
@override
Widget build(BuildContext context) {
return widget.child;
}
}
class StatefulElementSpy extends StatefulElement {
StatefulElementSpy(StatefulWidget widget) : super(widget);
_Stateful get _statefulWidget => widget as _Stateful;
@override
void rebuild() {
if (_statefulWidget.onElementRebuild != null) {
_statefulWidget.onElementRebuild(this);
}
super.rebuild();
}
}
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

HIAST - GitLab