Reland "Dispose render objects when owning element is unmounted. (#82883)" (#83790) (#83920)

examples/layers/rendering/src/sector_layout.dart

@@ -455,7 +455,6 @@ class RenderSectorSlice extends RenderSectorWithChildren {
 }
 
 class RenderBoxToRenderSectorAdapter extends RenderBox with RenderObjectWithChildMixin<RenderSector> {
-
   RenderBoxToRenderSectorAdapter({ double innerRadius = 0.0, RenderSector? child })
     : _innerRadius = innerRadius {
     this.child = child;
@@ -567,7 +566,6 @@ class RenderBoxToRenderSectorAdapter extends RenderBox with RenderObjectWithChil
     result.add(BoxHitTestEntry(this, position));
     return true;
   }
-
 }
 
 class RenderSolidColor extends RenderDecoratedSector {

examples/layers/widgets/sectors.dart

@@ -90,6 +90,13 @@ class SectorAppState extends State<SectorApp> {
     });
   }
 
+  void recursivelyDisposeChildren(RenderObject parent) {
+    parent.visitChildren((RenderObject child) {
+      recursivelyDisposeChildren(child);
+      child.dispose();
+    });
+  }
+
   Widget buildBody() {
     return Column(
       mainAxisAlignment: MainAxisAlignment.spaceBetween,
@@ -107,7 +114,12 @@ class SectorAppState extends State<SectorApp> {
                       Container(
                         padding: const EdgeInsets.all(4.0),
                         margin: const EdgeInsets.only(right: 10.0),
-                        child: WidgetToRenderBoxAdapter(renderBox: sectorAddIcon),
+                        child: WidgetToRenderBoxAdapter(
+                          renderBox: sectorAddIcon,
+                          onUnmount: () {
+                            recursivelyDisposeChildren(sectorAddIcon);
+                          },
+                        ),
                       ),
                       const Text('ADD SECTOR'),
                     ],
@@ -122,7 +134,12 @@ class SectorAppState extends State<SectorApp> {
                       Container(
                         padding: const EdgeInsets.all(4.0),
                         margin: const EdgeInsets.only(right: 10.0),
-                        child: WidgetToRenderBoxAdapter(renderBox: sectorRemoveIcon),
+                        child: WidgetToRenderBoxAdapter(
+                          renderBox: sectorRemoveIcon,
+                          onUnmount: () {
+                            recursivelyDisposeChildren(sectorRemoveIcon);
+                          },
+                        ),
                       ),
                       const Text('REMOVE SECTOR'),
                     ],
@@ -142,6 +159,9 @@ class SectorAppState extends State<SectorApp> {
             child: WidgetToRenderBoxAdapter(
               renderBox: sectors,
               onBuild: doUpdates,
+              onUnmount: () {
+                recursivelyDisposeChildren(sectors);
+              },
             ),
           ),
         ),

packages/flutter/lib/src/rendering/editable.dart

@@ -283,6 +283,15 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
   _RenderEditableCustomPaint? _foregroundRenderObject;
   _RenderEditableCustomPaint? _backgroundRenderObject;
 
+  @override
+  void dispose() {
+    _foregroundRenderObject?.dispose();
+    _foregroundRenderObject = null;
+    _backgroundRenderObject?.dispose();
+    _backgroundRenderObject = null;
+    super.dispose();
+  }
+
   void _updateForegroundPainter(RenderEditablePainter? newPainter) {
     final _CompositeRenderEditablePainter effectivePainter = newPainter == null
       ? _builtInForegroundPainters

packages/flutter/lib/src/rendering/image.dart

@@ -438,6 +438,13 @@ class RenderImage extends RenderBox {
     );
   }
 
+  @override
+  void dispose() {
+    _image?.dispose();
+    _image = null;
+    super.dispose();
+  }
+
   @override
   void debugFillProperties(DiagnosticPropertiesBuilder properties) {
     super.debugFillProperties(properties);

packages/flutter/lib/src/rendering/object.dart

@@ -1111,6 +1111,19 @@ class PipelineOwner {
 /// The [RenderBox] subclass introduces the opinion that the layout
 /// system uses Cartesian coordinates.
 ///
+/// ## Lifecycle
+///
+/// A [RenderObject] must [dispose] when it is no longer needed. The creator
+/// of the object is responsible for disposing of it. Typically, the creator is
+/// a [RenderObjectElement], and that element will dispose the object it creates
+/// when it is unmounted.
+///
+/// [RenderObject]s are responsible for cleaning up any expensive resources
+/// they hold when [dispose] is called, such as [Picture] or [Image] objects.
+/// This includes any [Layer]s that the render object has directly created. The
+/// base implementation of dispose will nullify the [layer] property. Subclasses
+/// must also nullify any other layer(s) it directly creates.
+///
 /// ## Writing a RenderObject subclass
 ///
 /// In most cases, subclassing [RenderObject] itself is overkill, and
@@ -1230,6 +1243,52 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
     });
   }
 
