Make `_focusDebug` not interpolate in debug mode (#119680)
* Make _focusDebug not interpolate in debug mode
* Add test
* Revert undesired change
* Fix test to fail before too
* Remove accidental skips
* Switch to using a generating closure for arguments.
* Remove a word
diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart
index b915c88..de94e41 100644
--- a/packages/flutter/lib/src/widgets/focus_manager.dart
+++ b/packages/flutter/lib/src/widgets/focus_manager.dart
@@ -22,16 +22,38 @@
/// be logged.
bool debugFocusChanges = false;
-bool _focusDebug(String message, [Iterable<String>? 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<Object> 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<Object> details = detailsFunc?.call() ?? const <Object>[];
+ 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 @@
&& 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 @@
///
/// Calling [FocusNode.dispose] will also automatically detach the node.
void detach() {
- assert(_focusDebug('Detaching node:', <String>[_node.toString(), 'With enclosing scope ${_node.enclosingScope}']));
+ assert(_focusDebug(() => 'Detaching node:', () => <Object>[_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 @@
scope._doRequestFocus(findFirstFocus: true);
break;
}
- assert(_focusDebug('Unfocused node:', <String>['primary focus was $this', 'next focus will be ${_manager?._markedForFocus}']));
+ assert(_focusDebug(() => 'Unfocused node:', () => <Object>['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 @@
// 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 @@
return;
}
_hasKeyboardToken = true;
- assert(_focusDebug('Node requesting focus: $this'));
+ assert(_focusDebug(() => 'Node requesting focus: $this'));
_markNextFocus(this);
}
@@ -1109,7 +1131,7 @@
FocusNode scopeFocus = this;
for (final FocusScopeNode ancestor in ancestors.whereType<FocusScopeNode>()) {
assert(scopeFocus != ancestor, 'Somehow made a loop by setting focusedChild to its scope.');
- assert(_focusDebug('Setting $scopeFocus as focused child for scope:', <String>[ancestor.toString()]));
+ assert(_focusDebug(() => 'Setting $scopeFocus as focused child for scope:', () => <Object>[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 @@
/// 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:', <String>[scope.toString()]));
+ assert(_focusDebug(() => 'Setting scope as first focus in $this to node:', () => <Object>[scope]));
if (scope._parent == null) {
_reparent(scope);
}
@@ -1306,7 +1328,7 @@
}
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 @@
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 @@
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 @@
// 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 @@
// 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 @@
}
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 @@
for (final FocusNode node in _dirtyNodes) {
node._notify();
}
- assert(_focusDebug('Notified ${_dirtyNodes.length} dirty nodes:', _dirtyNodes.toList().map<String>((FocusNode node) => node.toString())));
+ assert(_focusDebug(() => 'Notified ${_dirtyNodes.length} dirty nodes:', () => _dirtyNodes));
_dirtyNodes.clear();
if (previousFocus != _primaryFocus) {
notifyListeners();
@@ -1766,9 +1788,9 @@
_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 @@
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 @@
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 53501fc..21c8b0d 100644
--- a/packages/flutter/test/widgets/focus_manager_test.dart
+++ b/packages/flutter/test/widgets/focus_manager_test.dart
@@ -1700,4 +1700,61 @@
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<void> 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");
+ }
}
