Avoid calling didChangeDependences on a State that has dropped out of the tree (flutter#49527)

Author: dnfield

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

@@ -657,7 +657,7 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier {
   /// Has no effect on nodes that return true from [hasFocus], but false from
   /// [hasPrimaryFocus].
   ///
-  /// if [focusPrevious] is true, then rather than losing all focus, the focus
+  /// If [focusPrevious] is true, then rather than losing all focus, the focus
   /// will be moved to the node that the [enclosingScope] thinks should have it,
   /// based on its history of nodes that were set as first focus on it using
   /// [FocusScopeNode.setFirstFocus].

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

@@ -398,6 +398,13 @@ class _FocusState extends State<Focus> {
   @override
   void deactivate() {
     super.deactivate();
+    // The focus node's location in the tree is no longer valid here. But
+    // we can't unfocus or remove the node from the tree because if the widget
+    // is moved to a different part of the tree (via global key) it should
+    // retain its focus state. That's why we temporarily park it on the root
+    // focus node (via reparent) until it either gets moved to a different part
+    // of the tree (via didChangeDependencies) or until it is disposed.
+    _focusAttachment?.reparent();
     _didAutofocus = false;
   }
 

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

@@ -4437,7 +4437,13 @@ class StatefulElement extends ComponentElement {
   }
 
   @override
-  Widget build() => state.build(this);
+  Widget build() {
+    if (_didChangeDependencies) {
+      _state.didChangeDependencies();
+      _didChangeDependencies = false;
+    }
+    return state.build(this);
+  }
 
   /// The [State] instance associated with this location in the tree.
   ///
@@ -4617,10 +4623,21 @@ class StatefulElement extends ComponentElement {
     return super.dependOnInheritedElement(ancestor as InheritedElement, aspect: aspect);
   }
 
+  /// This controls whether we should call [State.didChangeDependencies] from
+  /// the start of [build], to avoid calls when the [State] will not get built.
+  /// This can happen when the widget has dropped out of the tree, but depends
+  /// on an [InheritedWidget] that is still in the tree.
+  ///
+  /// It is set initially to false, since [_firstBuild] makes the initial call
+  /// on the [state]. When it is true, [build] will call
+  /// `state.didChangeDependencies` and then sets it to false. Subsequent calls
+  /// to [didChangeDependencies] set it to true.
+  bool _didChangeDependencies = false;
+
   @override
   void didChangeDependencies() {
     super.didChangeDependencies();
-    _state.didChangeDependencies();
+    _didChangeDependencies = true;
   }
 
   @override

packages/flutter/test/widgets/framework_test.dart

@@ -710,6 +710,35 @@ void main() {
       );
     }
   });
+
+  testWidgets('didUpdateDependencies is not called on a State that never rebuilds', (WidgetTester tester) async {
+    final GlobalKey<DependentState> key = GlobalKey<DependentState>();
+
+    /// Initial build - should call didChangeDependencies, not deactivate
+    await tester.pumpWidget(Inherited(1, child: DependentStatefulWidget(key: key)));
+    final DependentState state = key.currentState;
+    expect(key.currentState, isNotNull);
+    expect(state.didChangeDependenciesCount, 1);
+    expect(state.deactivatedCount, 0);
+
+    /// Rebuild with updated value - should call didChangeDependencies
+    await tester.pumpWidget(Inherited(2, child: DependentStatefulWidget(key: key)));
+    expect(key.currentState, isNotNull);
+    expect(state.didChangeDependenciesCount, 2);
+    expect(state.deactivatedCount, 0);
+
+    // reparent it - should call deactivate and didChangeDependencies
+    await tester.pumpWidget(Inherited(3, child: SizedBox(child: DependentStatefulWidget(key: key))));
+    expect(key.currentState, isNotNull);
+    expect(state.didChangeDependenciesCount, 3);
+    expect(state.deactivatedCount, 1);
+
+    // Remove it - should call deactivate, but not didChangeDependencies
+    await tester.pumpWidget(const Inherited(4, child: SizedBox()));
+    expect(key.currentState, isNull);
+    expect(state.didChangeDependenciesCount, 3);
+    expect(state.deactivatedCount, 2);
+  });
 }
 
 class NullChildTest extends Widget {
@@ -755,3 +784,42 @@ class DirtyElementWithCustomBuildOwner extends Element {
   @override
   bool get dirty => true;
 }
+
+class Inherited extends InheritedWidget {
+  const Inherited(this.value, {Widget child, Key key}) : super(key: key, child: child);
+
+  final int value;
+
+  @override
+  bool updateShouldNotify(Inherited oldWidget) => oldWidget.value != value;
+}
+
+class DependentStatefulWidget extends StatefulWidget {
+  const DependentStatefulWidget({Key key}) : super(key: key);
+
+  @override
+  State<StatefulWidget> createState() => DependentState();
+}
+
+class DependentState extends State<DependentStatefulWidget> {
+  int didChangeDependenciesCount = 0;
+  int deactivatedCount = 0;
+
+  @override
+  void didChangeDependencies() {
+    super.didChangeDependencies();
+    didChangeDependenciesCount += 1;
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    context.dependOnInheritedWidgetOfExactType<Inherited>();
+    return const SizedBox();
+  }
+
+  @override
+  void deactivate() {
+    super.deactivate();
+    deactivatedCount += 1;
+  }
+}