diff --git a/engine/src/flutter/lib/ui/dart_ui.cc b/engine/src/flutter/lib/ui/dart_ui.cc index c669b920576..c8c85972a74 100644 --- a/engine/src/flutter/lib/ui/dart_ui.cc +++ b/engine/src/flutter/lib/ui/dart_ui.cc @@ -197,6 +197,7 @@ typedef CanvasPath Path; V(Paragraph, alphabeticBaseline, 1) \ V(Paragraph, computeLineMetrics, 1) \ V(Paragraph, didExceedMaxLines, 1) \ + V(Paragraph, dispose, 1) \ V(Paragraph, getLineBoundary, 2) \ V(Paragraph, getPositionForOffset, 3) \ V(Paragraph, getRectsForPlaceholders, 1) \ @@ -280,7 +281,8 @@ typedef CanvasPath Path; V(SemanticsUpdateBuilder, build, 2) \ V(SemanticsUpdateBuilder, updateCustomAction, 5) \ V(SemanticsUpdateBuilder, updateNode, 36) \ - V(SemanticsUpdate, dispose, 1) + V(SemanticsUpdate, dispose, 1) \ + V(Vertices, dispose, 1) #define FFI_FUNCTION_INSERT(FUNCTION, ARGS) \ g_function_dispatchers.insert(std::make_pair( \ diff --git a/engine/src/flutter/lib/ui/painting.dart b/engine/src/flutter/lib/ui/painting.dart index aee51120369..68b634a9df6 100644 --- a/engine/src/flutter/lib/ui/painting.dart +++ b/engine/src/flutter/lib/ui/painting.dart @@ -4417,6 +4417,36 @@ class Vertices extends NativeFieldWrapperClass1 { Float32List? textureCoordinates, Int32List? colors, Uint16List? indices); + + /// Release the resources used by this object. The object is no longer usable + /// after this method is called. + void dispose() { + assert(!_disposed); + assert(() { + _disposed = true; + return true; + }()); + _dispose(); + } + + /// This can't be a leaf call because the native function calls Dart API + /// (Dart_SetNativeInstanceField). + @FfiNative)>('Vertices::dispose') + external void _dispose(); + + bool _disposed = false; + /// Whether this reference to the underlying picture is [dispose]d. + /// + /// This only returns a valid value if asserts are enabled, and must not be + /// used otherwise. + bool get debugDisposed { + bool? disposed; + assert(() { + disposed = _disposed; + return true; + }()); + return disposed ?? (throw StateError('$runtimeType.debugDisposed is only available when asserts are enabled.')); + } } /// Defines how a list of points is interpreted when drawing a set of points. @@ -5009,6 +5039,7 @@ class Canvas extends NativeFieldWrapperClass1 { /// given [Offset]. The image is composited into the canvas using the given [Paint]. void drawImage(Image image, Offset offset, Paint paint) { assert(image != null); // image is checked on the engine side + assert(!image.debugDisposed); assert(_offsetIsValid(offset)); assert(paint != null); final String? error = _drawImage(image._image, offset.dx, offset.dy, paint._objects, paint._data, paint.filterQuality.index); @@ -5031,6 +5062,7 @@ class Canvas extends NativeFieldWrapperClass1 { /// performance. void drawImageRect(Image image, Rect src, Rect dst, Paint paint) { assert(image != null); // image is checked on the engine side + assert(!image.debugDisposed); assert(_rectIsValid(src)); assert(_rectIsValid(dst)); assert(paint != null); @@ -5081,6 +5113,7 @@ class Canvas extends NativeFieldWrapperClass1 { /// positions. void drawImageNine(Image image, Rect center, Rect dst, Paint paint) { assert(image != null); // image is checked on the engine side + assert(!image.debugDisposed); assert(_rectIsValid(center)); assert(_rectIsValid(dst)); assert(paint != null); @@ -5120,6 +5153,7 @@ class Canvas extends NativeFieldWrapperClass1 { /// [PictureRecorder]. void drawPicture(Picture picture) { assert(picture != null); // picture is checked on the engine side + assert(!picture.debugDisposed); _drawPicture(picture); } @@ -5148,6 +5182,7 @@ class Canvas extends NativeFieldWrapperClass1 { /// [Paragraph.layout], to the `offset` argument's [Offset.dx] coordinate. void drawParagraph(Paragraph paragraph, Offset offset) { assert(paragraph != null); + assert(!paragraph.debugDisposed); assert(_offsetIsValid(offset)); assert(!paragraph._needsLayout); paragraph._paint(this, offset.dx, offset.dy); @@ -5211,8 +5246,8 @@ class Canvas extends NativeFieldWrapperClass1 { /// rather than unencoded lists. /// * [paint], Image shaders can be used to draw images on a triangular mesh. void drawVertices(Vertices vertices, BlendMode blendMode, Paint paint) { - assert(vertices != null); // vertices is checked on the engine side + assert(!vertices.debugDisposed); assert(paint != null); assert(blendMode != null); _drawVertices(vertices, blendMode.index, paint._objects, paint._data); @@ -5360,6 +5395,7 @@ class Canvas extends NativeFieldWrapperClass1 { Rect? cullRect, Paint paint) { assert(atlas != null); // atlas is checked on the engine side + assert(!atlas.debugDisposed); assert(transforms != null); assert(rects != null); assert(colors == null || colors.isEmpty || blendMode != null); diff --git a/engine/src/flutter/lib/ui/painting/vertices.cc b/engine/src/flutter/lib/ui/painting/vertices.cc index e7680d115bf..78d8c825a07 100644 --- a/engine/src/flutter/lib/ui/painting/vertices.cc +++ b/engine/src/flutter/lib/ui/painting/vertices.cc @@ -81,7 +81,15 @@ bool Vertices::init(Dart_Handle vertices_handle, } size_t Vertices::GetAllocationSize() const { + if (!vertices_) { + return sizeof(*this); + } return vertices_->size(); } +void Vertices::dispose() { + vertices_.reset(); + ClearDartWrapper(); +} + } // namespace flutter diff --git a/engine/src/flutter/lib/ui/painting/vertices.h b/engine/src/flutter/lib/ui/painting/vertices.h index 717562c80a9..f5a210fe746 100644 --- a/engine/src/flutter/lib/ui/painting/vertices.h +++ b/engine/src/flutter/lib/ui/painting/vertices.h @@ -30,6 +30,8 @@ class Vertices : public RefCountedDartWrappable { size_t GetAllocationSize() const override; + void dispose(); + private: Vertices(); diff --git a/engine/src/flutter/lib/ui/semantics.dart b/engine/src/flutter/lib/ui/semantics.dart index b58a70b8342..ff168c8ae88 100644 --- a/engine/src/flutter/lib/ui/semantics.dart +++ b/engine/src/flutter/lib/ui/semantics.dart @@ -1017,6 +1017,8 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass1 { /// /// The returned object can be passed to [PlatformDispatcher.updateSemantics] /// to actually update the semantics retained by the system. + /// + /// This object is unusable after calling build. SemanticsUpdate build() { final SemanticsUpdate semanticsUpdate = SemanticsUpdate._(); _build(semanticsUpdate); diff --git a/engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc b/engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc index 90e35df3335..d91c4ad57fe 100644 --- a/engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc +++ b/engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc @@ -131,6 +131,7 @@ void SemanticsUpdateBuilder::updateCustomAction(int id, void SemanticsUpdateBuilder::build(Dart_Handle semantics_update_handle) { SemanticsUpdate::create(semantics_update_handle, std::move(nodes_), std::move(actions_)); + ClearDartWrapper(); } } // namespace flutter diff --git a/engine/src/flutter/lib/ui/text.dart b/engine/src/flutter/lib/ui/text.dart index 3627aa28ac3..e8898feacb6 100644 --- a/engine/src/flutter/lib/ui/text.dart +++ b/engine/src/flutter/lib/ui/text.dart @@ -2907,6 +2907,36 @@ class Paragraph extends NativeFieldWrapperClass1 { @FfiNative)>('Paragraph::computeLineMetrics') external Float64List _computeLineMetrics(); + + /// Release the resources used by this object. The object is no longer usable + /// after this method is called. + void dispose() { + assert(!_disposed); + assert(() { + _disposed = true; + return true; + }()); + _dispose(); + } + + /// This can't be a leaf call because the native function calls Dart API + /// (Dart_SetNativeInstanceField). + @FfiNative)>('Paragraph::dispose') + external void _dispose(); + + bool _disposed = false; + /// Whether this reference to the underlying picture is [dispose]d. + /// + /// This only returns a valid value if asserts are enabled, and must not be + /// used otherwise. + bool get debugDisposed { + bool? disposed; + assert(() { + disposed = _disposed; + return true; + }()); + return disposed ?? (throw StateError('$runtimeType.debugDisposed is only available when asserts are enabled.')); + } } /// Builds a [Paragraph] containing text with the given styling information. @@ -3166,8 +3196,8 @@ class ParagraphBuilder extends NativeFieldWrapperClass1 { _placeholderScales.add(scale); } - @FfiNative, Double, Double, Uint32, Double, Uint32)>('ParagraphBuilder::addPlaceholder') - external String? _addPlaceholder(double width, double height, int alignment, double baselineOffset, int baseline); + @FfiNative, Double, Double, Uint32, Double, Uint32)>('ParagraphBuilder::addPlaceholder') + external void _addPlaceholder(double width, double height, int alignment, double baselineOffset, int baseline); /// Applies the given paragraph style and returns a [Paragraph] containing the /// added text and associated styling. diff --git a/engine/src/flutter/lib/ui/text/paragraph.cc b/engine/src/flutter/lib/ui/text/paragraph.cc index f38f4196cc9..58b5230f59a 100644 --- a/engine/src/flutter/lib/ui/text/paragraph.cc +++ b/engine/src/flutter/lib/ui/text/paragraph.cc @@ -66,6 +66,11 @@ void Paragraph::layout(double width) { } void Paragraph::paint(Canvas* canvas, double x, double y) { + if (!m_paragraph || !canvas) { + // disposed. + return; + } + SkCanvas* sk_canvas = canvas->canvas(); if (!sk_canvas) { return; @@ -168,4 +173,9 @@ tonic::Float64List Paragraph::computeLineMetrics() { return result; } +void Paragraph::dispose() { + m_paragraph.reset(); + ClearDartWrapper(); +} + } // namespace flutter diff --git a/engine/src/flutter/lib/ui/text/paragraph.h b/engine/src/flutter/lib/ui/text/paragraph.h index 66aaa75ddae..3f24d1f03e2 100644 --- a/engine/src/flutter/lib/ui/text/paragraph.h +++ b/engine/src/flutter/lib/ui/text/paragraph.h @@ -51,6 +51,8 @@ class Paragraph : public RefCountedDartWrappable { size_t GetAllocationSize() const override; + void dispose(); + private: std::unique_ptr m_paragraph; diff --git a/engine/src/flutter/lib/ui/text/paragraph_builder.cc b/engine/src/flutter/lib/ui/text/paragraph_builder.cc index bd71ca90158..990c2957e95 100644 --- a/engine/src/flutter/lib/ui/text/paragraph_builder.cc +++ b/engine/src/flutter/lib/ui/text/paragraph_builder.cc @@ -528,22 +528,22 @@ Dart_Handle ParagraphBuilder::addText(const std::u16string& text) { return Dart_Null(); } -Dart_Handle ParagraphBuilder::addPlaceholder(double width, - double height, - unsigned alignment, - double baseline_offset, - unsigned baseline) { +void ParagraphBuilder::addPlaceholder(double width, + double height, + unsigned alignment, + double baseline_offset, + unsigned baseline) { txt::PlaceholderRun placeholder_run( width, height, static_cast(alignment), static_cast(baseline), baseline_offset); m_paragraphBuilder->AddPlaceholder(placeholder_run); - - return Dart_Null(); } void ParagraphBuilder::build(Dart_Handle paragraph_handle) { Paragraph::Create(paragraph_handle, m_paragraphBuilder->Build()); + m_paragraphBuilder.reset(); + ClearDartWrapper(); } } // namespace flutter diff --git a/engine/src/flutter/lib/ui/text/paragraph_builder.h b/engine/src/flutter/lib/ui/text/paragraph_builder.h index 19fd5f435d7..0018f969da6 100644 --- a/engine/src/flutter/lib/ui/text/paragraph_builder.h +++ b/engine/src/flutter/lib/ui/text/paragraph_builder.h @@ -60,11 +60,11 @@ class ParagraphBuilder : public RefCountedDartWrappable { // Internally, this method adds a single object replacement character (0xFFFC) // and emplaces a new PlaceholderRun instance to the vector of inline // placeholders. - Dart_Handle addPlaceholder(double width, - double height, - unsigned alignment, - double baseline_offset, - unsigned baseline); + void addPlaceholder(double width, + double height, + unsigned alignment, + double baseline_offset, + unsigned baseline); void build(Dart_Handle paragraph_handle); diff --git a/engine/src/flutter/lib/web_ui/lib/canvas.dart b/engine/src/flutter/lib/web_ui/lib/canvas.dart index 1bcb8173c59..ac90121158c 100644 --- a/engine/src/flutter/lib/web_ui/lib/canvas.dart +++ b/engine/src/flutter/lib/web_ui/lib/canvas.dart @@ -21,7 +21,7 @@ enum VertexMode { triangleFan, } -class Vertices { +abstract class Vertices { factory Vertices( VertexMode mode, List positions, { @@ -48,6 +48,9 @@ class Vertices { colors: colors, indices: indices); } + + void dispose(); + bool get debugDisposed; } abstract class PictureRecorder { diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart index 01ee84fc7bd..433bcf9039a 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart @@ -797,6 +797,23 @@ class CkParagraph extends SkiaObject implements ui.Paragraph { } return result; } + + bool _disposed = false; + + @override + void dispose() { + delete(); + didDelete(); + _disposed = true; + } + + @override + bool get debugDisposed { + if (assertionsEnabled) { + return _disposed; + } + throw StateError('Paragraph.debugDisposed is only available when asserts are enabled.'); + } } class CkLineMetrics implements ui.LineMetrics { diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/vertices.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/vertices.dart index 22cfbebda12..4d34d34dfd2 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/vertices.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/vertices.dart @@ -6,6 +6,7 @@ import 'dart:typed_data'; import 'package:ui/ui.dart' as ui; +import '../util.dart'; import 'canvaskit_api.dart'; import 'skia_object_cache.dart'; @@ -108,4 +109,20 @@ class CkVertices extends ManagedSkiaObject implements ui.Vertices { void delete() { rawSkiaObject?.delete(); } + + bool _disposed = false; + + @override + void dispose() { + delete(); + _disposed = true; + } + + @override + bool get debugDisposed { + if (assertionsEnabled) { + return _disposed; + } + throw StateError('Vertices.debugDisposed is only avialalbe when asserts are enabled.'); + } } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/html/render_vertices.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/html/render_vertices.dart index 4c8305b2660..7ab470c4dba 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/html/render_vertices.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/html/render_vertices.dart @@ -57,6 +57,21 @@ class SurfaceVertices implements ui.Vertices { } return list; } + + bool _disposed = false; + + @override + void dispose() { + _disposed = true; + } + + @override + bool get debugDisposed { + if (assertionsEnabled) { + return _disposed; + } + throw StateError('Vertices.debugDisposed is only avialalbe when asserts are enabled.'); + } } /// Lazily initializes web gl. diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart index 4b35410321e..12971439f83 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart @@ -8,6 +8,7 @@ import '../dom.dart'; import '../embedder.dart'; import '../html/bitmap_canvas.dart'; import '../profiler.dart'; +import '../util.dart'; import 'layout_service.dart'; import 'paint_service.dart'; import 'paragraph.dart'; @@ -40,7 +41,7 @@ class CanvasParagraph implements ui.Paragraph { final EngineParagraphStyle paragraphStyle; /// The full textual content of the paragraph. - final String plainText; + late String plainText; /// The number of placeholders in this paragraph. final int placeholderCount; @@ -254,6 +255,23 @@ class CanvasParagraph implements ui.Paragraph { List computeLineMetrics() { return lines.map((ParagraphLine line) => line.lineMetrics).toList(); } + + bool _disposed = false; + + @override + void dispose() { + spans.clear(); + plainText = ''; + _disposed = true; + } + + @override + bool get debugDisposed { + if (assertionsEnabled) { + return _disposed; + } + throw StateError('Vertices.debugDisposed is only avialalbe when asserts are enabled.'); + } } void _positionSpanElement(DomElement element, ParagraphLine line, RangeBox box) { diff --git a/engine/src/flutter/lib/web_ui/lib/text.dart b/engine/src/flutter/lib/web_ui/lib/text.dart index 1712155ad91..7c7e3c6c100 100644 --- a/engine/src/flutter/lib/web_ui/lib/text.dart +++ b/engine/src/flutter/lib/web_ui/lib/text.dart @@ -670,6 +670,8 @@ abstract class Paragraph { TextRange getLineBoundary(TextPosition position); List getBoxesForPlaceholders(); List computeLineMetrics(); + void dispose(); + bool get debugDisposed; } abstract class ParagraphBuilder { diff --git a/engine/src/flutter/testing/dart/canvas_test.dart b/engine/src/flutter/testing/dart/canvas_test.dart index e9dc8b1337d..76c9c30dae2 100644 --- a/engine/src/flutter/testing/dart/canvas_test.dart +++ b/engine/src/flutter/testing/dart/canvas_test.dart @@ -109,6 +109,7 @@ void testNoCrashes() { testCanvas((Canvas canvas) => canvas.translate(double.nan, double.nan)); testCanvas((Canvas canvas) => canvas.drawVertices(Vertices(VertexMode.triangles, [], indices: []), BlendMode.screen, paint)); + testCanvas((Canvas canvas) => canvas.drawVertices(Vertices(VertexMode.triangles, [])..dispose(), BlendMode.screen, paint)); }); } diff --git a/engine/src/flutter/testing/dart/paragraph_builder_test.dart b/engine/src/flutter/testing/dart/paragraph_builder_test.dart index d7a6ebc8750..37e27a75814 100644 --- a/engine/src/flutter/testing/dart/paragraph_builder_test.dart +++ b/engine/src/flutter/testing/dart/paragraph_builder_test.dart @@ -25,7 +25,7 @@ void main() { final ParagraphBuilder paragraphBuilder = ParagraphBuilder(ParagraphStyle()); paragraphBuilder.build(); - paragraphBuilder.pushStyle(TextStyle()); + expect(() { paragraphBuilder.pushStyle(TextStyle()); }, throwsStateError); }); test('GetRectsForRange smoke test', () { diff --git a/engine/src/flutter/testing/dart/paragraph_test.dart b/engine/src/flutter/testing/dart/paragraph_test.dart index 9038ce11230..b6f4898669c 100644 --- a/engine/src/flutter/testing/dart/paragraph_test.dart +++ b/engine/src/flutter/testing/dart/paragraph_test.dart @@ -197,4 +197,19 @@ void main() { expect(line.start, 6); expect(line.end, 10); }); + + test('painting a disposed paragraph does not crash', () { + final Paragraph paragraph = ParagraphBuilder(ParagraphStyle()).build(); + paragraph.dispose(); + + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + + void callback() { canvas.drawParagraph(paragraph, Offset.zero); } + if (assertStatementsEnabled) { + expectAssertion(callback); + } else { + expect(callback, throwsStateError); + } + }); }