diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 3ce62d3c8db..ac7ef2103de 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -2059,7 +2059,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge assert(child._parent == this); assert(child.attached == attached); assert(child.parentData != null); - if (!(_isRelayoutBoundary ?? true)) { + if (!(child._isRelayoutBoundary ?? true)) { child._isRelayoutBoundary = null; } child.parentData!.detach(); diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index 90483526cbd..95f4bb07cbd 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -2053,13 +2053,54 @@ abstract class MultiChildRenderObjectWidget extends RenderObjectWidget { // ELEMENTS -enum _ElementLifecycle { initial, active, inactive, defunct } +enum _ElementLifecycle { + /// The [Element] is created but has not yet been incorporated into the element + /// tree. + initial, + + /// The [Element] is incorporated into the Element tree, either via + /// [Element.mount] or [Element.activate]. + active, + + /// The previously `active` [Element] is removed from the Element tree via + /// [Element.deactivate]. + /// + /// This [Element] may become `active` again if a parent reclaims it using + /// a [GlobalKey], or `defunct` if no parent reclaims it at the end of the + /// build phase. + inactive, + + /// The [Element] encountered an unrecoverable error while being rebuilt when it + /// was `active` or while being incorporated in the tree. + /// + /// This indicates the [Element]'s subtree is in an inconsistent state and must + /// not be re-incorporated into the tree again. + /// + /// When an unrecoverable error is encountered, the framework calls + /// [Element.deactivate] on this [Element] and sets its state to `failed`. This + /// process is done on a best-effort basis and does not surface any additional + /// errors. + /// + /// This is one of the two final stages of the element lifecycle and is not + /// reversible. Reaching this state typically means that a widget implementation + /// is throwing unhandled exceptions that need to be properly handled. + failed, + + /// The [Element] is disposed and should not be interacted with. + /// + /// The [Element] must be `inactive` before transitioning into this state, + /// and the state transition occurs in [BuildOwner.finalizeTree] which signals + /// the end of the build phase. + /// + /// This is the final stage of the element lifecycle and is not reversible. + defunct, +} class _InactiveElements { bool _locked = false; final Set _elements = HashSet(); - void _unmount(Element element) { + static void _unmount(Element element) { assert(element._lifecycleState == _ElementLifecycle.inactive); assert(() { if (debugPrintGlobalKeyedWidgetLifecycle) { @@ -2091,8 +2132,12 @@ class _InactiveElements { static void _deactivateRecursively(Element element) { assert(element._lifecycleState == _ElementLifecycle.active); - element.deactivate(); - assert(element._lifecycleState == _ElementLifecycle.inactive); + try { + element.deactivate(); + } catch (_) { + Element._deactivateFailedSubtreeRecursively(element); + rethrow; + } element.visitChildren(_deactivateRecursively); assert(() { element.debugDeactivated(); @@ -2104,10 +2149,18 @@ class _InactiveElements { assert(!_locked); assert(!_elements.contains(element)); assert(element._parent == null); - if (element._lifecycleState == _ElementLifecycle.active) { - _deactivateRecursively(element); + + switch (element._lifecycleState) { + case _ElementLifecycle.active: + _deactivateRecursively(element); + // This element is only added to _elements if the whole subtree is + // successfully deactivated. + _elements.add(element); + case _ElementLifecycle.inactive: + _elements.add(element); + case _ElementLifecycle.initial || _ElementLifecycle.failed || _ElementLifecycle.defunct: + assert(false, '$element must not be deactivated when in ${element._lifecycleState} state.'); } - _elements.add(element); } void remove(Element element) { @@ -2115,7 +2168,7 @@ class _InactiveElements { assert(_elements.contains(element)); assert(element._parent == null); _elements.remove(element); - assert(element._lifecycleState != _ElementLifecycle.active); + assert(element._lifecycleState == _ElementLifecycle.inactive); } bool debugContains(Element element) { @@ -2934,7 +2987,7 @@ class BuildOwner { buildScope._scheduleBuildFor(element); assert(() { if (debugPrintScheduleBuildForStacks) { - debugPrint("...the build scope's dirty list is now: $buildScope._dirtyElements"); + debugPrint("...the build scope's dirty list is now: ${buildScope._dirtyElements}"); } return true; }()); @@ -4516,39 +4569,32 @@ abstract class Element extends DiagnosticableTree implements BuildContext { try { final Key? key = newWidget.key; - if (key is GlobalKey) { - final Element? newChild = _retakeInactiveElement(key, newWidget); - if (newChild != null) { - assert(newChild._parent == null); - assert(() { - _debugCheckForCycles(newChild); - return true; - }()); - try { - newChild._activateWithParent(this, newSlot); - } catch (_) { - // Attempt to do some clean-up if activation fails to leave tree in a reasonable state. - try { - deactivateChild(newChild); - } catch (_) { - // Clean-up failed. Only surface original exception. - } - rethrow; - } - final Element? updatedChild = updateChild(newChild, newWidget, newSlot); - assert(newChild == updatedChild); - return updatedChild!; - } - } - final Element newChild = newWidget.createElement(); + final Element? inactiveChild = key is GlobalKey + ? _retakeInactiveElement(key, newWidget) + : null; + final Element newChild = inactiveChild ?? newWidget.createElement(); assert(() { _debugCheckForCycles(newChild); return true; }()); - newChild.mount(this, newSlot); - assert(newChild._lifecycleState == _ElementLifecycle.active); - - return newChild; + try { + if (inactiveChild != null) { + assert(inactiveChild._parent == null); + inactiveChild._activateWithParent(this, newSlot); + final Element? updatedChild = updateChild(inactiveChild, newWidget, newSlot); + assert(inactiveChild == updatedChild); + return updatedChild!; + } else { + newChild.mount(this, newSlot); + assert(newChild._lifecycleState == _ElementLifecycle.active); + return newChild; + } + } catch (_) { + // Attempt to do some clean-up if activation or mount fails + // to leave tree in a reasonable state. + _deactivateFailedChildSilently(newChild); + rethrow; + } } finally { if (isTimelineTracked) { FlutterTimeline.finishSync(); @@ -4583,6 +4629,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { /// parent proactively calls the old parent's [deactivateChild], first using /// [forgetChild] to cause the old parent to update its child model. @protected + @mustCallSuper void deactivateChild(Element child) { assert(child._parent == this); child._parent = null; @@ -4598,6 +4645,38 @@ abstract class Element extends DiagnosticableTree implements BuildContext { }()); } + void _deactivateFailedChildSilently(Element child) { + try { + child._parent = null; + child.detachRenderObject(); + _deactivateFailedSubtreeRecursively(child); + } catch (_) { + // Do not rethrow: + // The subtree has already thrown a different error and the framework is + // cleaning up on a best-effort basis. + } + } + + // This method calls _ensureDeactivated for the subtree rooted at `element`, + // supressing all exceptions thrown. + // + // This method will attempt to keep doing treewalk even one of the nodes + // failed to deactivate. + // + // The subtree has already thrown a different error and the framework is + // cleaning up on a best-effort basis. + static void _deactivateFailedSubtreeRecursively(Element element) { + try { + element.deactivate(); + } catch (_) { + element._ensureDeactivated(); + } + element._lifecycleState = _ElementLifecycle.failed; + try { + element.visitChildren(_deactivateFailedSubtreeRecursively); + } catch (_) {} + } + // The children that have been forgotten by forgetChild. This will be used in // [update] to remove the global key reservations of forgotten children. // @@ -4672,6 +4751,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { /// Implementations of this method should start with a call to the inherited /// method, as in `super.activate()`. @mustCallSuper + @visibleForOverriding void activate() { assert(_lifecycleState == _ElementLifecycle.inactive); assert(owner != null); @@ -4701,18 +4781,34 @@ abstract class Element extends DiagnosticableTree implements BuildContext { /// animation frame, if the element has not be reactivated, the framework will /// unmount the element. /// - /// This is (indirectly) called by [deactivateChild]. + /// In case of an uncaught exception when rebuild a widget subtree, the + /// framework also calls this method on the failing subtree to make sure the + /// widget tree is in a relatively consistent state. The deactivation of such + /// subtrees are performed only on a best-effort basis, and the errors thrown + /// during deactivation will not be rethrown. + /// + /// This is indirectly called by [deactivateChild]. /// /// See the lifecycle documentation for [Element] for additional information. /// /// Implementations of this method should end with a call to the inherited /// method, as in `super.deactivate()`. @mustCallSuper + @visibleForOverriding void deactivate() { assert(_lifecycleState == _ElementLifecycle.active); assert(_widget != null); // Use the private property to avoid a CastError during hot reload. - if (_dependencies?.isNotEmpty ?? false) { - for (final InheritedElement dependency in _dependencies!) { + _ensureDeactivated(); + } + + /// Removes dependencies and sets the lifecycle state of this [Element] to + /// inactive. + /// + /// This method is immediately called after [Element.deactivate], even if that + /// call throws an exception. + void _ensureDeactivated() { + if (_dependencies case final Set dependencies? when dependencies.isNotEmpty) { + for (final InheritedElement dependency in dependencies) { dependency.removeDependent(this); } // For expediency, we don't actually clear the list here, even though it's @@ -4977,8 +5073,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { @override InheritedWidget dependOnInheritedElement(InheritedElement ancestor, {Object? aspect}) { - _dependencies ??= HashSet(); - _dependencies!.add(ancestor); + (_dependencies ??= HashSet()).add(ancestor); ancestor.updateDependencies(this, aspect); return ancestor.widget as InheritedWidget; } @@ -5714,7 +5809,7 @@ abstract class ComponentElement extends Element { @override @pragma('vm:notify-debugger-on-exception') void performRebuild() { - Widget? built; + Widget built; try { assert(() { _debugDoingBuild = true; @@ -5757,6 +5852,10 @@ abstract class ComponentElement extends Element { ], ), ); + // _Make sure the old child subtree are deactivated and disposed. + try { + _child?.deactivate(); + } catch (_) {} _child = updateChild(null, built, slot); } } @@ -6167,10 +6266,7 @@ class InheritedElement extends ProxyElement { @override void debugDeactivated() { - assert(() { - assert(_dependents.isEmpty); - return true; - }()); + assert(_dependents.isEmpty); super.debugDeactivated(); } @@ -6748,6 +6844,7 @@ abstract class RenderObjectElement extends Element { } @override + @visibleForOverriding void deactivate() { super.deactivate(); assert( @@ -6758,6 +6855,7 @@ abstract class RenderObjectElement extends Element { } @override + @visibleForOverriding void unmount() { assert( !renderObject.debugDisposed!, diff --git a/packages/flutter/test/cupertino/tab_test.dart b/packages/flutter/test/cupertino/tab_test.dart index 8fe9fd2605e..82aafb83206 100644 --- a/packages/flutter/test/cupertino/tab_test.dart +++ b/packages/flutter/test/cupertino/tab_test.dart @@ -95,10 +95,6 @@ void main() { expect(tester.takeException(), isFlutterError); expect(unknownForRouteCalled, '/'); - - // Work-around for https://github.com/flutter/flutter/issues/65655. - await tester.pumpWidget(Container()); - expect(tester.takeException(), isAssertionError); }, ); diff --git a/packages/flutter/test/material/app_test.dart b/packages/flutter/test/material/app_test.dart index 302b387f871..9d06b2eb594 100644 --- a/packages/flutter/test/material/app_test.dart +++ b/packages/flutter/test/material/app_test.dart @@ -347,10 +347,6 @@ void main() { ); expect(tester.takeException(), isFlutterError); expect(log, ['onGenerateRoute /', 'onUnknownRoute /']); - - // Work-around for https://github.com/flutter/flutter/issues/65655. - await tester.pumpWidget(Container()); - expect(tester.takeException(), isAssertionError); }, ); diff --git a/packages/flutter/test/widgets/framework_test.dart b/packages/flutter/test/widgets/framework_test.dart index 98b141cddbc..6da409f7d60 100644 --- a/packages/flutter/test/widgets/framework_test.dart +++ b/packages/flutter/test/widgets/framework_test.dart @@ -2075,6 +2075,71 @@ The findRenderObject() method was called for the following element: ), ); }); + + testWidgets( + 'widget is not active if throw in deactivated', + experimentalLeakTesting: LeakTesting.settings + .withIgnoredAll(), // leaking by design because of exception + (WidgetTester tester) async { + final FlutterExceptionHandler? onError = FlutterError.onError; + FlutterError.onError = (_) {}; + const Widget child = Placeholder(); + await tester.pumpWidget( + StatefulWidgetSpy(onDeactivate: (_) => throw StateError('kaboom'), child: child), + ); + final Element element = tester.element(find.byWidget(child)); + assert(element.debugIsActive); + + await tester.pumpWidget(const SizedBox()); + FlutterError.onError = onError; + expect(element.debugIsActive, false); + expect(element.debugIsDefunct, false); + }, + ); + + testWidgets( + 'widget is not active if throw in activated', + experimentalLeakTesting: LeakTesting.settings + .withIgnoredAll(), // leaking by design because of exception + (WidgetTester tester) async { + final FlutterExceptionHandler? onError = FlutterError.onError; + FlutterError.onError = (_) {}; + const Widget child = Placeholder(); + final Widget widget = StatefulWidgetSpy( + key: GlobalKey(), + onActivate: (_) => throw StateError('kaboom'), + child: child, + ); + await tester.pumpWidget(widget); + final Element element = tester.element(find.byWidget(child)); + + await tester.pumpWidget(MetaData(child: widget)); + FlutterError.onError = onError; + expect(element.debugIsActive, false); + expect(element.debugIsDefunct, false); + }, + ); + + testWidgets( + 'widget is unmounted if throw in dispose', + experimentalLeakTesting: LeakTesting.settings + .withIgnoredAll(), // leaking by design because of exception + (WidgetTester tester) async { + final FlutterExceptionHandler? onError = FlutterError.onError; + FlutterError.onError = (_) {}; + const Widget child = Placeholder(); + final Widget widget = StatefulWidgetSpy( + onDispose: (_) => throw StateError('kaboom'), + child: child, + ); + await tester.pumpWidget(widget); + final Element element = tester.element(find.byWidget(child)); + await tester.pumpWidget(child); + + FlutterError.onError = onError; + expect(element.debugIsDefunct, true); + }, + ); } class _TestInheritedElement extends InheritedElement { @@ -2323,6 +2388,7 @@ class StatefulWidgetSpy extends StatefulWidget { this.onDeactivate, this.onActivate, this.onDidUpdateWidget, + this.child = const SizedBox(), }); final void Function(BuildContext)? onBuild; @@ -2332,6 +2398,7 @@ class StatefulWidgetSpy extends StatefulWidget { final void Function(BuildContext)? onDeactivate; final void Function(BuildContext)? onActivate; final void Function(BuildContext)? onDidUpdateWidget; + final Widget child; @override State createState() => _StatefulWidgetSpyState(); @@ -2377,7 +2444,7 @@ class _StatefulWidgetSpyState extends State { @override Widget build(BuildContext context) { widget.onBuild?.call(context); - return Container(); + return widget.child; } } diff --git a/packages/flutter/test/widgets/memory_allocations_test.dart b/packages/flutter/test/widgets/memory_allocations_test.dart index 7cdbefe5536..83f6b958541 100644 --- a/packages/flutter/test/widgets/memory_allocations_test.dart +++ b/packages/flutter/test/widgets/memory_allocations_test.dart @@ -80,6 +80,7 @@ class _TestElement extends RenderObjectElement with RootElementMixin { final FocusManager newFocusManager = FocusManager(); assignOwner(BuildOwner(focusManager: newFocusManager)); mount(null, null); + // ignore: invalid_use_of_visible_for_overriding_member deactivate(); } @@ -145,6 +146,7 @@ Future<_EventStats> _activateFlutterObjectsAndReturnCountOfEvents() async { element.makeInactive(); result.creations += 4; // 1 for the new BuildOwner, 1 for the new FocusManager, 1 for the new FocusScopeNode, 1 for the new _HighlightModeManager + // ignore: invalid_use_of_visible_for_overriding_member element.unmount(); result.disposals += 2; // 1 for the old BuildOwner, 1 for the element renderObject.dispose(); diff --git a/packages/flutter/test/widgets/navigator_restoration_test.dart b/packages/flutter/test/widgets/navigator_restoration_test.dart index 8e7b900fdc4..3e0abfecfd4 100644 --- a/packages/flutter/test/widgets/navigator_restoration_test.dart +++ b/packages/flutter/test/widgets/navigator_restoration_test.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; @@ -1095,18 +1094,6 @@ void main() { (exception as AssertionError).message, contains('All routes returned by onGenerateInitialRoutes are not restorable.'), ); - - // The previous assert leaves the widget tree in a broken state, so the - // following code catches any remaining exceptions from attempting to build - // new widget tree. - final FlutterExceptionHandler? oldHandler = FlutterError.onError; - dynamic remainingException; - FlutterError.onError = (FlutterErrorDetails details) { - remainingException ??= details.exception; - }; - await tester.pumpWidget(Container(key: UniqueKey())); - FlutterError.onError = oldHandler; - expect(remainingException, isAssertionError); }, ); } diff --git a/packages/flutter/test/widgets/tree_shape_test.dart b/packages/flutter/test/widgets/tree_shape_test.dart index 76dabaa83bb..d32ac4901ba 100644 --- a/packages/flutter/test/widgets/tree_shape_test.dart +++ b/packages/flutter/test/widgets/tree_shape_test.dart @@ -94,29 +94,32 @@ void main() { }, ); - testWidgets('A View can not be moved via GlobalKey to be a child of a RenderObject', ( - WidgetTester tester, - ) async { - final Widget globalKeyedView = View( - key: GlobalKey(), - view: FakeView(tester.view), - child: const ColoredBox(color: Colors.red), - ); + testWidgets( + 'A View can not be moved via GlobalKey to be a child of a RenderObject', + experimentalLeakTesting: LeakTesting.settings + .withIgnoredAll(), // leaking by design because of exception + (WidgetTester tester) async { + final Widget globalKeyedView = View( + key: GlobalKey(), + view: FakeView(tester.view), + child: const ColoredBox(color: Colors.red), + ); - await tester.pumpWidget(wrapWithView: false, globalKeyedView); - expect(tester.takeException(), isNull); + await tester.pumpWidget(wrapWithView: false, globalKeyedView); + expect(tester.takeException(), isNull); - await tester.pumpWidget(wrapWithView: false, View(view: tester.view, child: globalKeyedView)); + await tester.pumpWidget(wrapWithView: false, View(view: tester.view, child: globalKeyedView)); - expect( - tester.takeException(), - isFlutterError.having( - (FlutterError error) => error.message, - 'message', - contains('cannot maintain an independent render tree at its current location.'), - ), - ); - }); + expect( + tester.takeException(), + isFlutterError.having( + (FlutterError error) => error.message, + 'message', + contains('cannot maintain an independent render tree at its current location.'), + ), + ); + }, + ); testWidgets('The view property of a ViewAnchor cannot be a render object widget', ( WidgetTester tester, diff --git a/packages/flutter/test_release/widgets/memory_allocations_test.dart b/packages/flutter/test_release/widgets/memory_allocations_test.dart index 659c6ccbf43..b901c0ab3a8 100644 --- a/packages/flutter/test_release/widgets/memory_allocations_test.dart +++ b/packages/flutter/test_release/widgets/memory_allocations_test.dart @@ -60,6 +60,7 @@ class _TestElement extends RenderTreeRootElement with RootElementMixin { void makeInactive() { assignOwner(BuildOwner(focusManager: FocusManager())); mount(null, null); + // ignore: invalid_use_of_visible_for_overriding_member deactivate(); } @@ -95,6 +96,7 @@ class _MyStatefulWidgetState extends State<_MyStatefulWidget> { Future _activateFlutterObjects(WidgetTester tester) async { final _TestElement element = _TestElement(); element.makeInactive(); + // ignore: invalid_use_of_visible_for_overriding_member element.unmount(); // Create and dispose State: