From fe8e882a484335178e38d77977870ca51a50f2d5 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Tue, 9 Nov 2021 11:06:32 -0800 Subject: [PATCH] Do not rebuild when TickerMode changes (#93166) --- .../lib/src/widgets/editable_text.dart | 54 +++--- .../lib/src/widgets/ticker_provider.dart | 164 ++++++++++++++--- .../test/widgets/ticker_mode_test.dart | 171 +++++++++++++++++- 3 files changed, 333 insertions(+), 56 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index c7fa74d90b..3d6481e526 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -1524,10 +1524,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien ScrollController? _internalScrollController; ScrollController get _scrollController => widget.scrollController ?? (_internalScrollController ??= ScrollController()); - late final AnimationController _cursorBlinkOpacityController = AnimationController( - vsync: this, - duration: _fadeDuration, - )..addListener(_onCursorColorTick); + AnimationController? _cursorBlinkOpacityController; final LayerLink _toolbarLayerLink = LayerLink(); final LayerLink _startHandleLayerLink = LayerLink(); @@ -1564,14 +1561,12 @@ class EditableTextState extends State with AutomaticKeepAliveClien // cursor position after the user has finished placing it. static const Duration _floatingCursorResetTime = Duration(milliseconds: 125); - late final AnimationController _floatingCursorResetController = AnimationController( - vsync: this, - )..addListener(_onFloatingCursorResetTick); + AnimationController? _floatingCursorResetController; @override bool get wantKeepAlive => widget.focusNode.hasFocus; - Color get _cursorColor => widget.cursorColor.withOpacity(_cursorBlinkOpacityController.value); + Color get _cursorColor => widget.cursorColor.withOpacity(_cursorBlinkOpacityController!.value); @override bool get cutEnabled => widget.toolbarOptions.cut && !widget.readOnly; @@ -1698,6 +1693,10 @@ class EditableTextState extends State with AutomaticKeepAliveClien @override void initState() { super.initState(); + _cursorBlinkOpacityController = AnimationController( + vsync: this, + duration: _fadeDuration, + )..addListener(_onCursorColorTick); _clipboardStatus?.addListener(_onChangedClipboardStatus); widget.controller.addListener(_didChangeTextEditingValue); widget.focusNode.addListener(_handleFocusChanged); @@ -1808,12 +1807,14 @@ class EditableTextState extends State with AutomaticKeepAliveClien _internalScrollController?.dispose(); _currentAutofillScope?.unregister(autofillId); widget.controller.removeListener(_didChangeTextEditingValue); - _floatingCursorResetController.dispose(); + _floatingCursorResetController?.dispose(); + _floatingCursorResetController = null; _closeInputConnectionIfNeeded(); assert(!_hasInputConnection); _cursorTimer?.cancel(); _cursorTimer = null; - _cursorBlinkOpacityController.dispose(); + _cursorBlinkOpacityController?.dispose(); + _cursorBlinkOpacityController = null; _selectionOverlay?.dispose(); _selectionOverlay = null; widget.focusNode.removeListener(_handleFocusChanged); @@ -1952,10 +1953,13 @@ class EditableTextState extends State with AutomaticKeepAliveClien @override void updateFloatingCursor(RawFloatingCursorPoint point) { + _floatingCursorResetController ??= AnimationController( + vsync: this, + )..addListener(_onFloatingCursorResetTick); switch(point.state) { case FloatingCursorDragState.Start: - if (_floatingCursorResetController.isAnimating) { - _floatingCursorResetController.stop(); + if (_floatingCursorResetController!.isAnimating) { + _floatingCursorResetController!.stop(); _onFloatingCursorResetTick(); } // We want to send in points that are centered around a (0,0) origin, so @@ -1980,8 +1984,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien case FloatingCursorDragState.End: // We skip animation if no update has happened. if (_lastTextPosition != null && _lastBoundedOffset != null) { - _floatingCursorResetController.value = 0.0; - _floatingCursorResetController.animateTo(1.0, duration: _floatingCursorResetTime, curve: Curves.decelerate); + _floatingCursorResetController!.value = 0.0; + _floatingCursorResetController!.animateTo(1.0, duration: _floatingCursorResetTime, curve: Curves.decelerate); } break; } @@ -1989,7 +1993,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien void _onFloatingCursorResetTick() { final Offset finalPosition = renderEditable.getLocalRectForCaret(_lastTextPosition!).centerLeft - _floatingCursorOffset; - if (_floatingCursorResetController.isCompleted) { + if (_floatingCursorResetController!.isCompleted) { renderEditable.setFloatingCursor(FloatingCursorDragState.End, finalPosition, _lastTextPosition!); if (_lastTextPosition!.offset != renderEditable.selection!.baseOffset) // The cause is technically the force cursor, but the cause is listed as tap as the desired functionality is the same. @@ -1999,7 +2003,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien _pointOffsetOrigin = null; _lastBoundedOffset = null; } else { - final double lerpValue = _floatingCursorResetController.value; + final double lerpValue = _floatingCursorResetController!.value; final double lerpX = ui.lerpDouble(_lastBoundedOffset!.dx, finalPosition.dx, lerpValue)!; final double lerpY = ui.lerpDouble(_lastBoundedOffset!.dy, finalPosition.dy, lerpValue)!; @@ -2528,14 +2532,14 @@ class EditableTextState extends State with AutomaticKeepAliveClien } void _onCursorColorTick() { - renderEditable.cursorColor = widget.cursorColor.withOpacity(_cursorBlinkOpacityController.value); - _cursorVisibilityNotifier.value = widget.showCursor && _cursorBlinkOpacityController.value > 0; + renderEditable.cursorColor = widget.cursorColor.withOpacity(_cursorBlinkOpacityController!.value); + _cursorVisibilityNotifier.value = widget.showCursor && _cursorBlinkOpacityController!.value > 0; } /// Whether the blinking cursor is actually visible at this precise moment /// (it's hidden half the time, since it blinks). @visibleForTesting - bool get cursorCurrentlyVisible => _cursorBlinkOpacityController.value > 0; + bool get cursorCurrentlyVisible => _cursorBlinkOpacityController!.value > 0; /// The cursor blink interval (the amount of time the cursor is in the "on" /// state or the "off" state). A complete cursor blink period is twice this @@ -2561,9 +2565,9 @@ class EditableTextState extends State with AutomaticKeepAliveClien // // These values and curves have been obtained through eyeballing, so are // likely not exactly the same as the values for native iOS. - _cursorBlinkOpacityController.animateTo(targetOpacity, curve: Curves.easeOut); + _cursorBlinkOpacityController!.animateTo(targetOpacity, curve: Curves.easeOut); } else { - _cursorBlinkOpacityController.value = targetOpacity; + _cursorBlinkOpacityController!.value = targetOpacity; } if (_obscureShowCharTicksPending > 0) { @@ -2591,7 +2595,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien return; } _targetCursorVisibility = true; - _cursorBlinkOpacityController.value = 1.0; + _cursorBlinkOpacityController!.value = 1.0; if (EditableText.debugDeterministicCursor) return; if (widget.cursorOpacityAnimates) { @@ -2606,14 +2610,14 @@ class EditableTextState extends State with AutomaticKeepAliveClien _cursorTimer?.cancel(); _cursorTimer = null; _targetCursorVisibility = false; - _cursorBlinkOpacityController.value = 0.0; + _cursorBlinkOpacityController!.value = 0.0; if (EditableText.debugDeterministicCursor) return; if (resetCharTicks) _obscureShowCharTicksPending = 0; if (widget.cursorOpacityAnimates) { - _cursorBlinkOpacityController.stop(); - _cursorBlinkOpacityController.value = 0.0; + _cursorBlinkOpacityController!.stop(); + _cursorBlinkOpacityController!.value = 0.0; } } diff --git a/packages/flutter/lib/src/widgets/ticker_provider.dart b/packages/flutter/lib/src/widgets/ticker_provider.dart index 9f9b3cdd33..ef45229a72 100644 --- a/packages/flutter/lib/src/widgets/ticker_provider.dart +++ b/packages/flutter/lib/src/widgets/ticker_provider.dart @@ -15,7 +15,7 @@ export 'package:flutter/scheduler.dart' show TickerProvider; /// This only works if [AnimationController] objects are created using /// widget-aware ticker providers. For example, using a /// [TickerProviderStateMixin] or a [SingleTickerProviderStateMixin]. -class TickerMode extends StatelessWidget { +class TickerMode extends StatefulWidget { /// Creates a widget that enables or disables tickers. /// /// The [enabled] argument must not be null. @@ -62,18 +62,85 @@ class TickerMode extends StatelessWidget { return widget?.enabled ?? true; } + /// Obtains a [ValueNotifier] from the [TickerMode] surrounding the `context`, + /// which indicates whether tickers are enabled in the given subtree. + /// + /// When that [TickerMode] enabled or disabled tickers, the notifier notifies + /// its listeners. + /// + /// While the [ValueNotifier] is stable for the lifetime of the surrounding + /// [TickerMode], calling this method does not establish a dependency between + /// the `context` and the [TickerMode] and the widget owning the `context` + /// does not rebuild when the ticker mode changes from true to false or vice + /// versa. This is preferable when the ticker mode does not impact what is + /// currently rendered on screen, e.g. because it is ony used to mute/unmute a + /// [Ticker]. Since no dependency is established, the widget owning the + /// `context` is also not informed when it is moved to a new location in the + /// tree where it may have a different [TickerMode] ancestor. When this + /// happens, the widget must manually unsubscribe from the old notifier, + /// obtain a new one from the new ancestor [TickerMode] by calling this method + /// again, and re-subscribe to it. [StatefulWidget]s can, for example, do this + /// in [State.activate], which is called after the widget has been moved to + /// a new location. + /// + /// Alternatively, [of] can be used instead of this method to create a + /// dependency between the provided `context` and the ancestor [TickerMode]. + /// In this case, the widget automatically rebuilds when the ticker mode + /// changes or when it is moved to a new [TickerMode] ancestor, which + /// simplifies the management cost in the widget at the expensive of some + /// potential unnecessary rebuilds. + /// + /// In the absence of a [TickerMode] widget, this function returns a + /// [ValueNotifier], whose [ValueNotifier.value] is always true. + static ValueNotifier getNotifier(BuildContext context) { + final _EffectiveTickerMode? widget = context.getElementForInheritedWidgetOfExactType<_EffectiveTickerMode>()?.widget as _EffectiveTickerMode?; + return widget?.notifier ?? ValueNotifier(true); + } + + @override + State createState() => _TickerModeState(); +} + +class _TickerModeState extends State { + bool _ancestorTicketMode = true; + final ValueNotifier _effectiveMode = ValueNotifier(true); + + @override + void didChangeDependencies() { + super.didChangeDependencies(); + _ancestorTicketMode = TickerMode.of(context); + _updateEffectiveMode(); + } + + @override + void didUpdateWidget(TickerMode oldWidget) { + super.didUpdateWidget(oldWidget); + _updateEffectiveMode(); + } + + @override + void dispose() { + _effectiveMode.dispose(); + super.dispose(); + } + + void _updateEffectiveMode() { + _effectiveMode.value = _ancestorTicketMode && widget.enabled; + } + @override Widget build(BuildContext context) { return _EffectiveTickerMode( - enabled: enabled && TickerMode.of(context), - child: child, + enabled: _effectiveMode.value, + notifier: _effectiveMode, + child: widget.child, ); } @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); - properties.add(FlagProperty('requested mode', value: enabled, ifTrue: 'enabled', ifFalse: 'disabled', showName: true)); + properties.add(FlagProperty('requested mode', value: widget.enabled, ifTrue: 'enabled', ifFalse: 'disabled', showName: true)); } } @@ -81,11 +148,13 @@ class _EffectiveTickerMode extends InheritedWidget { const _EffectiveTickerMode({ Key? key, required this.enabled, + required this.notifier, required Widget child, }) : assert(enabled != null), - super(key: key, child: child); + super(key: key, child: child); final bool enabled; + final ValueNotifier notifier; @override bool updateShouldNotify(_EffectiveTickerMode oldWidget) => enabled != oldWidget.enabled; @@ -127,10 +196,8 @@ mixin SingleTickerProviderStateMixin on State imple ]); }()); _ticker = Ticker(onTick, debugLabel: kDebugMode ? 'created by ${describeIdentity(this)}' : null); - // We assume that this is called from initState, build, or some sort of - // event handler, and that thus TickerMode.of(context) would return true. We - // can't actually check that here because if we're in initState then we're - // not allowed to do inheritance checks yet. + _updateTickerModeNotifier(); + _updateTicker(); // Sets _ticker.mute correctly. return _ticker!; } @@ -154,14 +221,35 @@ mixin SingleTickerProviderStateMixin on State imple _ticker!.describeForError('The offending ticker was'), ]); }()); + _tickerModeNotifier?.removeListener(_updateTicker); + _tickerModeNotifier = null; super.dispose(); } + ValueNotifier? _tickerModeNotifier; + @override - void didChangeDependencies() { - if (_ticker != null) - _ticker!.muted = !TickerMode.of(context); - super.didChangeDependencies(); + void activate() { + super.activate(); + // We may have a new TickerMode ancestor. + _updateTickerModeNotifier(); + _updateTicker(); + } + + void _updateTicker() { + if (_ticker != null) { + _ticker!.muted = !_tickerModeNotifier!.value; + } + } + + void _updateTickerModeNotifier() { + final ValueNotifier newNotifier = TickerMode.getNotifier(context); + if (newNotifier == _tickerModeNotifier) { + return; + } + _tickerModeNotifier?.removeListener(_updateTicker); + newNotifier.addListener(_updateTicker); + _tickerModeNotifier = newNotifier; } @override @@ -198,8 +286,14 @@ mixin TickerProviderStateMixin on State implements @override Ticker createTicker(TickerCallback onTick) { + if (_tickerModeNotifier == null) { + // Setup TickerMode notifier before we vend the first ticker. + _updateTickerModeNotifier(); + } + assert(_tickerModeNotifier != null); _tickers ??= <_WidgetTicker>{}; - final _WidgetTicker result = _WidgetTicker(onTick, this, debugLabel: kDebugMode ? 'created by ${describeIdentity(this)}' : null); + final _WidgetTicker result = _WidgetTicker(onTick, this, debugLabel: kDebugMode ? 'created by ${describeIdentity(this)}' : null) + ..muted = !_tickerModeNotifier!.value; _tickers!.add(result); return result; } @@ -210,6 +304,35 @@ mixin TickerProviderStateMixin on State implements _tickers!.remove(ticker); } + ValueNotifier? _tickerModeNotifier; + + @override + void activate() { + super.activate(); + // We may have a new TickerMode ancestor, get its Notifier. + _updateTickerModeNotifier(); + _updateTickers(); + } + + void _updateTickers() { + if (_tickers != null) { + final bool muted = !_tickerModeNotifier!.value; + for (final Ticker ticker in _tickers!) { + ticker.muted = muted; + } + } + } + + void _updateTickerModeNotifier() { + final ValueNotifier newNotifier = TickerMode.getNotifier(context); + if (newNotifier == _tickerModeNotifier) { + return; + } + _tickerModeNotifier?.removeListener(_updateTickers); + newNotifier.addListener(_updateTickers); + _tickerModeNotifier = newNotifier; + } + @override void dispose() { assert(() { @@ -235,20 +358,11 @@ mixin TickerProviderStateMixin on State implements } return true; }()); + _tickerModeNotifier?.removeListener(_updateTickers); + _tickerModeNotifier = null; super.dispose(); } - @override - void didChangeDependencies() { - final bool muted = !TickerMode.of(context); - if (_tickers != null) { - for (final Ticker ticker in _tickers!) { - ticker.muted = muted; - } - } - super.didChangeDependencies(); - } - @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); diff --git a/packages/flutter/test/widgets/ticker_mode_test.dart b/packages/flutter/test/widgets/ticker_mode_test.dart index 60a8273023..30760c4874 100644 --- a/packages/flutter/test/widgets/ticker_mode_test.dart +++ b/packages/flutter/test/widgets/ticker_mode_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:flutter/material.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -98,36 +99,194 @@ void main() { expect(outerTickCount, 0); expect(innerTickCount, 0); }); + + testWidgets('Changing TickerMode does not rebuild widgets with SingleTickerProviderStateMixin', (WidgetTester tester) async { + Widget widgetUnderTest({required bool tickerEnabled}) { + return TickerMode( + enabled: tickerEnabled, + child: const _TickingWidget(), + ); + } + _TickingWidgetState state() => tester.state<_TickingWidgetState>(find.byType(_TickingWidget)); + + await tester.pumpWidget(widgetUnderTest(tickerEnabled: true)); + expect(state().ticker.isTicking, isTrue); + expect(state().buildCount, 1); + + await tester.pumpWidget(widgetUnderTest(tickerEnabled: false)); + expect(state().ticker.isTicking, isFalse); + expect(state().buildCount, 1); + + await tester.pumpWidget(widgetUnderTest(tickerEnabled: true)); + expect(state().ticker.isTicking, isTrue); + expect(state().buildCount, 1); + }); + + testWidgets('Changing TickerMode does not rebuild widgets with TickerProviderStateMixin', (WidgetTester tester) async { + Widget widgetUnderTest({required bool tickerEnabled}) { + return TickerMode( + enabled: tickerEnabled, + child: const _MultiTickingWidget(), + ); + } + _MultiTickingWidgetState state() => tester.state<_MultiTickingWidgetState>(find.byType(_MultiTickingWidget)); + + await tester.pumpWidget(widgetUnderTest(tickerEnabled: true)); + expect(state().ticker.isTicking, isTrue); + expect(state().buildCount, 1); + + await tester.pumpWidget(widgetUnderTest(tickerEnabled: false)); + expect(state().ticker.isTicking, isFalse); + expect(state().buildCount, 1); + + await tester.pumpWidget(widgetUnderTest(tickerEnabled: true)); + expect(state().ticker.isTicking, isTrue); + expect(state().buildCount, 1); + }); + + testWidgets('Moving widgets with SingleTickerProviderStateMixin to a new TickerMode ancestor works', (WidgetTester tester) async { + final GlobalKey tickingWidgetKey = GlobalKey(); + Widget widgetUnderTest({required LocalKey tickerModeKey, required bool tickerEnabled}) { + return TickerMode( + key: tickerModeKey, + enabled: tickerEnabled, + child: _TickingWidget(key: tickingWidgetKey), + ); + } + // Using different local keys to simulate changing TickerMode ancestors. + await tester.pumpWidget(widgetUnderTest(tickerEnabled: true, tickerModeKey: UniqueKey())); + final State tickerModeState = tester.state(find.byType(TickerMode)); + final _TickingWidgetState tickingState = tester.state<_TickingWidgetState>(find.byType(_TickingWidget)); + expect(tickingState.ticker.isTicking, isTrue); + + await tester.pumpWidget(widgetUnderTest(tickerEnabled: false, tickerModeKey: UniqueKey())); + expect(tester.state(find.byType(TickerMode)), isNot(same(tickerModeState))); + expect(tickingState, same(tester.state<_TickingWidgetState>(find.byType(_TickingWidget)))); + expect(tickingState.ticker.isTicking, isFalse); + }); + + testWidgets('Moving widgets with TickerProviderStateMixin to a new TickerMode ancestor works', (WidgetTester tester) async { + final GlobalKey tickingWidgetKey = GlobalKey(); + Widget widgetUnderTest({required LocalKey tickerModeKey, required bool tickerEnabled}) { + return TickerMode( + key: tickerModeKey, + enabled: tickerEnabled, + child: _MultiTickingWidget(key: tickingWidgetKey), + ); + } + // Using different local keys to simulate changing TickerMode ancestors. + await tester.pumpWidget(widgetUnderTest(tickerEnabled: true, tickerModeKey: UniqueKey())); + final State tickerModeState = tester.state(find.byType(TickerMode)); + final _MultiTickingWidgetState tickingState = tester.state<_MultiTickingWidgetState>(find.byType(_MultiTickingWidget)); + expect(tickingState.ticker.isTicking, isTrue); + + await tester.pumpWidget(widgetUnderTest(tickerEnabled: false, tickerModeKey: UniqueKey())); + expect(tester.state(find.byType(TickerMode)), isNot(same(tickerModeState))); + expect(tickingState, same(tester.state<_MultiTickingWidgetState>(find.byType(_MultiTickingWidget)))); + expect(tickingState.ticker.isTicking, isFalse); + }); + + testWidgets('Ticking widgets in old route do not rebuild when new route is pushed', (WidgetTester tester) async { + await tester.pumpWidget(MaterialApp( + routes: { + '/foo' : (BuildContext context) => const Text('New route'), + }, + home: Row( + children: const [ + _TickingWidget(), + _MultiTickingWidget(), + Text('Old route'), + ], + ), + )); + + _MultiTickingWidgetState multiTickingState() => tester.state<_MultiTickingWidgetState>(find.byType(_MultiTickingWidget, skipOffstage: false)); + _TickingWidgetState tickingState() => tester.state<_TickingWidgetState>(find.byType(_TickingWidget, skipOffstage: false)); + + expect(find.text('Old route'), findsOneWidget); + expect(find.text('New route'), findsNothing); + + expect(multiTickingState().ticker.isTicking, isTrue); + expect(multiTickingState().buildCount, 1); + expect(tickingState().ticker.isTicking, isTrue); + expect(tickingState().buildCount, 1); + + tester.state(find.byType(Navigator)).pushNamed('/foo'); + await tester.pumpAndSettle(); + expect(find.text('Old route'), findsNothing); + expect(find.text('New route'), findsOneWidget); + + expect(multiTickingState().ticker.isTicking, isFalse); + expect(multiTickingState().buildCount, 1); + expect(tickingState().ticker.isTicking, isFalse); + expect(tickingState().buildCount, 1); + }); } class _TickingWidget extends StatefulWidget { - const _TickingWidget({required this.onTick}); + const _TickingWidget({Key? key, this.onTick}) : super(key: key); - final VoidCallback onTick; + final VoidCallback? onTick; @override State<_TickingWidget> createState() => _TickingWidgetState(); } class _TickingWidgetState extends State<_TickingWidget> with SingleTickerProviderStateMixin { - late Ticker _ticker; + late Ticker ticker; + int buildCount = 0; @override void initState() { super.initState(); - _ticker = createTicker((Duration _) { - widget.onTick(); + ticker = createTicker((Duration _) { + widget.onTick?.call(); })..start(); } @override Widget build(BuildContext context) { + buildCount += 1; return Container(); } @override void dispose() { - _ticker.dispose(); + ticker.dispose(); + super.dispose(); + } +} + +class _MultiTickingWidget extends StatefulWidget { + const _MultiTickingWidget({Key? key, this.onTick}) : super(key: key); + + final VoidCallback? onTick; + + @override + State<_MultiTickingWidget> createState() => _MultiTickingWidgetState(); +} + +class _MultiTickingWidgetState extends State<_MultiTickingWidget> with TickerProviderStateMixin { + late Ticker ticker; + int buildCount = 0; + + @override + void initState() { + super.initState(); + ticker = createTicker((Duration _) { + widget.onTick?.call(); + })..start(); + } + + @override + Widget build(BuildContext context) { + buildCount += 1; + return Container(); + } + + @override + void dispose() { + ticker.dispose(); super.dispose(); } }