+  /// Whether this has been disposed.
+  ///
+  /// If asserts are disabled, this property is always null.
+  bool? get debugDisposed {
+    bool? disposed;
+    assert(() {
+      disposed = _debugDisposed;
+      return true;
+    }());
+    return disposed;
+  }
+
+  bool _debugDisposed = false;
+
+  /// Release any resources held by this render object.
+  ///
+  /// The object that creates a RenderObject is in charge of disposing it.
+  /// If this render object has created any children directly, it must dispose
+  /// of those children in this method as well. It must not dispose of any
+  /// children that were created by some other object, such as
+  /// a [RenderObjectElement]. Those children will be disposed when that
+  /// element unmounts, which may be delayed if the element is moved to another
+  /// part of the tree.
+  ///
+  /// Implementations of this method must end with a call to the inherited
+  /// method, as in `super.dispose()`.
+  ///
+  /// The object is no longer usable after calling dispose.
+  @mustCallSuper
+  void dispose() {
+    assert(!_debugDisposed);
+    _layer = null;
+    assert(() {
+      // TODO(dnfield): Enable this assert once clients have had a chance to
+      // migrate.
+      // visitChildren((RenderObject child) {
+      //   assert(
+      //     child.debugDisposed!,
+      //     '${child.runtimeType} (child of $runtimeType) must be disposed before calling super.dispose().',
+      //   );
+      // });
+      _debugDisposed = true;
+      return true;
+    }());
+  }
+
   // LAYOUT
 
   /// Data for use by the parent render object.
