Skip to content

F
Front-End
Unverified Commit 63c49c3d authored by Ren You's avatar Ren You Committed by GitHub
Browse files
Options

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

This reverts commit c75edc26.
parent c75edc26
Hide whitespace changes
Inline Side-by-side
Showing with 12 additions and 240 deletions
examples/layers/rendering/src/sector_layout.dart
View file @ 63c49c3d
......@@ -455,6 +455,7 @@ class RenderSectorSlice extends RenderSectorWithChildren {
}
class RenderBoxToRenderSectorAdapter extends RenderBox with RenderObjectWithChildMixin<RenderSector> {
RenderBoxToRenderSectorAdapter({ double innerRadius = 0.0, RenderSector? child })
: _innerRadius = innerRadius {
this.child = child;
......@@ -566,6 +567,7 @@ class RenderBoxToRenderSectorAdapter extends RenderBox with RenderObjectWithChil
result.add(BoxHitTestEntry(this, position));
return true;
}
}
class RenderSolidColor extends RenderDecoratedSector {
......
examples/layers/widgets/sectors.dart
View file @ 63c49c3d
......@@ -90,13 +90,6 @@ class SectorAppState extends State<SectorApp> {
});
}
void recursivelyDisposeChildren(RenderObject parent) {
parent.visitChildren((RenderObject child) {
recursivelyDisposeChildren(child);
child.dispose();
});
}
Widget buildBody() {
return Column(
mainAxisAlignment: MainAxisAlignment.spaceBetween,
......@@ -114,12 +107,7 @@ class SectorAppState extends State<SectorApp> {
Container(
padding: const EdgeInsets.all(4.0),
margin: const EdgeInsets.only(right: 10.0),
child: WidgetToRenderBoxAdapter(
renderBox: sectorAddIcon,
onUnmount: () {
recursivelyDisposeChildren(sectorAddIcon);
},
),
child: WidgetToRenderBoxAdapter(renderBox: sectorAddIcon),
),
const Text('ADD SECTOR'),
],
......@@ -134,12 +122,7 @@ class SectorAppState extends State<SectorApp> {
Container(
padding: const EdgeInsets.all(4.0),
margin: const EdgeInsets.only(right: 10.0),
child: WidgetToRenderBoxAdapter(
renderBox: sectorRemoveIcon,
onUnmount: () {
recursivelyDisposeChildren(sectorRemoveIcon);
},
),
child: WidgetToRenderBoxAdapter(renderBox: sectorRemoveIcon),
),
const Text('REMOVE SECTOR'),
],
......@@ -159,9 +142,6 @@ class SectorAppState extends State<SectorApp> {
child: WidgetToRenderBoxAdapter(
renderBox: sectors,
onBuild: doUpdates,
onUnmount: () {
recursivelyDisposeChildren(sectors);
},
),
),
),
......
packages/flutter/lib/src/rendering/editable.dart
View file @ 63c49c3d
......@@ -283,15 +283,6 @@ 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
View file @ 63c49c3d
......@@ -438,13 +438,6 @@ 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
View file @ 63c49c3d
......@@ -1111,19 +1111,6 @@ 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
......@@ -1243,50 +1230,6 @@ 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(() {
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.
......@@ -1424,10 +1367,6 @@ 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;
......@@ -1462,7 +1401,6 @@ 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
......@@ -1674,7 +1612,6 @@ 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);
......@@ -1744,7 +1681,6 @@ 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);
......@@ -2104,7 +2040,6 @@ 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;
......@@ -2203,7 +2138,6 @@ 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;
......@@ -2288,7 +2222,6 @@ 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);
......@@ -2301,7 +2234,6 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
}
void _paintWithContext(PaintingContext context, Offset offset) {
assert(!_debugDisposed);
assert(() {
if (_debugDoingThisPaint) {
throw FlutterError.fromParts(<DiagnosticsNode>[
......@@ -2525,7 +2457,6 @@ 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);
......@@ -2648,7 +2579,6 @@ 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;
......@@ -2894,10 +2824,6 @@ 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?;
......@@ -3283,7 +3209,6 @@ 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
View file @ 63c49c3d
......@@ -3152,6 +3152,7 @@ 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
View file @ 63c49c3d
......@@ -6232,11 +6232,6 @@ 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.
///
......@@ -6244,7 +6239,6 @@ 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
......@@ -6253,9 +6247,6 @@ 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
......@@ -6264,25 +6255,6 @@ 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;
......@@ -6290,12 +6262,6 @@ 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
View file @ 63c49c3d
......@@ -3199,13 +3199,10 @@ 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._lifecycleState == _ElementLifecycle.defunct) {
return;
} else if (element is RenderObjectElement) {
if (element is RenderObjectElement)
result = element.renderObject;
} else {
else
element.visitChildren(visit);
}
}
visit(this);
return result;
......@@ -3844,10 +3841,6 @@ 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
......@@ -5429,13 +5422,8 @@ 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 {
assert(_renderObject != null, '$runtimeType unmounted');
return _renderObject!;
}
RenderObject get renderObject => _renderObject!;
RenderObject? _renderObject;
bool _debugDoingBuild = false;
......@@ -5503,7 +5491,6 @@ abstract class RenderObjectElement extends Element {
return true;
}());
_renderObject = widget.createRenderObject(this);
assert(!_renderObject!.debugDisposed!);
assert(() {
_debugDoingBuild = false;
return true;
......@@ -5781,11 +5768,6 @@ 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(
......@@ -5794,8 +5776,6 @@ 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
View file @ 63c49c3d
......@@ -268,8 +268,8 @@ class _SliverPersistentHeaderElement extends RenderObjectElement {
@override
void unmount() {
renderObject._element = null;
super.unmount();
renderObject._element = null;
}
@override
......
packages/flutter/test/rendering/image_test.dart
View file @ 63c49c3d
......@@ -175,7 +175,7 @@ Future<void> main() async {
expect(image.colorBlendMode, BlendMode.color);
});
test('RenderImage disposes its image', () async {
test('Render image disposes its image', () async {
final ui.Image image = await createTestImage(width: 10, height: 10, cache: false);
expect(image.debugGetOpenHandleStackTraces()!.length, 1);
......@@ -192,7 +192,7 @@ Future<void> main() async {
expect(image.debugGetOpenHandleStackTraces()!.length, 0);
}, skip: kIsWeb); // Web doesn't track open image handles.
test('RenderImage does not dispose its image if setting the same image twice', () async {
test('Render image 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);
......@@ -208,19 +208,4 @@ 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
View file @ 63c49c3d
......@@ -144,38 +144,6 @@ 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
......@@ -220,19 +188,12 @@ 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(allowPaintBounds); // For some tests, this should not get called.
assert(false); // The test shouldn't call this.
return Rect.zero;
}
......
packages/flutter/test/widgets/framework_test.dart
View file @ 63c49c3d
......@@ -1619,18 +1619,6 @@ void main() {
await pumpWidget(Container());
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);
});
}
class _WidgetWithNoVisitChildren extends StatelessWidget {
......
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

HIAST - GitLab