Unverified Commit 4d9a3753 authored by Dan Field's avatar Dan Field Committed by GitHub

Assert disposed layers are not reused (#85798)

parent 3ca4e288
...@@ -148,8 +148,7 @@ abstract class Layer extends AbstractNode with DiagnosticableTreeMixin { ...@@ -148,8 +148,7 @@ abstract class Layer extends AbstractNode with DiagnosticableTreeMixin {
}()); }());
return disposed; return disposed;
} }
// TODO(dnfield): https://github.com/flutter/flutter/issues/85066 bool _debugDisposed = false;
final bool _debugDisposed = false;
/// Set when this layer is appended to a [ContainerLayer], and /// Set when this layer is appended to a [ContainerLayer], and
/// unset when it is removed. /// unset when it is removed.
...@@ -220,8 +219,7 @@ abstract class Layer extends AbstractNode with DiagnosticableTreeMixin { ...@@ -220,8 +219,7 @@ abstract class Layer extends AbstractNode with DiagnosticableTreeMixin {
'Do not directly call dispose on a $runtimeType. Instead, ' 'Do not directly call dispose on a $runtimeType. Instead, '
'use createHandle and LayerHandle.dispose.', 'use createHandle and LayerHandle.dispose.',
); );
// TODO(dnfield): enable this. https://github.com/flutter/flutter/issues/85066 _debugDisposed = true;
// _debugDisposed = true;
return true; return true;
}()); }());
_engineLayer?.dispose(); _engineLayer?.dispose();
......
...@@ -602,13 +602,11 @@ void main() { ...@@ -602,13 +602,11 @@ void main() {
parent.buildScene(SceneBuilder()); parent.buildScene(SceneBuilder());
}, skip: isBrowser); // TODO(yjbanov): `toImage` doesn't work on the Web: https://github.com/flutter/flutter/issues/42767 }, skip: isBrowser); // TODO(yjbanov): `toImage` doesn't work on the Web: https://github.com/flutter/flutter/issues/42767
// TODO(dnfield): remove this when https://github.com/flutter/flutter/issues/85066 is resolved.
const bool bug85066 = true;
test('PictureLayer does not let you call dispose unless refcount is 0', () { test('PictureLayer does not let you call dispose unless refcount is 0', () {
PictureLayer layer = PictureLayer(Rect.zero); PictureLayer layer = PictureLayer(Rect.zero);
expect(layer.debugHandleCount, 0); expect(layer.debugHandleCount, 0);
layer.dispose(); layer.dispose();
expect(layer.debugDisposed, true, skip: bug85066); expect(layer.debugDisposed, true);
layer = PictureLayer(Rect.zero); layer = PictureLayer(Rect.zero);
final LayerHandle<PictureLayer> handle = LayerHandle<PictureLayer>(layer); final LayerHandle<PictureLayer> handle = LayerHandle<PictureLayer>(layer);
...@@ -616,8 +614,8 @@ void main() { ...@@ -616,8 +614,8 @@ void main() {
expect(() => layer.dispose(), throwsAssertionError); expect(() => layer.dispose(), throwsAssertionError);
handle.layer = null; handle.layer = null;
expect(layer.debugHandleCount, 0); expect(layer.debugHandleCount, 0);
expect(layer.debugDisposed, true, skip: bug85066); expect(layer.debugDisposed, true);
expect(() => layer.dispose(), throwsAssertionError, skip: bug85066); // already disposed. expect(() => layer.dispose(), throwsAssertionError); // already disposed.
}); });
test('Layer append/remove increases/decreases handle count', () { test('Layer append/remove increases/decreases handle count', () {
...@@ -632,7 +630,7 @@ void main() { ...@@ -632,7 +630,7 @@ void main() {
layer.remove(); layer.remove();
expect(layer.debugHandleCount, 0); expect(layer.debugHandleCount, 0);
expect(layer.debugDisposed, true, skip: bug85066); expect(layer.debugDisposed, true);
}); });
test('Layer.dispose disposes the engineLayer', () { test('Layer.dispose disposes the engineLayer', () {
...@@ -693,17 +691,17 @@ void main() { ...@@ -693,17 +691,17 @@ void main() {
holder.layer = layer2; holder.layer = layer2;
expect(layer.debugHandleCount, 0); expect(layer.debugHandleCount, 0);
expect(layer.debugDisposed, true, skip: bug85066); expect(layer.debugDisposed, true);
expect(layer2.debugHandleCount, 1); expect(layer2.debugHandleCount, 1);
expect(layer2.debugDisposed, false); expect(layer2.debugDisposed, false);
holder.layer = null; holder.layer = null;
expect(layer.debugHandleCount, 0); expect(layer.debugHandleCount, 0);
expect(layer.debugDisposed, true, skip: bug85066); expect(layer.debugDisposed, true);
expect(layer2.debugHandleCount, 0); expect(layer2.debugHandleCount, 0);
expect(layer2.debugDisposed, true, skip: bug85066); expect(layer2.debugDisposed, true);
expect(() => holder.layer = layer, throwsAssertionError, skip: bug85066); expect(() => holder.layer = layer, throwsAssertionError);
}); });
} }
......
...@@ -7,8 +7,6 @@ import 'package:flutter/widgets.dart'; ...@@ -7,8 +7,6 @@ import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
void main() { void main() {
// TODO(dnfield): remove when https://github.com/flutter/flutter/issues/85066 is resolved.
const bool bug85066 = true;
testWidgets('Tracks picture layers accurately when painting is interleaved with a pushLayer', (WidgetTester tester) async { testWidgets('Tracks picture layers accurately when painting is interleaved with a pushLayer', (WidgetTester tester) async {
// Creates a RenderObject that will paint into multiple picture layers. // Creates a RenderObject that will paint into multiple picture layers.
// Asserts that both layers get a handle, and that all layers get correctly // Asserts that both layers get a handle, and that all layers get correctly
...@@ -34,7 +32,7 @@ void main() { ...@@ -34,7 +32,7 @@ void main() {
await tester.pumpWidget(const SizedBox()); await tester.pumpWidget(const SizedBox());
for (final Layer layer in layers) { for (final Layer layer in layers) {
expect(layer.debugDisposed, true, skip: bug85066); expect(layer.debugDisposed, true);
} }
expect(renderObject.debugDisposed, true); expect(renderObject.debugDisposed, true);
}); });
......
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