Unverified Commit 77b4505c authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Fix a crash when disposing tabs (#63142)

parent ae432ca8
...@@ -220,8 +220,10 @@ class TabController extends ChangeNotifier { ...@@ -220,8 +220,10 @@ class TabController extends ChangeNotifier {
_animationController _animationController
.animateTo(_index.toDouble(), duration: duration, curve: curve) .animateTo(_index.toDouble(), duration: duration, curve: curve)
.whenCompleteOrCancel(() { .whenCompleteOrCancel(() {
_indexIsChangingCount -= 1; if (_animationController != null) { // don't notify if we've been disposed
notifyListeners(); _indexIsChangingCount -= 1;
notifyListeners();
}
}); });
} else { } else {
_indexIsChangingCount += 1; _indexIsChangingCount += 1;
......
...@@ -1199,8 +1199,6 @@ class TabBarView extends StatefulWidget { ...@@ -1199,8 +1199,6 @@ class TabBarView extends StatefulWidget {
_TabBarViewState createState() => _TabBarViewState(); _TabBarViewState createState() => _TabBarViewState();
} }
const PageScrollPhysics _kTabBarViewPhysics = PageScrollPhysics();
class _TabBarViewState extends State<TabBarView> { class _TabBarViewState extends State<TabBarView> {
TabController _controller; TabController _controller;
PageController _pageController; PageController _pageController;
...@@ -1370,8 +1368,8 @@ class _TabBarViewState extends State<TabBarView> { ...@@ -1370,8 +1368,8 @@ class _TabBarViewState extends State<TabBarView> {
dragStartBehavior: widget.dragStartBehavior, dragStartBehavior: widget.dragStartBehavior,
controller: _pageController, controller: _pageController,
physics: widget.physics == null physics: widget.physics == null
? _kTabBarViewPhysics.applyTo(const ClampingScrollPhysics()) ? const PageScrollPhysics().applyTo(const ClampingScrollPhysics())
: _kTabBarViewPhysics.applyTo(widget.physics), : const PageScrollPhysics().applyTo(widget.physics),
children: _childrenWithKey, children: _childrenWithKey,
), ),
); );
......
...@@ -428,8 +428,9 @@ class RangeMaintainingScrollPhysics extends ScrollPhysics { ...@@ -428,8 +428,9 @@ class RangeMaintainingScrollPhysics extends ScrollPhysics {
@required bool isScrolling, @required bool isScrolling,
@required double velocity, @required double velocity,
}) { }) {
if (velocity != 0.0 || ((oldPosition.minScrollExtent == newPosition.minScrollExtent) && (oldPosition.maxScrollExtent == newPosition.maxScrollExtent))) if (velocity != 0.0 || ((oldPosition.minScrollExtent == newPosition.minScrollExtent) && (oldPosition.maxScrollExtent == newPosition.maxScrollExtent))) {
return super.adjustPositionForNewDimensions(oldPosition: oldPosition, newPosition: newPosition, isScrolling: isScrolling, velocity: velocity); return super.adjustPositionForNewDimensions(oldPosition: oldPosition, newPosition: newPosition, isScrolling: isScrolling, velocity: velocity);
}
if (oldPosition.pixels < oldPosition.minScrollExtent) { if (oldPosition.pixels < oldPosition.minScrollExtent) {
final double oldDelta = oldPosition.minScrollExtent - oldPosition.pixels; final double oldDelta = oldPosition.minScrollExtent - oldPosition.pixels;
return newPosition.minScrollExtent - oldDelta; return newPosition.minScrollExtent - oldDelta;
......
...@@ -432,55 +432,6 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { ...@@ -432,55 +432,6 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
return true; return true;
} }
Set<SemanticsAction> _semanticActions;
/// Called whenever the scroll position or the dimensions of the scroll view
/// change to schedule an update of the available semantics actions. The
/// actual update will be performed in the next frame. If non is pending
/// a frame will be scheduled.
///
/// For example: If the scroll view has been scrolled all the way to the top,
/// the action to scroll further up needs to be removed as the scroll view
/// cannot be scrolled in that direction anymore.
///
/// This method is potentially called twice per frame (if scroll position and
/// scroll view dimensions both change) and therefore shouldn't do anything
/// expensive.
void _updateSemanticActions() {
SemanticsAction forward;
SemanticsAction backward;
switch (axisDirection) {
case AxisDirection.up:
forward = SemanticsAction.scrollDown;
backward = SemanticsAction.scrollUp;
break;
case AxisDirection.right:
forward = SemanticsAction.scrollLeft;
backward = SemanticsAction.scrollRight;
break;
case AxisDirection.down:
forward = SemanticsAction.scrollUp;
backward = SemanticsAction.scrollDown;
break;
case AxisDirection.left:
forward = SemanticsAction.scrollRight;
backward = SemanticsAction.scrollLeft;
break;
}
final Set<SemanticsAction> actions = <SemanticsAction>{};
if (pixels > minScrollExtent)
actions.add(backward);
if (pixels < maxScrollExtent)
actions.add(forward);
if (setEquals<SemanticsAction>(actions, _semanticActions))
return;
_semanticActions = actions;
context.setSemanticsActions(_semanticActions);
}
@override @override
bool applyContentDimensions(double minScrollExtent, double maxScrollExtent) { bool applyContentDimensions(double minScrollExtent, double maxScrollExtent) {
assert(minScrollExtent != null); assert(minScrollExtent != null);
...@@ -560,6 +511,55 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { ...@@ -560,6 +511,55 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
_updateSemanticActions(); // will potentially request a semantics update. _updateSemanticActions(); // will potentially request a semantics update.
} }
Set<SemanticsAction> _semanticActions;
/// Called whenever the scroll position or the dimensions of the scroll view
/// change to schedule an update of the available semantics actions. The
/// actual update will be performed in the next frame. If non is pending
/// a frame will be scheduled.
///
/// For example: If the scroll view has been scrolled all the way to the top,
/// the action to scroll further up needs to be removed as the scroll view
/// cannot be scrolled in that direction anymore.
///
/// This method is potentially called twice per frame (if scroll position and
/// scroll view dimensions both change) and therefore shouldn't do anything
/// expensive.
void _updateSemanticActions() {
SemanticsAction forward;
SemanticsAction backward;
switch (axisDirection) {
case AxisDirection.up:
forward = SemanticsAction.scrollDown;
backward = SemanticsAction.scrollUp;
break;
case AxisDirection.right:
forward = SemanticsAction.scrollLeft;
backward = SemanticsAction.scrollRight;
break;
case AxisDirection.down:
forward = SemanticsAction.scrollUp;
backward = SemanticsAction.scrollDown;
break;
case AxisDirection.left:
forward = SemanticsAction.scrollRight;
backward = SemanticsAction.scrollLeft;
break;
}
final Set<SemanticsAction> actions = <SemanticsAction>{};
if (pixels > minScrollExtent)
actions.add(backward);
if (pixels < maxScrollExtent)
actions.add(forward);
if (setEquals<SemanticsAction>(actions, _semanticActions))
return;
_semanticActions = actions;
context.setSemanticsActions(_semanticActions);
}
/// Animates the position such that the given object is as visible as possible /// Animates the position such that the given object is as visible as possible
/// by just scrolling this position. /// by just scrolling this position.
/// ///
......
...@@ -2670,6 +2670,13 @@ void main() { ...@@ -2670,6 +2670,13 @@ void main() {
final PageView pageView = tester.widget<PageView>(find.byType(PageView)); final PageView pageView = tester.widget<PageView>(find.byType(PageView));
expect(pageView.physics.toString().contains('ClampingScrollPhysics'), isFalse); expect(pageView.physics.toString().contains('ClampingScrollPhysics'), isFalse);
}); });
testWidgets('Crash on dispose', (WidgetTester tester) async {
await tester.pumpWidget(Padding(padding: const EdgeInsets.only(right: 200.0), child: TabBarDemo()));
await tester.tap(find.byIcon(Icons.directions_bike));
// There was a time where this would throw an exception
// because we tried to send a notification on dispose.
});
} }
class KeepAliveInk extends StatefulWidget { class KeepAliveInk extends StatefulWidget {
...@@ -2693,3 +2700,33 @@ class _KeepAliveInkState extends State<KeepAliveInk> with AutomaticKeepAliveClie ...@@ -2693,3 +2700,33 @@ class _KeepAliveInkState extends State<KeepAliveInk> with AutomaticKeepAliveClie
@override @override
bool get wantKeepAlive => true; bool get wantKeepAlive => true;
} }
class TabBarDemo extends StatelessWidget {
@override
Widget build(BuildContext context) {
return MaterialApp(
home: DefaultTabController(
length: 3,
child: Scaffold(
appBar: AppBar(
bottom: const TabBar(
tabs: <Widget>[
Tab(icon: Icon(Icons.directions_car)),
Tab(icon: Icon(Icons.directions_transit)),
Tab(icon: Icon(Icons.directions_bike)),
],
),
title: const Text('Tabs Demo'),
),
body: const TabBarView(
children: <Widget>[
Icon(Icons.directions_car),
Icon(Icons.directions_transit),
Icon(Icons.directions_bike),
],
),
),
),
);
}
}
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