@@ -1367,6 +1426,10 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   bool get _debugCanPerformMutations {
     late bool result;
     assert(() {
+      if (_debugDisposed) {
+        result = false;
+        return true;
+      }
       if (owner != null && !owner!.debugDoingLayout) {
         result = true;
         return true;
@@ -1401,6 +1464,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
 
   @override
   void attach(PipelineOwner owner) {
+    assert(!_debugDisposed);
     super.attach(owner);
     // If the node was dirtied in some way while unattached, make sure to add
     // it to the appropriate dirty list now that an owner is available
@@ -1612,6 +1676,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   ///
   /// See [RenderView] for an example of how this function is used.
   void scheduleInitialLayout() {
+    assert(!_debugDisposed);
     assert(attached);
     assert(parent is! RenderObject);
     assert(!owner!._debugDoingLayout);
@@ -1681,6 +1746,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   /// work to update its layout information.
   @pragma('vm:notify-debugger-on-exception')
   void layout(Constraints constraints, { bool parentUsesSize = false }) {
+    assert(!_debugDisposed);
     if (!kReleaseMode && debugProfileLayoutsEnabled)
       Timeline.startSync('$runtimeType',  arguments: timelineArgumentsIndicatingLandmarkEvent);
 
@@ -2040,6 +2106,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   /// it cannot be the case that _only_ the compositing bits changed,
   /// something else will have scheduled a frame for us.
   void markNeedsCompositingBitsUpdate() {
+    assert(!_debugDisposed);
     if (_needsCompositingBitsUpdate)
       return;
     _needsCompositingBitsUpdate = true;
@@ -2138,6 +2205,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   ///    layer, thus limiting the number of nodes that [markNeedsPaint] must mark
   ///    dirty.
   void markNeedsPaint() {
+    assert(!_debugDisposed);
     assert(owner == null || !owner!.debugDoingPaint);
     if (_needsPaint)
       return;
@@ -2222,6 +2290,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   ///
   /// This might be called if, e.g., the device pixel ratio changed.
   void replaceRootLayer(OffsetLayer rootLayer) {
+    assert(!_debugDisposed);
     assert(rootLayer.attached);
     assert(attached);
     assert(parent is! RenderObject);
@@ -2234,6 +2303,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   }
 
   void _paintWithContext(PaintingContext context, Offset offset) {
+    assert(!_debugDisposed);
     assert(() {
       if (_debugDoingThisPaint) {
         throw FlutterError.fromParts(<DiagnosticsNode>[
@@ -2457,6 +2527,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   ///
   /// See [RendererBinding] for an example of how this function is used.
   void scheduleInitialSemantics() {
+    assert(!_debugDisposed);
     assert(attached);
     assert(parent is! RenderObject);
     assert(!owner!._debugDoingSemantics);
@@ -2579,6 +2650,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   /// [RenderObject] as annotated by [describeSemanticsConfiguration] changes in
   /// any way to update the semantics tree.
   void markNeedsSemanticsUpdate() {
+    assert(!_debugDisposed);
     assert(!attached || !owner!._debugDoingSemantics);
     if (!attached || owner!._semanticsOwner == null) {
       _cachedSemanticsConfiguration = null;
@@ -2824,6 +2896,10 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
   @override
   String toStringShort() {
     String header = describeIdentity(this);
+    if (_debugDisposed) {
+      header += ' DISPOSED';
+      return header;
+    }
     if (_relayoutBoundary != null && _relayoutBoundary != this) {
       int count = 1;
       RenderObject? target = parent as RenderObject?;
@@ -3209,6 +3285,7 @@ mixin ContainerRenderObjectMixin<ChildType extends RenderObject, ParentDataType
       }
     }
   }
+
   /// Insert child into this render object's child list after the given child.
   ///
   /// If `after` is null, then this inserts the child at the start of the list,

packages/flutter/lib/src/rendering/proxy_box.dart

@@ -3146,7 +3146,6 @@ class RenderRepaintBoundary extends RenderProxyBox {
     return offsetLayer.toImage(Offset.zero & size, pixelRatio: pixelRatio);
   }
 
-
   /// The number of times that this render object repainted at the same time as
   /// its parent. Repaint boundaries are only useful when the parent and child
   /// paint at different times. When both paint at the same time, the repaint

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

@@ -6234,6 +6234,11 @@ class DefaultAssetBundle extends InheritedWidget {
 /// A given render object can be placed at most once in the widget tree. This
 /// widget enforces that restriction by keying itself using a [GlobalObjectKey]
 /// for the given render object.
+///
+/// This widget will call [RenderObject.dispose] on the [renderBox] when it is
+/// unmounted. After that point, the [renderBox] will be unusable. If any
+/// children have been added to the [renderBox], they must be disposed in the
+/// [onUnmount] callback.
 class WidgetToRenderBoxAdapter extends LeafRenderObjectWidget {
   /// Creates an adapter for placing a specific [RenderBox] in the widget tree.
   ///
@@ -6241,6 +6246,7 @@ class WidgetToRenderBoxAdapter extends LeafRenderObjectWidget {
   WidgetToRenderBoxAdapter({
     required this.renderBox,
     this.onBuild,
+    this.onUnmount,
   }) : assert(renderBox != null),
        // WidgetToRenderBoxAdapter objects are keyed to their render box. This
        // prevents the widget being used in the widget hierarchy in two different
@@ -6249,6 +6255,9 @@ class WidgetToRenderBoxAdapter extends LeafRenderObjectWidget {
        super(key: GlobalObjectKey(renderBox));
 
   /// The render box to place in the widget tree.
+  ///
+  /// This widget takes ownership of the render object. When it is unmounted,
+  /// it also calls [RenderObject.dispose].
   final RenderBox renderBox;
 
   /// Called when it is safe to update the render box and its descendants. If
@@ -6257,6 +6266,25 @@ class WidgetToRenderBoxAdapter extends LeafRenderObjectWidget {
   /// tree will be dirty.
   final VoidCallback? onBuild;
 
+  /// Called when it is safe to dispose of children that were manually added to
+  /// the [renderBox].
+  ///
+  /// Do not dispose the [renderBox] itself, as it will be disposed by the
+  /// framework automatically. However, during that process the framework will
+  /// check that all children of the [renderBox] have also been disposed.
+  /// Typically, child [RenderObject]s are disposed by corresponding [Element]s
+  /// when they are unmounted. However, child render objects that were manually
+  /// added do not have corresponding [Element]s to manage their lifecycle, and
+  /// need to be manually disposed here.
+  ///
+  /// See also:
+  ///
+  ///   * [RenderObjectElement.unmount], which invokes this callback before
+  ///     disposing of its render object.
+  ///   * [RenderObject.dispose], which instructs a render object to release
+  ///     any resources it may be holding.
+  final VoidCallback? onUnmount;
+
   @override
   RenderBox createRenderObject(BuildContext context) => renderBox;
 
@@ -6264,6 +6292,12 @@ class WidgetToRenderBoxAdapter extends LeafRenderObjectWidget {
   void updateRenderObject(BuildContext context, RenderBox renderObject) {
     onBuild?.call();
   }
+
+  @override
+  void didUnmountRenderObject(RenderObject renderObject) {
+    assert(renderObject == renderBox);
+    onUnmount?.call();
+  }
 }
 
 

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

@@ -3200,10 +3200,13 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
     RenderObject? result;
     void visit(Element element) {
       assert(result == null); // this verifies that there's only one child
-      if (element is RenderObjectElement)
+      if (element._lifecycleState == _ElementLifecycle.defunct) {
+        return;
+      } else if (element is RenderObjectElement) {
         result = element.renderObject;
-      else
+      } else {
         element.visitChildren(visit);
+      }
     }
     visit(this);
     return result;
@@ -3842,6 +3845,10 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
   /// After this function is called, the element will not be incorporated into
   /// the tree again.
   ///
+  /// Any resources this element holds should be released at this point. For
+  /// example, [RenderObjectElement.unmount] calls [RenderObject.dispose] and
+  /// nulls out its reference to the render object.
+  ///
   /// See the lifecycle documentation for [Element] for additional information.
   ///
   /// Implementations of this method should end with a call to the inherited
@@ -5441,8 +5448,13 @@ abstract class RenderObjectElement extends Element {
   RenderObjectWidget get widget => super.widget as RenderObjectWidget;
 
   /// The underlying [RenderObject] for this element.
+  ///
+  /// If this element has been [unmount]ed, this getter will throw.
   @override
-  RenderObject get renderObject => _renderObject!;
+  RenderObject get renderObject {
+    assert(_renderObject != null, '$runtimeType unmounted');
+    return _renderObject!;
+  }
   RenderObject? _renderObject;
 
   bool _debugDoingBuild = false;
@@ -5510,6 +5522,7 @@ abstract class RenderObjectElement extends Element {
       return true;
     }());
     _renderObject = widget.createRenderObject(this);
+    assert(!_renderObject!.debugDisposed!);
     assert(() {
       _debugDoingBuild = false;
       return true;
@@ -5787,6 +5800,11 @@ abstract class RenderObjectElement extends Element {
 
   @override
   void unmount() {
+    assert(
+      !renderObject.debugDisposed!,
+      'A RenderObject was disposed prior to its owning element being unmounted: '
+      '$renderObject',
+    );
     final RenderObjectWidget oldWidget = widget;
     super.unmount();
     assert(
@@ -5795,6 +5813,8 @@ abstract class RenderObjectElement extends Element {
       'RenderObjectElement: $renderObject',
     );
     oldWidget.didUnmountRenderObject(renderObject);
+    _renderObject!.dispose();
+    _renderObject = null;
   }
 
   void _updateParentData(ParentDataWidget<ParentData> parentDataWidget) {

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

@@ -268,8 +268,8 @@ class _SliverPersistentHeaderElement extends RenderObjectElement {
 
   @override
   void unmount() {
-    super.unmount();
     renderObject._element = null;
+    super.unmount();
   }
 
   @override

packages/flutter/test/rendering/image_test.dart

@@ -174,7 +174,7 @@ Future<void> main() async {
     expect(image.colorBlendMode, BlendMode.color);
   });
 
-  test('Render image disposes its image', () async {
+  test('RenderImage disposes its image', () async {
     final ui.Image image = await createTestImage(width: 10, height: 10, cache: false);
     expect(image.debugGetOpenHandleStackTraces()!.length, 1);
 
@@ -191,7 +191,7 @@ Future<void> main() async {
     expect(image.debugGetOpenHandleStackTraces()!.length, 0);
   }, skip: kIsWeb); // Web doesn't track open image handles.
 
-  test('Render image does not dispose its image if setting the same image twice', () async {
+  test('RenderImage does not dispose its image if setting the same image twice', () async {
     final ui.Image image = await createTestImage(width: 10, height: 10, cache: false);
     expect(image.debugGetOpenHandleStackTraces()!.length, 1);
 
@@ -207,4 +207,19 @@ Future<void> main() async {
     image.dispose();
     expect(image.debugGetOpenHandleStackTraces()!.length, 0);
   }, skip: kIsWeb); // Web doesn't track open image handles.
+
+  test('Render image disposes its image when it is disposed', () async {
+    final ui.Image image = await createTestImage(width: 10, height: 10, cache: false);
+    expect(image.debugGetOpenHandleStackTraces()!.length, 1);
+
+    final RenderImage renderImage = RenderImage(image: image.clone());
+    expect(image.debugGetOpenHandleStackTraces()!.length, 2);
+
+    renderImage.dispose();
+    expect(image.debugGetOpenHandleStackTraces()!.length, 1);
+    expect(renderImage.image, null);
+
+    image.dispose();
+    expect(image.debugGetOpenHandleStackTraces()!.length, 0);
+  }, skip: kIsWeb); // Web doesn't track open image handles.
 }

packages/flutter/test/rendering/object_test.dart

@@ -144,6 +144,38 @@ void main() {
       return context.pushOpacity(offset, 100, painter, oldLayer: oldLayer as OpacityLayer?);
     });
   });
+
+  test('RenderObject.dispose sets debugDisposed to true', () {
+    final TestRenderObject renderObject = TestRenderObject();
+    expect(renderObject.debugDisposed, false);
+    renderObject.dispose();
+    expect(renderObject.debugDisposed, true);
+    expect(renderObject.toStringShort(), contains('DISPOSED'));
+  });
+
+  test('RenderObject.dispose null the layer on repaint boundaries', () {
+    final TestRenderObject renderObject = TestRenderObject(allowPaintBounds: true);
+    // Force a layer to get set.
+    renderObject.isRepaintBoundary = true;
+    PaintingContext.repaintCompositedChild(renderObject, debugAlsoPaintedParent: true);
+    expect(renderObject.debugLayer, isA<OffsetLayer>());
+
+    // Dispose with repaint boundary still being true.
+    renderObject.dispose();
+    expect(renderObject.debugLayer, null);
+  });
+
+  test('RenderObject.dispose nulls the layer on non-repaint boundaries', () {
+    final TestRenderObject renderObject = TestRenderObject(allowPaintBounds: true);
+    // Force a layer to get set.
+    renderObject.isRepaintBoundary = true;
+    PaintingContext.repaintCompositedChild(renderObject, debugAlsoPaintedParent: true);
+
+    // Dispose with repaint boundary being false.
+    renderObject.isRepaintBoundary = false;
+    renderObject.dispose();
+    expect(renderObject.debugLayer, null);
+  });
 }
 
 // Tests the create-update cycle by pumping two frames. The first frame has no
@@ -188,12 +220,19 @@ class _TestCustomLayerBox extends RenderBox {
 class TestParentData extends ParentData with ContainerParentDataMixin<RenderBox> { }
 
 class TestRenderObject extends RenderObject {
+  TestRenderObject({this.allowPaintBounds = false});
+
+  final bool allowPaintBounds;
+
+  @override
+  bool isRepaintBoundary = false;
+
   @override
   void debugAssertDoesMeetConstraints() { }
 
   @override
   Rect get paintBounds {
-    assert(false); // The test shouldn't call this.
+    assert(allowPaintBounds); // For some tests, this should not get called.
     return Rect.zero;
   }
 

packages/flutter/test/widgets/framework_test.dart

@@ -1620,6 +1620,18 @@ void main() {
     expect(states, <String>['deactivate', 'dispose']);
   });
 
+  testWidgets('RenderObjectElement.unmount dispsoes of its renderObject', (WidgetTester tester) async {
+    await tester.pumpWidget(const Placeholder());
+    final RenderObjectElement element = tester.allElements.whereType<RenderObjectElement>().first;
+    final RenderObject renderObject = element.renderObject;
+    expect(renderObject.debugDisposed, false);
+
+    await tester.pumpWidget(Container());
+
+    expect(() => element.renderObject, throwsAssertionError);
+    expect(renderObject.debugDisposed, true);
+  });
+
   testWidgets('Getting the render object of an unmounted element throws', (WidgetTester tester) async {
     await tester.pumpWidget(const _StatefulLeaf());
     final StatefulElement element = tester.element<StatefulElement>(find.byType(_StatefulLeaf));