diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart index b915c88a40a..de94e41cf3e 100644 --- a/packages/flutter/lib/src/widgets/focus_manager.dart +++ b/packages/flutter/lib/src/widgets/focus_manager.dart @@ -22,16 +22,38 @@ import 'framework.dart'; /// be logged. bool debugFocusChanges = false; -bool _focusDebug(String message, [Iterable? details]) { - if (debugFocusChanges) { - debugPrint('FOCUS: $message'); - if (details != null && details.isNotEmpty) { - for (final String detail in details) { - debugPrint(' $detail'); - } +// When using _focusDebug, always call it like so: +// +// assert(_focusDebug(() => 'Blah $foo')); +// +// It needs to be inside the assert in order to be removed in release mode, and +// it needs to use a closure to generate the string in order to avoid string +// interpolation when debugFocusChanges is false. +// +// It will throw a StateError if you try to call it when the app is in release +// mode. +bool _focusDebug( + String Function() messageFunc, [ + Iterable Function()? detailsFunc, +]) { + if (kReleaseMode) { + throw StateError( + '_focusDebug was called in Release mode. It should always be wrapped in ' + 'an assert. Always call _focusDebug like so:\n' + r" assert(_focusDebug(() => 'Blah $foo'));" + ); + } + if (!debugFocusChanges) { + return true; + } + debugPrint('FOCUS: ${messageFunc()}'); + final Iterable details = detailsFunc?.call() ?? const []; + if (details.isNotEmpty) { + for (final Object detail in details) { + debugPrint(' $detail'); } } - // Return true so that it can be easily used inside of an assert. + // Return true so that it can be used inside of an assert. return true; } @@ -118,10 +140,10 @@ class _Autofocus { && scope.focusedChild == null && autofocusNode.ancestors.contains(scope); if (shouldApply) { - assert(_focusDebug('Applying autofocus: $autofocusNode')); + assert(_focusDebug(() => 'Applying autofocus: $autofocusNode')); autofocusNode._doRequestFocus(findFirstFocus: true); } else { - assert(_focusDebug('Autofocus request discarded for node: $autofocusNode.')); + assert(_focusDebug(() => 'Autofocus request discarded for node: $autofocusNode.')); } } } @@ -169,7 +191,7 @@ class FocusAttachment { /// /// Calling [FocusNode.dispose] will also automatically detach the node. void detach() { - assert(_focusDebug('Detaching node:', [_node.toString(), 'With enclosing scope ${_node.enclosingScope}'])); + assert(_focusDebug(() => 'Detaching node:', () => [_node, 'With enclosing scope ${_node.enclosingScope}'])); if (isAttached) { if (_node.hasPrimaryFocus || (_node._manager != null && _node._manager!._markedForFocus == _node)) { _node.unfocus(disposition: UnfocusDisposition.previouslyFocusedChild); @@ -877,7 +899,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { scope._doRequestFocus(findFirstFocus: true); break; } - assert(_focusDebug('Unfocused node:', ['primary focus was $this', 'next focus will be ${_manager?._markedForFocus}'])); + assert(_focusDebug(() => 'Unfocused node:', () => ['primary focus was $this', 'next focus will be ${_manager?._markedForFocus}'])); } /// Removes the keyboard token from this focus node if it has one. @@ -1063,7 +1085,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { // Note that this is overridden in FocusScopeNode. void _doRequestFocus({required bool findFirstFocus}) { if (!canRequestFocus) { - assert(_focusDebug('Node NOT requesting focus because canRequestFocus is false: $this')); + assert(_focusDebug(() => 'Node NOT requesting focus because canRequestFocus is false: $this')); return; } // If the node isn't part of the tree, then we just defer the focus request @@ -1078,7 +1100,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { return; } _hasKeyboardToken = true; - assert(_focusDebug('Node requesting focus: $this')); + assert(_focusDebug(() => 'Node requesting focus: $this')); _markNextFocus(this); } @@ -1109,7 +1131,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { FocusNode scopeFocus = this; for (final FocusScopeNode ancestor in ancestors.whereType()) { assert(scopeFocus != ancestor, 'Somehow made a loop by setting focusedChild to its scope.'); - assert(_focusDebug('Setting $scopeFocus as focused child for scope:', [ancestor.toString()])); + assert(_focusDebug(() => 'Setting $scopeFocus as focused child for scope:', () => [ancestor])); // Remove it anywhere in the focused child history. ancestor._focusedChildren.remove(scopeFocus); // Add it to the end of the list, which is also the top of the queue: The @@ -1276,7 +1298,7 @@ class FocusScopeNode extends FocusNode { /// tree, the given scope must be a descendant of this scope. void setFirstFocus(FocusScopeNode scope) { assert(scope != this, 'Unexpected self-reference in setFirstFocus.'); - assert(_focusDebug('Setting scope as first focus in $this to node:', [scope.toString()])); + assert(_focusDebug(() => 'Setting scope as first focus in $this to node:', () => [scope])); if (scope._parent == null) { _reparent(scope); } @@ -1306,7 +1328,7 @@ class FocusScopeNode extends FocusNode { } assert(_manager != null); - assert(_focusDebug('Autofocus scheduled for $node: scope $this')); + assert(_focusDebug(() => 'Autofocus scheduled for $node: scope $this')); _manager?._pendingAutofocuses.add(_Autofocus(scope: this, autofocusNode: node)); _manager?._markNeedsUpdate(); } @@ -1542,7 +1564,7 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier { void _markDetached(FocusNode node) { // The node has been removed from the tree, so it no longer needs to be // notified of changes. - assert(_focusDebug('Node was detached: $node')); + assert(_focusDebug(() => 'Node was detached: $node')); if (_primaryFocus == node) { _primaryFocus = null; } @@ -1551,7 +1573,7 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier { void _markPropertiesChanged(FocusNode node) { _markNeedsUpdate(); - assert(_focusDebug('Properties changed for node $node.')); + assert(_focusDebug(() => 'Properties changed for node $node.')); _dirtyNodes.add(node); } @@ -1575,7 +1597,7 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier { // Request that an update be scheduled, optionally requesting focus for the // given newFocus node. void _markNeedsUpdate() { - assert(_focusDebug('Scheduling update, current focus is $_primaryFocus, next focus will be $_markedForFocus')); + assert(_focusDebug(() => 'Scheduling update, current focus is $_primaryFocus, next focus will be $_markedForFocus')); if (_haveScheduledUpdate) { return; } @@ -1597,7 +1619,7 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier { // then revert to the root scope. _markedForFocus = rootScope; } - assert(_focusDebug('Refreshing focus state. Next focus will be $_markedForFocus')); + assert(_focusDebug(() => 'Refreshing focus state. Next focus will be $_markedForFocus')); // A node has requested to be the next focus, and isn't already the primary // focus. if (_markedForFocus != null && _markedForFocus != _primaryFocus) { @@ -1613,7 +1635,7 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier { } assert(_markedForFocus == null); if (previousFocus != _primaryFocus) { - assert(_focusDebug('Updating focus from $previousFocus to $_primaryFocus')); + assert(_focusDebug(() => 'Updating focus from $previousFocus to $_primaryFocus')); if (previousFocus != null) { _dirtyNodes.add(previousFocus); } @@ -1624,7 +1646,7 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier { for (final FocusNode node in _dirtyNodes) { node._notify(); } - assert(_focusDebug('Notified ${_dirtyNodes.length} dirty nodes:', _dirtyNodes.toList().map((FocusNode node) => node.toString()))); + assert(_focusDebug(() => 'Notified ${_dirtyNodes.length} dirty nodes:', () => _dirtyNodes)); _dirtyNodes.clear(); if (previousFocus != _primaryFocus) { notifyListeners(); @@ -1766,9 +1788,9 @@ class _HighlightModeManager { _lastInteractionWasTouch = false; updateMode(); - assert(_focusDebug('Received key event $message')); + assert(_focusDebug(() => 'Received key event $message')); if (FocusManager.instance.primaryFocus == null) { - assert(_focusDebug('No primary focus for key event, ignored: $message')); + assert(_focusDebug(() => 'No primary focus for key event, ignored: $message')); return false; } @@ -1794,11 +1816,11 @@ class _HighlightModeManager { case KeyEventResult.ignored: continue; case KeyEventResult.handled: - assert(_focusDebug('Node $node handled key event $message.')); + assert(_focusDebug(() => 'Node $node handled key event $message.')); handled = true; break; case KeyEventResult.skipRemainingHandlers: - assert(_focusDebug('Node $node stopped key event propagation: $message.')); + assert(_focusDebug(() => 'Node $node stopped key event propagation: $message.')); handled = false; break; } @@ -1808,7 +1830,7 @@ class _HighlightModeManager { break; } if (!handled) { - assert(_focusDebug('Key event not handled by anyone: $message.')); + assert(_focusDebug(() => 'Key event not handled by anyone: $message.')); } return handled; } diff --git a/packages/flutter/test/widgets/focus_manager_test.dart b/packages/flutter/test/widgets/focus_manager_test.dart index 53501fc62eb..21c8b0dd837 100644 --- a/packages/flutter/test/widgets/focus_manager_test.dart +++ b/packages/flutter/test/widgets/focus_manager_test.dart @@ -1700,4 +1700,61 @@ void main() { expect(messagesStr, contains('FOCUS: Notified 2 dirty nodes')); expect(messagesStr, contains(RegExp(r'FOCUS: Scheduling update, current focus is null, next focus will be FocusScopeNode#.*parent1'))); }); + + testWidgets("doesn't call toString on a focus node when debugFocusChanges is false", (WidgetTester tester) async { + final bool oldDebugFocusChanges = debugFocusChanges; + final DebugPrintCallback oldDebugPrint = debugPrint; + final StringBuffer messages = StringBuffer(); + debugPrint = (String? message, {int? wrapWidth}) { + messages.writeln(message ?? ''); + }; + Future testDebugFocusChanges() async { + final BuildContext context = await setupWidget(tester); + final FocusScopeNode parent1 = FocusScopeNode(debugLabel: 'parent1'); + final FocusAttachment parent1Attachment = parent1.attach(context); + final FocusNode child1 = debugFocusChanges ? FocusNode(debugLabel: 'child1') : _LoggingTestFocusNode(debugLabel: 'child1'); + final FocusAttachment child1Attachment = child1.attach(context); + parent1Attachment.reparent(parent: tester.binding.focusManager.rootScope); + child1Attachment.reparent(parent: parent1); + + child1.requestFocus(); + await tester.pump(); + child1.dispose(); + parent1.dispose(); + await tester.pump(); + } + try { + debugFocusChanges = false; + await testDebugFocusChanges(); + expect(messages, isEmpty); + expect(tester.takeException(), isNull); + debugFocusChanges = true; + await testDebugFocusChanges(); + expect(messages.toString(), contains('FOCUS: Notified 3 dirty nodes:')); + expect(tester.takeException(), isNull); + } finally { + debugFocusChanges = oldDebugFocusChanges; + debugPrint = oldDebugPrint; + } + }); +} + +class _LoggingTestFocusNode extends FocusNode { + _LoggingTestFocusNode({super.debugLabel}); + + @override + String toString({ + DiagnosticLevel minLevel = DiagnosticLevel.debug, + }) { + throw StateError("Shouldn't call toString here"); + } + + @override + String toStringDeep({ + String prefixLineOne = '', + String? prefixOtherLines, + DiagnosticLevel minLevel = DiagnosticLevel.debug, + }) { + throw StateError("Shouldn't call toStringDeep here"); + } }