Reland "Cache FocusNode.enclosingScope, clean up descendantsAreFocusa…

…ble (flutter#144207)" (flutter#144330)

The [internal test failure](flutter#144207 (comment)) was caused by `Focus.withExternalFocusNode` modifying the external node's attributes. The extra changes are in this commit:  flutter@e53d98b

CL with (almost) passing TGP: cl/611157582

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

@@ -517,21 +517,8 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
   ///    focus traversal policy for a widget subtree.
   ///  * [FocusTraversalPolicy], a class that can be extended to describe a
   ///    traversal policy.
-  bool get canRequestFocus {
-    if (!_canRequestFocus) {
-      return false;
-    }
-    final FocusScopeNode? scope = enclosingScope;
-    if (scope != null && !scope.canRequestFocus) {
-      return false;
-    }
-    for (final FocusNode ancestor in ancestors) {
-      if (!ancestor.descendantsAreFocusable) {
-        return false;
-      }
-    }
-    return true;
-  }
+  bool get canRequestFocus => _canRequestFocus && ancestors.every(_allowDescendantsToBeFocused);
+  static bool _allowDescendantsToBeFocused(FocusNode ancestor) => ancestor.descendantsAreFocusable;
 
   bool _canRequestFocus;
   @mustCallSuper
@@ -791,6 +778,22 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
   /// Use [enclosingScope] to look for scopes above this node.
   FocusScopeNode? get nearestScope => enclosingScope;
 
+  FocusScopeNode? _enclosingScope;
+  void _clearEnclosingScopeCache() {
+    final FocusScopeNode? cachedScope = _enclosingScope;
+    if (cachedScope == null) {
+      return;
+    }
+    _enclosingScope = null;
+    if (children.isNotEmpty) {
+      for (final FocusNode child in children) {
+        if (identical(cachedScope, child._enclosingScope)) {
+          child._clearEnclosingScopeCache();
+        }
+      }
+    }
+  }
+
   /// Returns the nearest enclosing scope node above this node, or null if the
   /// node has not yet be added to the focus tree.
   ///
@@ -799,12 +802,9 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
   ///
   /// Use [nearestScope] to start at this node instead of above it.
   FocusScopeNode? get enclosingScope {
-    for (final FocusNode node in ancestors) {
-      if (node is FocusScopeNode) {
-        return node;
-      }
-    }
-    return null;
+    final FocusScopeNode? enclosingScope = _enclosingScope ??= parent?.nearestScope;
+    assert(enclosingScope == parent?.nearestScope, '$this has invalid scope cache: $_enclosingScope != ${parent?.nearestScope}');
+    return enclosingScope;
   }
 
   /// Returns the size of the attached widget's [RenderObject], in logical
@@ -990,6 +990,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
     }
 
     node._parent = null;
+    node._clearEnclosingScopeCache();
     _children.remove(node);
     for (final FocusNode ancestor in ancestors) {
       ancestor._descendants = null;
@@ -1268,13 +1269,14 @@ class FocusScopeNode extends FocusNode {
     super.skipTraversal,
     super.canRequestFocus,
     this.traversalEdgeBehavior = TraversalEdgeBehavior.closedLoop,
-  })  : super(
-          descendantsAreFocusable: true,
-        );
+  })  : super(descendantsAreFocusable: true);
 
   @override
   FocusScopeNode get nearestScope => this;
 
+  @override
+  bool get descendantsAreFocusable => _canRequestFocus && super.descendantsAreFocusable;
+
   /// Controls the transfer of focus beyond the first and the last items of a
   /// [FocusScopeNode].
   ///

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

