Skip to content

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

Reland fix 25807 implement move for sliver multibox widget (#31978)

parent 39d660be
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 1125 additions and 67 deletions
packages/flutter/lib/src/material/tabs.dart
View file @ 38808d9f
......@@ -1125,6 +1125,7 @@ class _TabBarViewState extends State<TabBarView> {
TabController _controller;
PageController _pageController;
List<Widget> _children;
List<Widget> _childrenWithKey;
int _currentIndex;
int _warpUnderwayCount = 0;
......@@ -1156,7 +1157,7 @@ class _TabBarViewState extends State<TabBarView> {
@override
void initState() {
super.initState();
_children = widget.children;
_updateChildren();
}
@override
......@@ -1173,7 +1174,7 @@ class _TabBarViewState extends State<TabBarView> {
if (widget.controller != oldWidget.controller)
_updateTabController();
if (widget.children != oldWidget.children && _warpUnderwayCount == 0)
_children = widget.children;
_updateChildren();
}
@override
......@@ -1184,6 +1185,11 @@ class _TabBarViewState extends State<TabBarView> {
super.dispose();
}
void _updateChildren() {
_children = widget.children;
_childrenWithKey = KeyedSubtree.ensureUniqueKeysForList(widget.children);
}
void _handleTabControllerAnimationTick() {
if (_warpUnderwayCount > 0 || !_controller.indexIsChanging)
return; // This widget is driving the controller's animation.
......@@ -1206,28 +1212,30 @@ class _TabBarViewState extends State<TabBarView> {
return _pageController.animateToPage(_currentIndex, duration: kTabScrollDuration, curve: Curves.ease);
assert((_currentIndex - previousIndex).abs() > 1);
int initialPage;
final int initialPage = _currentIndex > previousIndex
? _currentIndex - 1
: _currentIndex + 1;
final List<Widget> originalChildren = _childrenWithKey;
setState(() {
_warpUnderwayCount += 1;
_children = List<Widget>.from(widget.children, growable: false);
if (_currentIndex > previousIndex) {
_children[_currentIndex - 1] = _children[previousIndex];
initialPage = _currentIndex - 1;
} else {
_children[_currentIndex + 1] = _children[previousIndex];
initialPage = _currentIndex + 1;
}
});
_childrenWithKey = List<Widget>.from(_childrenWithKey, growable: false);
final Widget temp = _childrenWithKey[initialPage];
_childrenWithKey[initialPage] = _childrenWithKey[previousIndex];
_childrenWithKey[previousIndex] = temp;
});
_pageController.jumpToPage(initialPage);
await _pageController.animateToPage(_currentIndex, duration: kTabScrollDuration, curve: Curves.ease);
if (!mounted)
return Future<void>.value();
setState(() {
_warpUnderwayCount -= 1;
_children = widget.children;
if (widget.children != _children) {
_updateChildren();
} else {
_childrenWithKey = originalChildren;
}
});
}
......@@ -1272,7 +1280,7 @@ class _TabBarViewState extends State<TabBarView> {
dragStartBehavior: widget.dragStartBehavior,
controller: _pageController,
physics: widget.physics == null ? _kTabBarViewPhysics : _kTabBarViewPhysics.applyTo(widget.physics),
children: _children,
children: _childrenWithKey,
),
);
}
......
packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart
View file @ 38808d9f
......@@ -85,7 +85,8 @@ abstract class RenderSliverBoxChildManager {
/// list).
int get childCount;
/// Called during [RenderSliverMultiBoxAdaptor.adoptChild].
/// Called during [RenderSliverMultiBoxAdaptor.adoptChild] or
/// [RenderSliverMultiBoxAdaptor.move].
///
/// Subclasses must ensure that the [SliverMultiBoxAdaptorParentData.index]
/// field of the child's [RenderObject.parentData] accurately reflects the
......@@ -193,7 +194,12 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
RenderSliverMultiBoxAdaptor({
@required RenderSliverBoxChildManager childManager,
}) : assert(childManager != null),
_childManager = childManager;
_childManager = childManager {
assert(() {
_debugDanglingKeepAlives = <RenderBox>[];
return true;
}());
}
@override
void setupParentData(RenderObject child) {
......@@ -214,6 +220,27 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
/// The nodes being kept alive despite not being visible.
final Map<int, RenderBox> _keepAliveBucket = <int, RenderBox>{};
List<RenderBox> _debugDanglingKeepAlives;
/// Indicates whether integrity check is enabled.
///
/// Setting this property to true will immediately perform an integrity check.
///
/// The integrity check consists of:
///
/// 1. Verify that the children index in childList is in ascending order.
/// 2. Verify that there is no dangling keepalive child as the result of [move].
bool get debugChildIntegrityEnabled => _debugChildIntegrityEnabled;
bool _debugChildIntegrityEnabled = true;
set debugChildIntegrityEnabled(bool enabled) {
assert(enabled != null);
assert(() {
_debugChildIntegrityEnabled = enabled;
return _debugVerifyChildOrder() &&
(!_debugChildIntegrityEnabled || _debugDanglingKeepAlives.isEmpty);
}());
}
@override
void adoptChild(RenderObject child) {
super.adoptChild(child);
......@@ -224,21 +251,70 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
bool _debugAssertChildListLocked() => childManager.debugAssertChildListLocked();
/// Verify that the child list index is in strictly increasing order.
///
/// This has no effect in release builds.
bool _debugVerifyChildOrder(){
if (_debugChildIntegrityEnabled) {
RenderBox child = firstChild;
int index;
while (child != null) {
index = indexOf(child);
child = childAfter(child);
assert(child == null || indexOf(child) > index);
}
}
return true;
}
@override
void insert(RenderBox child, { RenderBox after }) {
assert(!_keepAliveBucket.containsValue(child));
super.insert(child, after: after);
assert(firstChild != null);
assert(() {
int index = indexOf(firstChild);
RenderBox child = childAfter(firstChild);
while (child != null) {
assert(indexOf(child) > index);
index = indexOf(child);
child = childAfter(child);
assert(_debugVerifyChildOrder());
}
@override
void move(RenderBox child, { RenderBox after }) {
// There are two scenarios:
//
// 1. The child is not keptAlive.
// The child is in the childList maintained by ContainerRenderObjectMixin.
// We can call super.move and update parentData with the new slot.
//
// 2. The child is keptAlive.
// In this case, the child is no longer in the childList but might be stored in
// [_keepAliveBucket]. We need to update the location of the child in the bucket.
final SliverMultiBoxAdaptorParentData childParentData = child.parentData;
if (!childParentData.keptAlive) {
super.move(child, after: after);
childManager.didAdoptChild(child); // updates the slot in the parentData
// Its slot may change even if super.move does not change the position.
// In this case, we still want to mark as needs layout.
markNeedsLayout();
} else {
// If the child in the bucket is not current child, that means someone has
// already moved and replaced current child, and we cannot remove this child.
if (_keepAliveBucket[childParentData.index] == child) {
_keepAliveBucket.remove(childParentData.index);
}
return true;
}());
assert(() {
_debugDanglingKeepAlives.remove(child);
return true;
}());
// Update the slot and reinsert back to _keepAliveBucket in the new slot.
childManager.didAdoptChild(child);
// If there is an existing child in the new slot, that mean that child will
// be moved to other index. In other cases, the existing child should have been
// removed by updateChild. Thus, it is ok to overwrite it.
assert(() {
if (_keepAliveBucket.containsKey(childParentData.index))
_debugDanglingKeepAlives.add(_keepAliveBucket[childParentData.index]);
return true;
}());
_keepAliveBucket[childParentData.index] = child;
}
}
@override
......@@ -249,6 +325,10 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
return;
}
assert(_keepAliveBucket[childParentData.index] == child);
assert(() {
_debugDanglingKeepAlives.remove(child);
return true;
}());
_keepAliveBucket.remove(childParentData.index);
dropChild(child);
}
......
packages/flutter/lib/src/widgets/framework.dart
View file @ 38808d9f
......@@ -149,6 +149,13 @@ abstract class GlobalKey<T extends State<StatefulWidget>> extends Key {
assert(() {
assert(parent != null);
if (_debugReservations.containsKey(this) && _debugReservations[this] != parent) {
// Reserving a new parent while the old parent is not attached is ok.
// This can happen when a renderObject detaches and re-attaches to rendering
// tree multiple times.
if (_debugReservations[this].renderObject?.attached == false) {
_debugReservations[this] = parent;
return true;
}
// It's possible for an element to get built multiple times in one
// frame, in which case it'll reserve the same child's key multiple
// times. We catch multiple children of one widget having the same key
......
packages/flutter/test/material/tabs_test.dart
View file @ 38808d9f
......@@ -49,6 +49,7 @@ class StateMarkerState extends State<StateMarker> {
}
class AlwaysKeepAliveWidget extends StatefulWidget {
const AlwaysKeepAliveWidget({ Key key}) : super(key: key);
static String text = 'AlwaysKeepAlive';
@override
AlwaysKeepAliveState createState() => AlwaysKeepAliveState();
......@@ -1987,6 +1988,56 @@ void main() {
});
testWidgets('Skipping tabs with global key does not crash', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/24660
final List<String> tabs = <String>[
'Tab1',
'Tab2',
'Tab3',
'Tab4',
];
final TabController controller = TabController(
vsync: const TestVSync(),
length: tabs.length,
);
await tester.pumpWidget(
MaterialApp(
home: Align(
alignment: Alignment.topLeft,
child: SizedBox(
width: 300.0,
height: 200.0,
child: Scaffold(
appBar: AppBar(
title: const Text('tabs'),
bottom: TabBar(
controller: controller,
tabs: tabs.map<Widget>((String tab) => Tab(text: tab)).toList(),
),
),
body: TabBarView(
controller: controller,
children: <Widget>[
Text('1', key: GlobalKey()),
Text('2', key: GlobalKey()),
Text('3', key: GlobalKey()),
Text('4', key: GlobalKey()),
],
),
),
),
),
),
);
expect(find.text('1'), findsOneWidget);
expect(find.text('4'), findsNothing);
await tester.tap(find.text('Tab4'));
await tester.pumpAndSettle();
expect(controller.index, 3);
expect(find.text('4'), findsOneWidget);
expect(find.text('1'), findsNothing);
});
testWidgets('Skipping tabs with a KeepAlive child works', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/11895
final List<String> tabs = <String>[
......@@ -2018,7 +2069,7 @@ void main() {
body: TabBarView(
controller: controller,
children: <Widget>[
AlwaysKeepAliveWidget(),
AlwaysKeepAliveWidget(key: UniqueKey()),
const Text('2'),
const Text('3'),
const Text('4'),
......
packages/flutter/test/widgets/box_sliver_mismatch_test.dart
View file @ 38808d9f
......@@ -9,10 +9,10 @@ import 'package:flutter/widgets.dart';
void main() {
testWidgets('Sliver in a box', (WidgetTester tester) async {
await tester.pumpWidget(
const DecoratedBox(
decoration: BoxDecoration(),
DecoratedBox(
decoration: const BoxDecoration(),
child: SliverList(
delegate: SliverChildListDelegate(<Widget>[]),
delegate: SliverChildListDelegate(const <Widget>[]),
),
),
);
......@@ -21,9 +21,9 @@ void main() {
await tester.pumpWidget(
Row(
children: const <Widget>[
children: <Widget>[
SliverList(
delegate: SliverChildListDelegate(<Widget>[]),
delegate: SliverChildListDelegate(const <Widget>[]),
),
],
),
......
packages/flutter/test/widgets/framework_test.dart
View file @ 38808d9f
......@@ -451,6 +451,41 @@ void main() {
expect(count, 2);
});
testWidgets('GlobalKey - dettach and re-attach child to different parents', (WidgetTester tester) async {
await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: Container(
height: 100,
child: CustomScrollView(
controller: ScrollController(),
slivers: <Widget>[
SliverList(
delegate: SliverChildListDelegate(<Widget>[
Text('child', key: GlobalKey()),
]),
)
],
),
),
),
));
final SliverMultiBoxAdaptorElement element = tester.element(find.byType(SliverList));
Element childElement;
// Removing and recreating child with same Global Key should not trigger
// duplicate key error.
element.visitChildren((Element e) {
childElement = e;
});
element.removeChild(childElement.renderObject);
element.createChild(0, after: null);
element.visitChildren((Element e) {
childElement = e;
});
element.removeChild(childElement.renderObject);
element.createChild(0, after: null);
});
testWidgets('Defunct setState throws exception', (WidgetTester tester) async {
StateSetter setState;
......
packages/flutter/test/widgets/slivers_appbar_floating_pinned_test.dart
View file @ 38808d9f
......@@ -22,12 +22,12 @@ void main() {
data: const MediaQueryData(),
child: CustomScrollView(
controller: controller,
slivers: const <Widget>[
SliverAppBar(floating: true, pinned: true, expandedHeight: 200.0, title: Text('A')),
SliverAppBar(primary: false, pinned: true, title: Text('B')),
slivers: <Widget>[
const SliverAppBar(floating: true, pinned: true, expandedHeight: 200.0, title: Text('A')),
const SliverAppBar(primary: false, pinned: true, title: Text('B')),
SliverList(
delegate: SliverChildListDelegate(
<Widget>[
const <Widget>[
Text('C'),
Text('D'),
SizedBox(height: 500.0),
......
packages/flutter/test/widgets/slivers_appbar_floating_test.dart
View file @ 38808d9f
......@@ -203,9 +203,9 @@ void main() {
physics: const BouncingScrollPhysics(),
slivers: <Widget>[
SliverPersistentHeader(delegate: TestDelegate(), floating: true),
const SliverList(
SliverList(
delegate: SliverChildListDelegate(<Widget>[
SizedBox(
const SizedBox(
height: 300.0,
child: Text('X'),
),
......
packages/flutter/test/widgets/slivers_appbar_pinned_test.dart
View file @ 38808d9f
......@@ -258,9 +258,9 @@ void main() {
physics: const BouncingScrollPhysics(),
slivers: <Widget>[
SliverPersistentHeader(delegate: TestDelegate(), pinned: true),
const SliverList(
SliverList(
delegate: SliverChildListDelegate(<Widget>[
SizedBox(
const SizedBox(
height: 300.0,
child: Text('X'),
),
......
packages/flutter/test/widgets/slivers_appbar_scrolling_test.dart
View file @ 38808d9f
......@@ -82,8 +82,40 @@ void main() {
physics: const BouncingScrollPhysics(),
slivers: <Widget>[
SliverPersistentHeader(delegate: TestDelegate()),
const SliverList(
SliverList(
delegate: SliverChildListDelegate(<Widget>[
const SizedBox(
height: 300.0,
child: Text('X'),
),
]),
),
],
),
),
);
expect(tester.getTopLeft(find.byType(Container)), Offset.zero);
expect(tester.getTopLeft(find.text('X')), const Offset(0.0, 200.0));
final ScrollPosition position = tester.state<ScrollableState>(find.byType(Scrollable)).position;
position.jumpTo(-50.0);
await tester.pump();
expect(tester.getTopLeft(find.byType(Container)), Offset.zero);
expect(tester.getTopLeft(find.text('X')), const Offset(0.0, 250.0));
});
testWidgets('Sliver appbars const child delegate - scrolling - overscroll gap is below header', (WidgetTester tester) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: CustomScrollView(
physics: const BouncingScrollPhysics(),
slivers: <Widget>[
SliverPersistentHeader(delegate: TestDelegate()),
const SliverList(
delegate: SliverChildListDelegate.fixed(<Widget>[
SizedBox(
height: 300.0,
child: Text('X'),
......
packages/flutter/test/widgets/slivers_block_test.dart
View file @ 38808d9f
......@@ -9,6 +9,28 @@ import 'package:flutter/widgets.dart';
import '../rendering/mock_canvas.dart';
Future<void> test(WidgetTester tester, double offset) {
return tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Viewport(
offset: ViewportOffset.fixed(offset),
slivers: <Widget>[
SliverList(
delegate: SliverChildListDelegate(const <Widget>[
SizedBox(height: 400.0, child: Text('a')),
SizedBox(height: 400.0, child: Text('b')),
SizedBox(height: 400.0, child: Text('c')),
SizedBox(height: 400.0, child: Text('d')),
SizedBox(height: 400.0, child: Text('e')),
]),
),
],
),
),
);
}
Future<void> testWithConstChildDelegate(WidgetTester tester, double offset) {
return tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
......@@ -16,7 +38,7 @@ Future<void> test(WidgetTester tester, double offset) {
offset: ViewportOffset.fixed(offset),
slivers: const <Widget>[
SliverList(
delegate: SliverChildListDelegate(<Widget>[
delegate: SliverChildListDelegate.fixed(<Widget>[
SizedBox(height: 400.0, child: Text('a')),
SizedBox(height: 400.0, child: Text('b')),
SizedBox(height: 400.0, child: Text('c')),
......@@ -76,6 +98,39 @@ void main() {
], 'ab');
});
testWidgets('Viewport+SliverBlock basic test with constant SliverChildListDelegate', (WidgetTester tester) async {
await testWithConstChildDelegate(tester, 0.0);
expect(tester.renderObject<RenderBox>(find.byType(Viewport)).size, equals(const Size(800.0, 600.0)));
verify(tester, <Offset>[
const Offset(0.0, 0.0),
const Offset(0.0, 400.0),
], 'ab');
await testWithConstChildDelegate(tester, 200.0);
verify(tester, <Offset>[
const Offset(0.0, -200.0),
const Offset(0.0, 200.0),
], 'ab');
await testWithConstChildDelegate(tester, 600.0);
verify(tester, <Offset>[
const Offset(0.0, -200.0),
const Offset(0.0, 200.0),
], 'bc');
await testWithConstChildDelegate(tester, 900.0);
verify(tester, <Offset>[
const Offset(0.0, -100.0),
const Offset(0.0, 300.0),
], 'cd');
await testWithConstChildDelegate(tester, 200.0);
verify(tester, <Offset>[
const Offset(0.0, -200.0),
const Offset(0.0, 200.0),
], 'ab');
});
testWidgets('Viewport with GlobalKey reparenting', (WidgetTester tester) async {
final Key key1 = GlobalKey();
final ViewportOffset offset = ViewportOffset.zero();
......@@ -150,9 +205,9 @@ void main() {
textDirection: TextDirection.ltr,
child: Viewport(
offset: offset,
slivers: const <Widget>[
slivers: <Widget>[
SliverList(
delegate: SliverChildListDelegate(<Widget>[
delegate: SliverChildListDelegate(const <Widget>[
SizedBox(height: 251.0, child: Text('a')),
SizedBox(height: 252.0, child: Text('b')),
]),
......@@ -261,9 +316,9 @@ void main() {
textDirection: TextDirection.ltr,
child: Viewport(
offset: ViewportOffset.zero(),
slivers: const <Widget>[
slivers: <Widget>[
SliverList(
delegate: SliverChildListDelegate(<Widget>[
delegate: SliverChildListDelegate(const <Widget>[
SizedBox(height: 400.0, child: Text('a')),
]),
),
......@@ -279,9 +334,9 @@ void main() {
textDirection: TextDirection.ltr,
child: Viewport(
offset: ViewportOffset.fixed(100.0),
slivers: const <Widget>[
slivers: <Widget>[
SliverList(
delegate: SliverChildListDelegate(<Widget>[
delegate: SliverChildListDelegate(const <Widget>[
SizedBox(height: 400.0, child: Text('a')),
]),
),
......@@ -297,9 +352,9 @@ void main() {
textDirection: TextDirection.ltr,
child: Viewport(
offset: ViewportOffset.fixed(100.0),
slivers: const <Widget>[
slivers: <Widget>[
SliverList(
delegate: SliverChildListDelegate(<Widget>[
delegate: SliverChildListDelegate(const <Widget>[
SizedBox(height: 4000.0, child: Text('a')),
]),
),
......@@ -315,9 +370,9 @@ void main() {
textDirection: TextDirection.ltr,
child: Viewport(
offset: ViewportOffset.zero(),
slivers: const <Widget>[
slivers: <Widget>[
SliverList(
delegate: SliverChildListDelegate(<Widget>[
delegate: SliverChildListDelegate(const <Widget>[
SizedBox(height: 4000.0, child: Text('a')),
]),
),
......
packages/flutter/test/widgets/slivers_keepalive_test.dart 0 → 100644
View file @ 38808d9f
This diff is collapsed.
packages/flutter/test/widgets/slivers_test.dart
View file @ 38808d9f
......@@ -206,7 +206,8 @@ void main() {
addRepaintBoundaries: false,
addSemanticIndexes: false,
);
expect(builderThrowsDelegate.build(null, 0), errorText);
final KeyedSubtree wrapped = builderThrowsDelegate.build(null, 0);
expect(wrapped.child, errorText);
expect(tester.takeException(), 'builder');
ErrorWidget.builder = oldBuilder;
});
......
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