Skip to content

Commit 4505068

Browse files
Reverts "Cache FocusNode.enclosingScope, clean up descendantsAreFocusable (flutter#144207)" (flutter#144292)
Reverts flutter#144207 Initiated by: CaseyHillers Reason for reverting: b/327301206 - Breaking a customer test Original PR Author: LongCatIsLooong Reviewed By: {gspencergoog} This change reverts the following previous change: Original Description: `FocusNode.canRequestFocus` was doing a double traversal if no ancestor disallows focus. The last for loop only has to reach as far as the enclosing scope. Also this caches the `FocusNode.enclosingScope` since the getter access happens much more frequently than node reparenting.
1 parent 96a390a commit 4505068

File tree

2 files changed

+27
-49
lines changed

2 files changed

+27
-49
lines changed

packages/flutter/lib/src/widgets/focus_manager.dart

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,21 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
517517
/// focus traversal policy for a widget subtree.
518518
/// * [FocusTraversalPolicy], a class that can be extended to describe a
519519
/// traversal policy.
520-
bool get canRequestFocus => _canRequestFocus && ancestors.every(_allowDescendantsToBeFocused);
521-
static bool _allowDescendantsToBeFocused(FocusNode ancestor) => ancestor.descendantsAreFocusable;
520+
bool get canRequestFocus {
521+
if (!_canRequestFocus) {
522+
return false;
523+
}
524+
final FocusScopeNode? scope = enclosingScope;
525+
if (scope != null && !scope.canRequestFocus) {
526+
return false;
527+
}
528+
for (final FocusNode ancestor in ancestors) {
529+
if (!ancestor.descendantsAreFocusable) {
530+
return false;
531+
}
532+
}
533+
return true;
534+
}
522535

523536
bool _canRequestFocus;
524537
@mustCallSuper
@@ -778,22 +791,6 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
778791
/// Use [enclosingScope] to look for scopes above this node.
779792
FocusScopeNode? get nearestScope => enclosingScope;
780793

781-
FocusScopeNode? _enclosingScope;
782-
void _clearEnclosingScopeCache() {
783-
final FocusScopeNode? cachedScope = _enclosingScope;
784-
if (cachedScope == null) {
785-
return;
786-
}
787-
_enclosingScope = null;
788-
if (children.isNotEmpty) {
789-
for (final FocusNode child in children) {
790-
if (identical(cachedScope, child._enclosingScope)) {
791-
child._clearEnclosingScopeCache();
792-
}
793-
}
794-
}
795-
}
796-
797794
/// Returns the nearest enclosing scope node above this node, or null if the
798795
/// node has not yet be added to the focus tree.
799796
///
@@ -802,9 +799,12 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
802799
///
803800
/// Use [nearestScope] to start at this node instead of above it.
804801
FocusScopeNode? get enclosingScope {
805-
final FocusScopeNode? enclosingScope = _enclosingScope ??= parent?.nearestScope;
806-
assert(enclosingScope == parent?.nearestScope, '$this has invalid scope cache: $_enclosingScope != ${parent?.nearestScope}');
807-
return enclosingScope;
802+
for (final FocusNode node in ancestors) {
803+
if (node is FocusScopeNode) {
804+
return node;
805+
}
806+
}
807+
return null;
808808
}
809809

810810
/// Returns the size of the attached widget's [RenderObject], in logical
@@ -990,7 +990,6 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
990990
}
991991

992992
node._parent = null;
993-
node._clearEnclosingScopeCache();
994993
_children.remove(node);
995994
for (final FocusNode ancestor in ancestors) {
996995
ancestor._descendants = null;
@@ -1269,14 +1268,13 @@ class FocusScopeNode extends FocusNode {
12691268
super.skipTraversal,
12701269
super.canRequestFocus,
12711270
this.traversalEdgeBehavior = TraversalEdgeBehavior.closedLoop,
1272-
}) : super(descendantsAreFocusable: true);
1271+
}) : super(
1272+
descendantsAreFocusable: true,
1273+
);
12731274

12741275
@override
12751276
FocusScopeNode get nearestScope => this;
12761277

1277-
@override
1278-
bool get descendantsAreFocusable => _canRequestFocus && super.descendantsAreFocusable;
1279-
12801278
/// Controls the transfer of focus beyond the first and the last items of a
12811279
/// [FocusScopeNode].
12821280
///

packages/flutter/test/widgets/focus_manager_test.dart

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -529,16 +529,16 @@ void main() {
529529
// child1
530530
// |
531531
// child2
532-
final FocusScopeNode scope1 = FocusScopeNode(debugLabel: 'scope1');
532+
final FocusScopeNode scope1 = FocusScopeNode(debugLabel: 'scope2');
533533
addTearDown(scope1.dispose);
534534
final FocusAttachment scope2Attachment = scope1.attach(context);
535535
scope2Attachment.reparent(parent: tester.binding.focusManager.rootScope);
536536

537-
final FocusNode child1 = FocusNode(debugLabel: 'child1');
537+
final FocusNode child1 = FocusNode(debugLabel: 'child2');
538538
addTearDown(child1.dispose);
539539
final FocusAttachment child2Attachment = child1.attach(context);
540540

541-
final FocusNode child2 = FocusNode(debugLabel: 'child2');
541+
final FocusNode child2 = FocusNode(debugLabel: 'child3');
542542
addTearDown(child2.dispose);
543543
final FocusAttachment child3Attachment = child2.attach(context);
544544

@@ -697,26 +697,6 @@ void main() {
697697
expect(parent2.children.first, equals(child1));
698698
});
699699

700-
test('FocusScopeNode.canRequestFocus affects descendantsAreFocusable', () {
701-
final FocusScopeNode scope = FocusScopeNode(debugLabel: 'Scope');
702-
703-
scope.descendantsAreFocusable = false;
704-
expect(scope.descendantsAreFocusable, isFalse);
705-
expect(scope.canRequestFocus, isTrue);
706-
707-
scope.descendantsAreFocusable = true;
708-
expect(scope.descendantsAreFocusable, isTrue);
709-
expect(scope.canRequestFocus, isTrue);
710-
711-
scope.canRequestFocus = false;
712-
expect(scope.descendantsAreFocusable, isFalse);
713-
expect(scope.canRequestFocus, isFalse);
714-
715-
scope.canRequestFocus = true;
716-
expect(scope.descendantsAreFocusable, isTrue);
717-
expect(scope.canRequestFocus, isTrue);
718-
});
719-
720700
testWidgets('canRequestFocus affects children.', (WidgetTester tester) async {
721701
final BuildContext context = await setupWidget(tester);
722702
final FocusScopeNode scope = FocusScopeNode(debugLabel: 'Scope');

0 commit comments

Comments
 (0)