@@ -511,7 +511,7 @@ class _FocusWithExternalFocusNode extends Focus {
 
 class _FocusState extends State<Focus> {
   FocusNode? _internalNode;
-  FocusNode get focusNode => widget.focusNode ?? _internalNode!;
+  FocusNode get focusNode => widget.focusNode ?? (_internalNode ??= _createNode());
   late bool _hadPrimaryFocus;
   late bool _couldRequestFocus;
   late bool _descendantsWereFocusable;
@@ -526,17 +526,13 @@ class _FocusState extends State<Focus> {
   }
 
   void _initNode() {
-    if (widget.focusNode == null) {
-      // Only create a new node if the widget doesn't have one.
-      // This calls a function instead of just allocating in place because
-      // _createNode is overridden in _FocusScopeState.
-      _internalNode ??= _createNode();
-    }
-    focusNode.descendantsAreFocusable = widget.descendantsAreFocusable;
-    focusNode.descendantsAreTraversable = widget.descendantsAreTraversable;
-    focusNode.skipTraversal = widget.skipTraversal;
-    if (widget._canRequestFocus != null) {
-      focusNode.canRequestFocus = widget._canRequestFocus!;
+    if (!widget._usingExternalFocus) {
+      focusNode.descendantsAreFocusable = widget.descendantsAreFocusable;
+      focusNode.descendantsAreTraversable = widget.descendantsAreTraversable;
+      focusNode.skipTraversal = widget.skipTraversal;
+      if (widget._canRequestFocus != null) {
+        focusNode.canRequestFocus = widget._canRequestFocus!;
+      }
     }
     _couldRequestFocus = focusNode.canRequestFocus;
     _descendantsWereFocusable = focusNode.descendantsAreFocusable;

packages/flutter/test/widgets/focus_manager_test.dart

@@ -529,16 +529,16 @@ void main() {
       // child1
       //   |
       // child2
-      final FocusScopeNode scope1 = FocusScopeNode(debugLabel: 'scope2');
+      final FocusScopeNode scope1 = FocusScopeNode(debugLabel: 'scope1');
       addTearDown(scope1.dispose);
       final FocusAttachment scope2Attachment = scope1.attach(context);
       scope2Attachment.reparent(parent: tester.binding.focusManager.rootScope);
 
-      final FocusNode child1 = FocusNode(debugLabel: 'child2');
+      final FocusNode child1 = FocusNode(debugLabel: 'child1');
       addTearDown(child1.dispose);
       final FocusAttachment child2Attachment = child1.attach(context);
 
-      final FocusNode child2 = FocusNode(debugLabel: 'child3');
+      final FocusNode child2 = FocusNode(debugLabel: 'child2');
       addTearDown(child2.dispose);
       final FocusAttachment child3Attachment = child2.attach(context);
 
@@ -697,6 +697,26 @@ void main() {
       expect(parent2.children.first, equals(child1));
     });
 
+    test('FocusScopeNode.canRequestFocus affects descendantsAreFocusable', () {
+      final FocusScopeNode scope = FocusScopeNode(debugLabel: 'Scope');
+
+      scope.descendantsAreFocusable = false;
+      expect(scope.descendantsAreFocusable, isFalse);
+      expect(scope.canRequestFocus, isTrue);
+
+      scope.descendantsAreFocusable = true;
+      expect(scope.descendantsAreFocusable, isTrue);
+      expect(scope.canRequestFocus, isTrue);
+
+      scope.canRequestFocus = false;
+      expect(scope.descendantsAreFocusable, isFalse);
+      expect(scope.canRequestFocus, isFalse);
+
+      scope.canRequestFocus = true;
+      expect(scope.descendantsAreFocusable, isTrue);
+      expect(scope.canRequestFocus, isTrue);
+    });
+
     testWidgets('canRequestFocus affects children.', (WidgetTester tester) async {
       final BuildContext context = await setupWidget(tester);
       final FocusScopeNode scope = FocusScopeNode(debugLabel: 'Scope');

packages/flutter/test/widgets/focus_scope_test.dart

@@ -1964,6 +1964,21 @@ void main() {
       expect(keyEventHandled, isTrue);
     });
 
+    testWidgets('Focus does not update the focusNode attributes when the widget updates if withExternalFocusNode is used 2', (WidgetTester tester) async {
+      final TestExternalFocusNode focusNode = TestExternalFocusNode();
+      assert(!focusNode.isModified);
+      addTearDown(focusNode.dispose);
+
+      final Focus focusWidget = Focus.withExternalFocusNode(
+        focusNode: focusNode,
+        child: Container(),
+      );
+
+      await tester.pumpWidget(focusWidget);
+      expect(focusNode.isModified, isFalse);
+      await tester.pumpWidget(const SizedBox());
+    });
+
     testWidgets('Focus passes changes in attribute values to its focus node', (WidgetTester tester) async {
       await tester.pumpWidget(
         Focus(
@@ -2194,3 +2209,44 @@ class TestFocusState extends State<TestFocus> {
     );
   }
 }
+
+class TestExternalFocusNode extends FocusNode {
+  TestExternalFocusNode();
+
+  bool isModified = false;
+
+  @override
+  FocusOnKeyEventCallback? get onKeyEvent => _onKeyEvent;
+  FocusOnKeyEventCallback? _onKeyEvent;
+  @override
+  set onKeyEvent(FocusOnKeyEventCallback? newValue) {
+    if (newValue != _onKeyEvent) {
+      _onKeyEvent = newValue;
+      isModified = true;
+    }
+  }
+
+  @override
+  set descendantsAreFocusable(bool newValue) {
+    super.descendantsAreFocusable = newValue;
+    isModified = true;
+  }
+
+  @override
+  set descendantsAreTraversable(bool newValue) {
+    super.descendantsAreTraversable = newValue;
+    isModified = true;
+  }
+
+  @override
+  set skipTraversal(bool newValue) {
+    super.skipTraversal = newValue;
+    isModified = true;
+  }
+
+  @override
+  set canRequestFocus(bool newValue) {
+    super.canRequestFocus = newValue;
+    isModified = true;
+  }
+}