Unverified Commit 7aec9b46 authored by Dan Field's avatar Dan Field Committed by GitHub

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

parent 11c5812d
......@@ -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].
......
......@@ -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;
}
......
......@@ -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
......
......@@ -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;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment