diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 99c865f4d9fc1..12e042c34c56e 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1876,7 +1876,6 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_tree.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/noto_font.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart + ../../../flutter/LICENSE @@ -4461,7 +4460,6 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.d FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_tree.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart -FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/noto_font.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart diff --git a/lib/web_ui/lib/src/engine.dart b/lib/web_ui/lib/src/engine.dart index 07e54ca03c127..a82b9d9b82a0e 100644 --- a/lib/web_ui/lib/src/engine.dart +++ b/lib/web_ui/lib/src/engine.dart @@ -39,7 +39,6 @@ export 'engine/canvaskit/layer_scene_builder.dart'; export 'engine/canvaskit/layer_tree.dart'; export 'engine/canvaskit/mask_filter.dart'; export 'engine/canvaskit/n_way_canvas.dart'; -export 'engine/canvaskit/native_memory.dart'; export 'engine/canvaskit/noto_font.dart'; export 'engine/canvaskit/painting.dart'; export 'engine/canvaskit/path.dart'; diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvas.dart b/lib/web_ui/lib/src/engine/canvaskit/canvas.dart index 9a5dd90426cc7..3d35df90b4c41 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvas.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvas.dart @@ -215,6 +215,7 @@ class CkCanvas { offset.dx, offset.dy, ); + paragraph.markUsed(); } void drawPath(CkPath path, CkPaint paint) { @@ -1111,6 +1112,7 @@ class CkDrawParagraphCommand extends CkPaintCommand { offset.dx, offset.dy, ); + paragraph.markUsed(); } } diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index faacd1dd886b8..884f38f24290e 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -3392,7 +3392,7 @@ abstract class Collector { class ProductionCollector implements Collector { ProductionCollector() { _skObjectFinalizationRegistry = - createSkObjectFinalizationRegistry((SkDeletable deletable) { + SkObjectFinalizationRegistry((SkDeletable deletable) { // This is called when GC decides to collect the wrapper object and // notify us, which may happen after the object is already deleted // explicitly, e.g. when its ref count drops to zero. When that happens @@ -3568,13 +3568,10 @@ extension JsConstructorExtension on JsConstructor { /// 6. We call `delete` on SkPaint. @JS('window.FinalizationRegistry') @staticInterop -class SkObjectFinalizationRegistry {} - -SkObjectFinalizationRegistry createSkObjectFinalizationRegistry(JSFunction cleanup) { - return js_util.callConstructor( - _finalizationRegistryConstructor!.toObjectShallow, - [cleanup], - ); +class SkObjectFinalizationRegistry { + // TODO(hterkelsen): Add a type for the `cleanup` function when + // native constructors support type parameters. + external factory SkObjectFinalizationRegistry(JSFunction cleanup); } extension SkObjectFinalizationRegistryExtension on SkObjectFinalizationRegistry { diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index 3e167422d992a..e43157da53138 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -15,10 +15,10 @@ import 'canvas.dart'; import 'canvaskit_api.dart'; import 'image_wasm_codecs.dart'; import 'image_web_codecs.dart'; -import 'native_memory.dart'; import 'painting.dart'; import 'picture.dart'; import 'picture_recorder.dart'; +import 'skia_object_cache.dart'; /// Instantiates a [ui.Codec] backed by an `SkAnimatedImage` from Skia. FutureOr skiaInstantiateImageCodec(Uint8List list, @@ -227,8 +227,52 @@ Future readChunked(HttpFetchPayload payload, int contentLength, WebOn /// A [ui.Image] backed by an `SkImage` from Skia. class CkImage implements ui.Image, StackTraceDebugger { CkImage(SkImage skImage, { this.videoFrame }) { - box = CountedRef(skImage, this, 'SkImage'); _init(); + if (browserSupportsFinalizationRegistry) { + box = SkiaObjectBox(this, skImage); + } else { + // If finalizers are not supported we need to be able to resurrect the + // image if it was temporarily deleted. To do that, we keep the original + // pixels and ask the SkiaObjectBox to make an image from them when + // resurrecting. + // + // IMPORTANT: the alphaType, colorType, and colorSpace passed to + // _encodeImage and to canvasKit.MakeImage must be the same. Otherwise + // Skia will misinterpret the pixels and corrupt the image. + final ByteData? originalBytes = _encodeImage( + skImage: skImage, + format: ui.ImageByteFormat.rawRgba, + alphaType: canvasKit.AlphaType.Premul, + colorType: canvasKit.ColorType.RGBA_8888, + colorSpace: SkColorSpaceSRGB, + ); + if (originalBytes == null) { + printWarning('Unable to encode image to bytes. We will not ' + 'be able to resurrect it once it has been garbage collected.'); + return; + } + final int originalWidth = skImage.width().toInt(); + final int originalHeight = skImage.height().toInt(); + box = SkiaObjectBox.resurrectable(this, skImage, () { + final SkImage? skImage = canvasKit.MakeImage( + SkImageInfo( + alphaType: canvasKit.AlphaType.Premul, + colorType: canvasKit.ColorType.RGBA_8888, + colorSpace: SkColorSpaceSRGB, + width: originalWidth.toDouble(), + height: originalHeight.toDouble(), + ), + originalBytes.buffer.asUint8List(), + (4 * originalWidth).toDouble(), + ); + if (skImage == null) { + throw ImageCodecException( + 'Failed to resurrect image from pixels.' + ); + } + return skImage; + }); + } } CkImage.cloneOf(this.box, {this.videoFrame}) { @@ -247,9 +291,9 @@ class CkImage implements ui.Image, StackTraceDebugger { StackTrace get debugStackTrace => _debugStackTrace; late StackTrace _debugStackTrace; - // Use ref counting because `SkImage` may be deleted either due to this object + // Use a box because `SkImage` may be deleted either due to this object // being garbage-collected, or by an explicit call to [delete]. - late final CountedRef box; + late final SkiaObjectBox box; /// For browsers that support `ImageDecoder` this field holds the video frame /// from which this image was created. @@ -261,9 +305,9 @@ class CkImage implements ui.Image, StackTraceDebugger { /// The underlying Skia image object. /// - /// Do not store the returned value. It is memory-managed by [CountedRef]. + /// Do not store the returned value. It is memory-managed by [SkiaObjectBox]. /// Storing it may result in use-after-free bugs. - SkImage get skImage => box.nativeObject; + SkImage get skImage => box.skiaObject; bool _disposed = false; diff --git a/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart b/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart deleted file mode 100644 index 9d9ed733698ca..0000000000000 --- a/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart +++ /dev/null @@ -1,216 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'dart:js_interop'; -import 'package:meta/meta.dart'; - -import '../../engine.dart' show Instrumentation; -import '../util.dart'; -import 'canvaskit_api.dart'; - -/// Collects native objects that weren't explicitly disposed of using -/// [UniqueRef.dispose] or [CountedRef.unref]. -SkObjectFinalizationRegistry _finalizationRegistry = createSkObjectFinalizationRegistry( - (UniqueRef uniq) { - uniq.collect(); - }.toJS -); - -NativeMemoryFinalizationRegistry nativeMemoryFinalizationRegistry = NativeMemoryFinalizationRegistry(); - -/// An indirection to [SkObjectFinalizationRegistry] to enable tests provide a -/// mock implementation of a finalization registry. -class NativeMemoryFinalizationRegistry { - void register(Object owner, UniqueRef ref) { - _finalizationRegistry.register(owner, ref); - } -} - -/// Manages the lifecycle of a C++ object referenced by a single Dart object. -/// -/// It is expected that when the C++ object is no longer needed [dispose] is -/// called. -/// -/// To prevent memory leaks, the underlying C++ object is deleted by the GC if -/// it wasn't previously disposed of explicitly. -class UniqueRef { - UniqueRef(Object owner, T nativeObject, this._debugOwnerLabel) { - _nativeObject = nativeObject; - if (Instrumentation.enabled) { - Instrumentation.instance.incrementCounter('$_debugOwnerLabel Created'); - } - nativeMemoryFinalizationRegistry.register(owner, this); - } - - T? _nativeObject; - final String _debugOwnerLabel; - - /// Returns the underlying native object reference, if it has not been - /// disposed of yet. - /// - /// The returned reference must not be stored. I should only be borrowed - /// temporarily. Storing this reference may result in dangling pointer errors. - T get nativeObject { - assert(!isDisposed, 'Native object was disposed.'); - return _nativeObject!; - } - - /// Returns whether the underlying native object has been disposed and - /// therefore can no longer be used. - bool get isDisposed => _nativeObject == null; - - /// Disposes the underlying native object. - /// - /// The underlying object may be deleted or its ref count may be bumped down. - /// The exact action taken depends on the sharing model of that particular - /// object. For example, an [SkImage] may not be immediately deleted if a - /// [SkPicture] exists that still references it. On the other hand, [SkPaint] - /// is deleted eagerly. - void dispose() { - assert(!isDisposed, 'A native object reference cannot be disposed more than once.'); - if (Instrumentation.enabled) { - Instrumentation.instance.incrementCounter('$_debugOwnerLabel Deleted'); - } - final SkDeletable object = nativeObject as SkDeletable; - if (!object.isDeleted()) { - object.delete(); - } - _nativeObject = null; - } - - /// Called by the garbage [Collector] when the owner of this handle is - /// collected. - /// - /// Garbage collection is used as a back-up for the cases when the handle - /// isn't disposed of explicitly by calling [dispose]. It most likely - /// indicates a memory leak or inefficiency in the framework or application - /// code. - @visibleForTesting - void collect() { - if (!isDisposed) { - if (Instrumentation.enabled) { - Instrumentation.instance.incrementCounter('$_debugOwnerLabel Leaked'); - } - dispose(); - } - } -} - -/// Interface that classes wrapping [UniqueRef] must implement. -/// -/// Used to collect stack traces in debug mode. -abstract class StackTraceDebugger { - /// The stack trace pointing to code location that created or upreffed a - /// [CountedRef]. - StackTrace get debugStackTrace; -} - -/// Manages the lifecycle of a C++ object referenced by multiple Dart objects. -/// -/// Uses reference counting to manage the lifecycle of the C++ object. -/// -/// If the C++ object has a unique owner, use [UniqueRef] instead. -/// -/// The [ref] method can be used to increment the refcount to tell this box to -/// keep the underlying C++ object alive. -/// -/// The [unref] method can be used to decrement the refcount indicating that a -/// referring object no longer needs it. When the refcount drops to zero the -/// underlying C++ object is deleted. -/// -/// In addition to ref counting, this object is also managed by GC. When this -/// reference is garbage collected, the underlying C++ object is automatically -/// deleted. This is mostly done to prevent memory leaks in production. Well -/// behaving framework and app code are expected to rely on [ref] and [unref] -/// for timely collection of resources. -class CountedRef { - /// Creates a counted reference. - CountedRef(T nativeObject, R debugReferrer, String debugLabel) { - _ref = UniqueRef(this, nativeObject, debugLabel); - if (assertionsEnabled) { - debugReferrers.add(debugReferrer); - } - assert(refCount == debugReferrers.length); - } - - /// The native object reference whose lifecycle is being managed by this ref - /// count. - /// - /// Do not store this value outside this class. - late final UniqueRef _ref; - - /// Returns the underlying native object reference, if it has not been - /// disposed of yet. - /// - /// The returned reference must not be stored. I should only be borrowed - /// temporarily. Storing this reference may result in dangling pointer errors. - T get nativeObject => _ref.nativeObject; - - /// The number of objects sharing references to this box. - /// - /// When this count reaches zero, the underlying [nativeObject] is scheduled - /// for deletion. - int get refCount => _refCount; - int _refCount = 1; - - /// Whether the underlying [nativeObject] has been disposed and is no longer - /// accessible. - bool get isDisposed => _ref.isDisposed; - - /// When assertions are enabled, stores all objects that share this box. - /// - /// The length of this list is always identical to [refCount]. - /// - /// This list can be used for debugging ref counting issues. - final Set debugReferrers = {}; - - /// If asserts are enabled, the [StackTrace]s representing when a reference - /// was created. - List debugGetStackTraces() { - if (assertionsEnabled) { - return debugReferrers - .map((R referrer) => referrer.debugStackTrace) - .toList(); - } - throw UnsupportedError(''); - } - - /// Increases the reference count of this box because a new object began - /// sharing ownership of the underlying [nativeObject]. - void ref(R debugReferrer) { - assert( - !_ref.isDisposed, - 'Cannot increment ref count on a deleted handle.', - ); - assert(_refCount > 0); - assert( - debugReferrers.add(debugReferrer), - 'Attempted to increment ref count by the same referrer more than once.', - ); - _refCount += 1; - assert(refCount == debugReferrers.length); - } - - /// Decrements the reference count for the [nativeObject]. - /// - /// Does nothing if the object has already been deleted. - /// - /// If this causes the reference count to drop to zero, deletes the - /// [nativeObject]. - void unref(R debugReferrer) { - assert( - !_ref.isDisposed, - 'Attempted to unref an already deleted native object.', - ); - assert( - debugReferrers.remove(debugReferrer), - 'Attempted to decrement ref count by the same referrer more than once.', - ); - _refCount -= 1; - assert(refCount == debugReferrers.length); - if (_refCount == 0) { - _ref.dispose(); - } - } -} diff --git a/lib/web_ui/lib/src/engine/canvaskit/picture.dart b/lib/web_ui/lib/src/engine/canvaskit/picture.dart index 936d5897ba9ec..28511bbba7bc3 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/picture.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/picture.dart @@ -46,7 +46,8 @@ class CkPicture extends ManagedSkiaObject implements ui.Picture { /// false. /// /// This extra flag is necessary on top of [rawSkiaObject] because - /// [rawSkiaObject] being null does not indicate permanent deletion. + /// [rawSkiaObject] being null does not indicate permanent deletion. See + /// similar flag [SkiaObjectBox.isDeletedPermanently]. bool _isDisposed = false; /// The stack trace taken when [dispose] was called. diff --git a/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart b/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart index 677397b953082..21debea93cfb9 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart @@ -7,6 +7,7 @@ import 'dart:collection'; import 'package:meta/meta.dart'; import '../../engine.dart' show Instrumentation; +import '../util.dart'; import 'canvaskit_api.dart'; import 'renderer.dart'; @@ -87,6 +88,85 @@ class SkiaObjectCache { } } +/// Like [SkiaObjectCache] but enforces the [maximumSize] of the cache +/// synchronously instead of waiting until a post-frame callback. +class SynchronousSkiaObjectCache { + SynchronousSkiaObjectCache(this.maximumSize) + : _itemQueue = DoubleLinkedQueue>(), + _itemMap = , DoubleLinkedQueueEntry>>{}; + + /// This cache will never exceed this limit, even temporarily. + final int maximumSize; + + /// A doubly linked list of the objects in the cache. + /// + /// This makes it fast to move a recently used object to the front. + final DoubleLinkedQueue> _itemQueue; + + /// A map of objects to their associated node in the [_itemQueue]. + /// + /// This makes it fast to find the node in the queue when we need to + /// move the object to the front of the queue. + final Map, DoubleLinkedQueueEntry>> _itemMap; + + /// The number of objects in the cache. + int get length => _itemQueue.length; + + /// Whether or not [object] is in the cache. + /// + /// This is only for testing. + @visibleForTesting + bool debugContains(SkiaObject object) { + return _itemMap.containsKey(object); + } + + /// Adds [object] to the cache. + /// + /// If adding [object] causes the total size of the cache to exceed + /// [maximumSize], then the least recently used objects are evicted and + /// deleted. + void add(SkiaObject object) { + assert( + !_itemMap.containsKey(object), + 'Cannot add object. Object is already in the cache: $object', + ); + _itemQueue.addFirst(object); + _itemMap[object] = _itemQueue.firstEntry()!; + _enforceCacheLimit(); + } + + /// Marks the [object] as most recently used. + /// + /// If [object] is in the cache returns true. If the object is not in + /// the cache, for example, because it was never added or because it was + /// evicted as a result of the app reaching the cache limit, returns false. + bool markUsed(SkiaObject object) { + final DoubleLinkedQueueEntry>? item = _itemMap[object]; + + if (item == null) { + return false; + } + + item.remove(); + _itemQueue.addFirst(object); + _itemMap[object] = _itemQueue.firstEntry()!; + return true; + } + + /// Ensures the cache does not exceed [maximumSize], evicting objects if + /// necessary. + /// + /// Calls `delete` and `didDelete` on objects evicted from the cache. + void _enforceCacheLimit() { + while (_itemQueue.length > maximumSize) { + final SkiaObject oldObject = _itemQueue.removeLast(); + _itemMap.remove(oldObject); + oldObject.delete(); + oldObject.didDelete(); + } + } +} + /// An object backed by a JavaScript object mapped onto a Skia C++ object in the /// WebAssembly heap. /// @@ -231,9 +311,205 @@ abstract class ManagedSkiaObject extends SkiaObject { bool get isResurrectionExpensive => false; } +/// Interface that classes wrapping [SkiaObjectBox] must implement. +/// +/// Used to collect stack traces in debug mode. +abstract class StackTraceDebugger { + /// The stack trace pointing to code location that created or upreffed a + /// [SkiaObjectBox]. + StackTrace get debugStackTrace; +} + /// A function that restores a Skia object that was temporarily deleted. typedef Resurrector = T Function(); +/// Uses reference counting to manage the lifecycle of a Skia object owned by a +/// wrapper object. +/// +/// The [ref] method can be used to increment the refcount to tell this box to +/// keep the underlying Skia object alive. +/// +/// The [unref] method can be used to decrement the refcount to tell this box +/// that a wrapper object no longer needs it. When the refcount drops to zero +/// the underlying Skia object is deleted permanently (see [isDeletedPermanently]). +/// +/// In addition to ref counting, this object is also managed by GC. In browsers +/// that support [SkFinalizationRegistry] the underlying Skia object is deleted +/// permanently when no JavaScript objects have references to this box. In +/// browsers that do not support [SkFinalizationRegistry] the underlying Skia +/// object may undergo several cycles of temporary deletions and resurrections +/// prior to being deleted permanently. A temporary deletion may effectively +/// be permanent if this object is garbage collected. This is safe because a +/// temporarily deleted object has no C++ resources to collect. +class SkiaObjectBox + extends SkiaObject { + /// Creates an object box that's memory-managed using [SkFinalizationRegistry]. + /// + /// This constructor must only be used if [browserSupportsFinalizationRegistry] is true. + SkiaObjectBox(R debugReferrer, T initialValue) : + assert(browserSupportsFinalizationRegistry), _resurrector = null { + _initialize(debugReferrer, initialValue); + Collector.instance.register(this, _skDeletable!); + } + + /// Creates an object box that's memory-managed using a [Resurrector]. + /// + /// This constructor must only be used if [browserSupportsFinalizationRegistry] is false. + SkiaObjectBox.resurrectable( + R debugReferrer, T initialValue, this._resurrector) : + assert(!browserSupportsFinalizationRegistry) { + _initialize(debugReferrer, initialValue); + if (Instrumentation.enabled) { + Instrumentation.instance.incrementCounter( + '${_skDeletable?.constructor.name} created', + ); + } + SkiaObjects.manageExpensive(this); + } + + void _initialize(R debugReferrer, T initialValue) { + _update(initialValue); + if (assertionsEnabled) { + debugReferrers.add(debugReferrer); + } + assert(refCount == debugReferrers.length); + } + + /// The number of objects sharing references to this box. + /// + /// When this count reaches zero, the underlying [skiaObject] is scheduled + /// for deletion. + int get refCount => _refCount; + int _refCount = 1; + + /// When assertions are enabled, stores all objects that share this box. + /// + /// The length of this list is always identical to [refCount]. + /// + /// This list can be used for debugging ref counting issues. + final Set debugReferrers = {}; + + /// If asserts are enabled, the [StackTrace]s representing when a reference + /// was created. + List debugGetStackTraces() { + if (assertionsEnabled) { + return debugReferrers + .map((R referrer) => referrer.debugStackTrace) + .toList(); + } + throw UnsupportedError(''); + } + + /// The Skia object whose lifecycle is being managed. + /// + /// Do not store this value outside this box. It is memory-managed by + /// [SkiaObjectBox]. Storing it may result in use-after-free bugs. + T? rawSkiaObject; + SkDeletable? _skDeletable; + Resurrector? _resurrector; + + void _update(T? newSkiaObject) { + rawSkiaObject = newSkiaObject; + _skDeletable = newSkiaObject as SkDeletable?; + } + + @override + T get skiaObject => rawSkiaObject ?? _doResurrect(); + + T _doResurrect() { + assert(!browserSupportsFinalizationRegistry); + assert(_resurrector != null); + assert(!_isDeletedPermanently, 'Cannot use deleted object.'); + _update(_resurrector!()); + if (Instrumentation.enabled) { + Instrumentation.instance.incrementCounter( + '${_skDeletable?.constructor.name} resurrected', + ); + } + SkiaObjects.manageExpensive(this); + return skiaObject; + } + + @override + void delete() { + _skDeletable?.delete(); + } + + @override + void didDelete() { + if (Instrumentation.enabled) { + Instrumentation.instance.incrementCounter( + '${_skDeletable?.constructor.name} deleted', + ); + } + assert(!browserSupportsFinalizationRegistry); + _update(null); + } + + /// Whether this object has been deleted permanently. + /// + /// If this is true it will remain true forever, and the Skia object is no + /// longer resurrectable. + /// + /// See also [isDeletedTemporarily]. + bool get isDeletedPermanently => _isDeletedPermanently; + bool _isDeletedPermanently = false; + + /// Whether the underlying [rawSkiaObject] has been deleted, but it may still + /// be resurrected (see [SkiaObjectBox.resurrectable]). + bool get isDeletedTemporarily => + rawSkiaObject == null && !_isDeletedPermanently; + + /// Increases the reference count of this box because a new object began + /// sharing ownership of the underlying [skiaObject]. + /// + /// Clones must be [dispose]d when finished. + void ref(R debugReferrer) { + assert(!_isDeletedPermanently, + 'Cannot increment ref count on a deleted handle.'); + assert(_refCount > 0); + assert( + debugReferrers.add(debugReferrer), + 'Attempted to increment ref count by the same referrer more than once.', + ); + _refCount += 1; + assert(refCount == debugReferrers.length); + } + + /// Decrements the reference count for the [skObject]. + /// + /// Does nothing if the object has already been deleted. + /// + /// If this causes the reference count to drop to zero, deletes the + /// [skObject]. + void unref(R debugReferrer) { + assert(!_isDeletedPermanently, + 'Attempted to unref an already deleted Skia object.'); + assert( + debugReferrers.remove(debugReferrer), + 'Attempted to decrement ref count by the same referrer more than once.', + ); + _refCount -= 1; + assert(refCount == debugReferrers.length); + if (_refCount == 0) { + // The object may be null because it was deleted temporarily, i.e. it was + // expecting the possibility of resurrection. + if (_skDeletable != null) { + if (browserSupportsFinalizationRegistry) { + Collector.instance.collect(_skDeletable!); + } else { + delete(); + didDelete(); + } + } + rawSkiaObject = null; + _skDeletable = null; + _resurrector = null; + _isDeletedPermanently = true; + } + } +} + // ignore: avoid_classes_with_only_static_members /// Singleton that manages the lifecycles of [SkiaObject] instances. class SkiaObjects { diff --git a/lib/web_ui/lib/src/engine/canvaskit/text.dart b/lib/web_ui/lib/src/engine/canvaskit/text.dart index ed14a91689099..2b4ab31d53728 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/text.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/text.dart @@ -10,7 +10,6 @@ import 'package:ui/ui.dart' as ui; import '../util.dart'; import 'canvaskit_api.dart'; import 'font_fallbacks.dart'; -import 'native_memory.dart'; import 'painting.dart'; import 'renderer.dart'; import 'skia_object_cache.dart'; @@ -562,27 +561,128 @@ SkFontStyle toSkFontStyle(ui.FontWeight? fontWeight, ui.FontStyle? fontStyle) { /// while painting a small subset of it. To achieve this a /// [SynchronousSkiaObjectCache] is used that limits the number of live laid out /// paragraphs at any point in time within or outside the frame. -class CkParagraph implements ui.Paragraph { - CkParagraph(SkParagraph skParagraph, this._paragraphStyle) { - _ref = UniqueRef(this, skParagraph, 'Paragraph'); - } +class CkParagraph extends SkiaObject implements ui.Paragraph { + CkParagraph(this._skParagraph, this._paragraphStyle, this._paragraphCommands); - late final UniqueRef _ref; + /// The result of calling `build()` on the JS CkParagraphBuilder. + /// + /// This may be invalidated later. + SkParagraph? _skParagraph; - SkParagraph get skiaObject => _ref.nativeObject; + /// The paragraph style used to build this paragraph. + /// + /// This is used to resurrect the paragraph if the initial paragraph + /// is deleted. + final CkParagraphStyle _paragraphStyle; - /// The constraints from the last time we laid the paragraph out. + /// The paragraph builder commands used to build this paragraph. /// /// This is used to resurrect the paragraph if the initial paragraph /// is deleted. - double _lastLayoutConstraints = double.negativeInfinity; + final List<_ParagraphCommand> _paragraphCommands; - /// The paragraph style used to build this paragraph. + /// The constraints from the last time we laid the paragraph out. /// /// This is used to resurrect the paragraph if the initial paragraph /// is deleted. - final CkParagraphStyle _paragraphStyle; + ui.ParagraphConstraints? _lastLayoutConstraints; + @override + SkParagraph get skiaObject => _ensureInitialized(_lastLayoutConstraints!); + + SkParagraph _ensureInitialized(ui.ParagraphConstraints constraints) { + SkParagraph? paragraph = _skParagraph; + + // Paragraph objects are immutable. It's OK to skip initialization and reuse + // existing object. + bool didRebuildSkiaObject = false; + if (paragraph == null) { + final CkParagraphBuilder builder = CkParagraphBuilder(_paragraphStyle); + for (final _ParagraphCommand command in _paragraphCommands) { + switch (command.type) { + case _ParagraphCommandType.addText: + builder.addText(command.text!); + case _ParagraphCommandType.pop: + builder.pop(); + case _ParagraphCommandType.pushStyle: + builder.pushStyle(command.style!); + case _ParagraphCommandType.addPlaceholder: + builder._addPlaceholder(command.placeholderStyle!); + } + } + paragraph = builder._buildSkParagraph(); + _skParagraph = paragraph; + didRebuildSkiaObject = true; + } + + final bool constraintsChanged = _lastLayoutConstraints != constraints; + if (didRebuildSkiaObject || constraintsChanged) { + _lastLayoutConstraints = constraints; + // TODO(het): CanvasKit throws an exception when laid out with + // a font that wasn't registered. + try { + paragraph.layout(constraints.width); + _alphabeticBaseline = paragraph.getAlphabeticBaseline(); + _didExceedMaxLines = paragraph.didExceedMaxLines(); + _height = paragraph.getHeight(); + _ideographicBaseline = paragraph.getIdeographicBaseline(); + _longestLine = paragraph.getLongestLine(); + _maxIntrinsicWidth = paragraph.getMaxIntrinsicWidth(); + _minIntrinsicWidth = paragraph.getMinIntrinsicWidth(); + _width = paragraph.getMaxWidth(); + _boxesForPlaceholders = + skRectsToTextBoxes(paragraph.getRectsForPlaceholders()); + } catch (e) { + printWarning('CanvasKit threw an exception while laying ' + 'out the paragraph. The font was "${_paragraphStyle._fontFamily}". ' + 'Exception:\n$e'); + rethrow; + } + } + + return paragraph; + } + + // Caches laid out paragraphs and synchronously reclaims memory if there's + // memory pressure. + // + // On May 26, 2021, 500 seemed like a reasonable number to pick for the cache + // size. At the time a single laid out SkParagraph used 100KB of memory. So, + // 500 items in the cache is roughly 50MB of memory, which is not too high, + // while at the same time enough for most use-cases. + // + // TODO(yjbanov): this strategy is not sufficient for the use-case where a + // lot of paragraphs are laid out _and_ rendered. To support + // this use-case without blowing up memory usage we need this: + // https://github.com/flutter/flutter/issues/81224 + static final SynchronousSkiaObjectCache _paragraphCache = + SynchronousSkiaObjectCache(500); + + /// Marks this paragraph as having been used this frame. + /// + /// Puts this paragraph in a [SynchronousSkiaObjectCache], which will delete it + /// if there's memory pressure to do so. This protects our memory usage from + /// blowing up if within a single frame the framework needs to layout a lot of + /// paragraphs. One common use-case is `ListView.builder`, which needs to layout + /// more of its content than it actually renders to compute the scroll position. + void markUsed() { + // If the paragraph is already in the cache, just mark it as most recently + // used. Otherwise, add to cache. + if (!_paragraphCache.markUsed(this)) { + _paragraphCache.add(this); + } + } + + @override + void delete() { + _skParagraph?.delete(); + _skParagraph = null; + } + + @override + void didDelete() { + _skParagraph = null; + } @override double get alphabeticBaseline => _alphabeticBaseline; @@ -627,12 +727,12 @@ class CkParagraph implements ui.Paragraph { ui.BoxHeightStyle boxHeightStyle = ui.BoxHeightStyle.tight, ui.BoxWidthStyle boxWidthStyle = ui.BoxWidthStyle.tight, }) { - assert(!_disposed, 'Paragraph has been disposed.'); if (start < 0 || end < 0) { return const []; } - final List skRects = skiaObject.getRectsForRange( + final SkParagraph paragraph = _ensureInitialized(_lastLayoutConstraints!); + final List skRects = paragraph.getRectsForRange( start.toDouble(), end.toDouble(), toSkRectHeightStyle(boxHeightStyle), @@ -643,7 +743,6 @@ class CkParagraph implements ui.Paragraph { } List skRectsToTextBoxes(List skRects) { - assert(!_disposed, 'Paragraph has been disposed.'); final List result = []; for (int i = 0; i < skRects.length; i++) { @@ -664,8 +763,9 @@ class CkParagraph implements ui.Paragraph { @override ui.TextPosition getPositionForOffset(ui.Offset offset) { - assert(!_disposed, 'Paragraph has been disposed.'); - final SkTextPosition positionWithAffinity = skiaObject.getGlyphPositionAtCoordinate( + final SkParagraph paragraph = _ensureInitialized(_lastLayoutConstraints!); + final SkTextPosition positionWithAffinity = + paragraph.getGlyphPositionAtCoordinate( offset.dx, offset.dy, ); @@ -674,7 +774,7 @@ class CkParagraph implements ui.Paragraph { @override ui.TextRange getWordBoundary(ui.TextPosition position) { - assert(!_disposed, 'Paragraph has been disposed.'); + final SkParagraph paragraph = _ensureInitialized(_lastLayoutConstraints!); final int characterPosition; switch (position.affinity) { case ui.TextAffinity.upstream: @@ -682,46 +782,26 @@ class CkParagraph implements ui.Paragraph { case ui.TextAffinity.downstream: characterPosition = position.offset; } - final SkTextRange skRange = skiaObject.getWordBoundary(characterPosition.toDouble()); + final SkTextRange skRange = paragraph.getWordBoundary(characterPosition.toDouble()); return ui.TextRange(start: skRange.start.toInt(), end: skRange.end.toInt()); } @override void layout(ui.ParagraphConstraints constraints) { - assert(!_disposed, 'Paragraph has been disposed.'); - if (_lastLayoutConstraints == constraints.width) { + if (_lastLayoutConstraints == constraints) { return; } + _ensureInitialized(constraints); - _lastLayoutConstraints = constraints.width; - - // TODO(het): CanvasKit throws an exception when laid out with - // a font that wasn't registered. - try { - final SkParagraph paragraph = skiaObject; - paragraph.layout(constraints.width); - _alphabeticBaseline = paragraph.getAlphabeticBaseline(); - _didExceedMaxLines = paragraph.didExceedMaxLines(); - _height = paragraph.getHeight(); - _ideographicBaseline = paragraph.getIdeographicBaseline(); - _longestLine = paragraph.getLongestLine(); - _maxIntrinsicWidth = paragraph.getMaxIntrinsicWidth(); - _minIntrinsicWidth = paragraph.getMinIntrinsicWidth(); - _width = paragraph.getMaxWidth(); - _boxesForPlaceholders = - skRectsToTextBoxes(paragraph.getRectsForPlaceholders()); - } catch (e) { - printWarning('CanvasKit threw an exception while laying ' - 'out the paragraph. The font was "${_paragraphStyle._fontFamily}". ' - 'Exception:\n$e'); - rethrow; - } + // See class-level and _paragraphCache doc comments for why we're releasing + // the paragraph immediately after layout. + markUsed(); } @override ui.TextRange getLineBoundary(ui.TextPosition position) { - assert(!_disposed, 'Paragraph has been disposed.'); - final List metrics = skiaObject.getLineMetrics(); + final SkParagraph paragraph = _ensureInitialized(_lastLayoutConstraints!); + final List metrics = paragraph.getLineMetrics(); final int offset = position.offset; for (final SkLineMetrics metric in metrics) { if (offset >= metric.startIndex && offset <= metric.endIndex) { @@ -733,8 +813,8 @@ class CkParagraph implements ui.Paragraph { @override List computeLineMetrics() { - assert(!_disposed, 'Paragraph has been disposed.'); - final List skLineMetrics = skiaObject.getLineMetrics(); + final SkParagraph paragraph = _ensureInitialized(_lastLayoutConstraints!); + final List skLineMetrics = paragraph.getLineMetrics(); final List result = []; for (final SkLineMetrics metric in skLineMetrics) { result.add(CkLineMetrics._(metric)); @@ -746,8 +826,8 @@ class CkParagraph implements ui.Paragraph { @override void dispose() { - assert(!_disposed, 'Paragraph has been disposed.'); - _ref.dispose(); + delete(); + didDelete(); _disposed = true; } @@ -797,7 +877,8 @@ class CkLineMetrics implements ui.LineMetrics { class CkParagraphBuilder implements ui.ParagraphBuilder { CkParagraphBuilder(ui.ParagraphStyle style) - : _style = style as CkParagraphStyle, + : _commands = <_ParagraphCommand>[], + _style = style as CkParagraphStyle, _placeholderCount = 0, _placeholderScales = [], _styleStack = [], @@ -810,6 +891,7 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { final SkParagraphBuilder _paragraphBuilder; final CkParagraphStyle _style; + final List<_ParagraphCommand> _commands; int _placeholderCount; final List _placeholderScales; final List _styleStack; @@ -841,6 +923,7 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { } void _addPlaceholder(_CkParagraphPlaceholder placeholderStyle) { + _commands.add(_ParagraphCommand.addPlaceholder(placeholderStyle)); _paragraphBuilder.addPlaceholder( placeholderStyle.width, placeholderStyle.height, @@ -878,13 +961,14 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { fontFamilies.addAll(style.fontFamilyFallback!); } FontFallbackData.instance.ensureFontsSupportText(text, fontFamilies); + _commands.add(_ParagraphCommand.addText(text)); _paragraphBuilder.addText(text); } @override CkParagraph build() { final SkParagraph builtParagraph = _buildSkParagraph(); - return CkParagraph(builtParagraph, _style); + return CkParagraph(builtParagraph, _style, _commands); } /// Builds the CkParagraph with the builder and deletes the builder. @@ -915,6 +999,7 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { } return; } + _commands.add(const _ParagraphCommand.pop()); _styleStack.removeLast(); _paragraphBuilder.pop(); } @@ -941,6 +1026,7 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { final CkTextStyle ckStyle = style as CkTextStyle; final CkTextStyle skStyle = baseStyle.mergeWith(ckStyle); _styleStack.add(skStyle); + _commands.add(_ParagraphCommand.pushStyle(ckStyle)); if (skStyle.foreground != null || skStyle.background != null) { SkPaint? foreground = skStyle.foreground?.skiaObject; if (foreground == null) { @@ -976,6 +1062,41 @@ class _CkParagraphPlaceholder { final double offset; } +class _ParagraphCommand { + const _ParagraphCommand._( + this.type, + this.text, + this.style, + this.placeholderStyle, + ); + + const _ParagraphCommand.addText(String text) + : this._(_ParagraphCommandType.addText, text, null, null); + + const _ParagraphCommand.pop() + : this._(_ParagraphCommandType.pop, null, null, null); + + const _ParagraphCommand.pushStyle(CkTextStyle style) + : this._(_ParagraphCommandType.pushStyle, null, style, null); + + const _ParagraphCommand.addPlaceholder( + _CkParagraphPlaceholder placeholderStyle) + : this._( + _ParagraphCommandType.addPlaceholder, null, null, placeholderStyle); + + final _ParagraphCommandType type; + final String? text; + final CkTextStyle? style; + final _CkParagraphPlaceholder? placeholderStyle; +} + +enum _ParagraphCommandType { + addText, + pop, + pushStyle, + addPlaceholder, +} + List _getEffectiveFontFamilies(String? fontFamily, [List? fontFamilyFallback]) { final List fontFamilies = []; diff --git a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart index d2140d9f94618..6bdd2b301d54d 100644 --- a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart +++ b/lib/web_ui/test/canvaskit/canvaskit_api_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 'dart:js_interop'; import 'dart:math'; import 'dart:typed_data'; @@ -1433,6 +1432,7 @@ void _canvasTests() { builder.addText('Hello'); final CkParagraph paragraph = builder.build(); + paragraph.delete(); paragraph.dispose(); expect(paragraph.debugDisposed, true); }); @@ -1894,15 +1894,6 @@ void _paragraphTests() { 'http://localhost:1234/foo/canvaskit.wasm', ); }); - - test('SkObjectFinalizationRegistry', () { - // There's no reliable way to test the actual functionality of - // FinalizationRegistry because it depends on GC, which cannot be controlled, - // So the test simply tests that a FinalizationRegistry can be constructed - // and its `register` method can be called. - final SkObjectFinalizationRegistry registry = createSkObjectFinalizationRegistry((String arg) {}.toJS); - registry.register(Object(), Object()); - }); } diff --git a/lib/web_ui/test/canvaskit/image_golden_test.dart b/lib/web_ui/test/canvaskit/image_golden_test.dart index ffc9507620f2e..3fae53fc9d978 100644 --- a/lib/web_ui/test/canvaskit/image_golden_test.dart +++ b/lib/web_ui/test/canvaskit/image_golden_test.dart @@ -137,10 +137,10 @@ void _testForImageCodecs({required bool useBrowserImageDecoder}) { .makeImageAtCurrentFrame(); final CkImage image = CkImage(skImage); expect(image.debugDisposed, isFalse); - expect(image.box.isDisposed, isFalse); + expect(image.box.isDeletedPermanently, isFalse); image.dispose(); expect(image.debugDisposed, isTrue); - expect(image.box.isDisposed, isTrue); + expect(image.box.isDeletedPermanently, isTrue); // Disallow double-dispose. expect(() => image.dispose(), throwsAssertionError); @@ -152,7 +152,7 @@ void _testForImageCodecs({required bool useBrowserImageDecoder}) { canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage)! .makeImageAtCurrentFrame(); final CkImage image = CkImage(skImage); - final CountedRef box = image.box; + final SkiaObjectBox box = image.box; expect(box.refCount, 1); expect(box.debugGetStackTraces().length, 1); @@ -161,19 +161,19 @@ void _testForImageCodecs({required bool useBrowserImageDecoder}) { expect(box.debugGetStackTraces().length, 2); expect(image.isCloneOf(clone), isTrue); - expect(box.isDisposed, isFalse); + expect(box.isDeletedPermanently, isFalse); testCollector.collectNow(); expect(skImage.isDeleted(), isFalse); image.dispose(); expect(box.refCount, 1); - expect(box.isDisposed, isFalse); + expect(box.isDeletedPermanently, isFalse); testCollector.collectNow(); expect(skImage.isDeleted(), isFalse); clone.dispose(); expect(box.refCount, 0); - expect(box.isDisposed, isTrue); + expect(box.isDeletedPermanently, isTrue); testCollector.collectNow(); expect(skImage.isDeleted(), isTrue); @@ -266,6 +266,26 @@ void _testForImageCodecs({required bool useBrowserImageDecoder}) { // TODO(hterkelsen): Firefox and Safari do not currently support ImageDecoder. }, skip: isFirefox || isSafari); + // Regression test for https://github.com/flutter/flutter/issues/72469 + test('CkImage can be resurrected', () { + browserSupportsFinalizationRegistry = false; + final SkImage skImage = + canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage)! + .makeImageAtCurrentFrame(); + final CkImage image = CkImage(skImage); + expect(image.box.rawSkiaObject, isNotNull); + + // Pretend that the image is temporarily deleted. + image.box.delete(); + image.box.didDelete(); + expect(image.box.rawSkiaObject, isNull); + + // Attempting to access the skia object here would previously throw + // "Stack Overflow" in Safari. + expect(image.box.skiaObject, isNotNull); + testCollector.collectNow(); + }); + test('skiaInstantiateWebImageCodec loads an image from the network', () async { mockHttpFetchResponseFactory = (String url) async { diff --git a/lib/web_ui/test/canvaskit/native_memory_test.dart b/lib/web_ui/test/canvaskit/native_memory_test.dart deleted file mode 100644 index 42a99d284b65a..0000000000000 --- a/lib/web_ui/test/canvaskit/native_memory_test.dart +++ /dev/null @@ -1,258 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'dart:js_interop'; -import 'package:js/js.dart'; - -import 'package:test/bootstrap/browser.dart'; -import 'package:test/test.dart'; - -import 'package:ui/src/engine.dart'; - -import 'common.dart'; - -void main() { - internalBootstrapBrowserTest(() => testMain); -} - -void testMain() { - setUpCanvasKitTest(); - - late _MockNativeMemoryFinalizationRegistry mockFinalizationRegistry; - - setUp(() { - TestSkDeletableMock.deleteCount = 0; - nativeMemoryFinalizationRegistry = mockFinalizationRegistry = _MockNativeMemoryFinalizationRegistry(); - }); - - tearDown(() { - nativeMemoryFinalizationRegistry = NativeMemoryFinalizationRegistry(); - }); - - group(UniqueRef, () { - test('create-dispose-collect cycle', () { - expect(mockFinalizationRegistry.registeredPairs, hasLength(0)); - final Object owner = Object(); - final TestSkDeletable nativeObject = TestSkDeletable(); - final UniqueRef ref = UniqueRef(owner, nativeObject, 'TestSkDeletable'); - expect(ref.isDisposed, isFalse); - expect(ref.nativeObject, same(nativeObject)); - expect(TestSkDeletableMock.deleteCount, 0); - expect(mockFinalizationRegistry.registeredPairs, hasLength(1)); - expect(mockFinalizationRegistry.registeredPairs.single.owner, same(owner)); - expect(mockFinalizationRegistry.registeredPairs.single.ref, same(ref)); - - ref.dispose(); - expect(TestSkDeletableMock.deleteCount, 1); - expect(ref.isDisposed, isTrue); - expect( - reason: 'Cannot access object that was disposed', - () => ref.nativeObject, throwsA(isA()), - ); - expect( - reason: 'Cannot dispose object more than once', - () => ref.dispose(), throwsA(isA()), - ); - expect(TestSkDeletableMock.deleteCount, 1); - - // Simulate a GC - mockFinalizationRegistry.registeredPairs.single.ref.collect(); - expect( - reason: 'Manually disposed object should not be deleted again by GC.', - TestSkDeletableMock.deleteCount, 1, - ); - }); - - test('create-collect cycle', () { - expect(mockFinalizationRegistry.registeredPairs, hasLength(0)); - final Object owner = Object(); - final TestSkDeletable nativeObject = TestSkDeletable(); - final UniqueRef ref = UniqueRef(owner, nativeObject, 'TestSkDeletable'); - expect(ref.isDisposed, isFalse); - expect(ref.nativeObject, same(nativeObject)); - expect(TestSkDeletableMock.deleteCount, 0); - expect(mockFinalizationRegistry.registeredPairs, hasLength(1)); - - ref.collect(); - expect(TestSkDeletableMock.deleteCount, 1); - // There's nothing else to test for any practical gain. UniqueRef.collect - // is called when GC decided that the owner is no longer reachable. So - // there must not be anything else calling into this object for anything - // useful. - }); - }); - - group(CountedRef, () { - test('single owner', () { - expect(mockFinalizationRegistry.registeredPairs, hasLength(0)); - final TestSkDeletable nativeObject = TestSkDeletable(); - final TestCountedRefOwner owner = TestCountedRefOwner(nativeObject); - expect(owner.ref.debugReferrers, hasLength(1)); - expect(owner.ref.debugReferrers.single, owner); - expect(owner.ref.refCount, 1); - expect(owner.ref.nativeObject, nativeObject); - expect(TestSkDeletableMock.deleteCount, 0); - expect(mockFinalizationRegistry.registeredPairs, hasLength(1)); - - owner.dispose(); - expect(owner.ref.debugReferrers, isEmpty); - expect(owner.ref.refCount, 0); - expect( - reason: 'Cannot access object that was disposed', - () => owner.ref.nativeObject, throwsA(isA()), - ); - expect(TestSkDeletableMock.deleteCount, 1); - - expect( - reason: 'Cannot dispose object more than once', - () => owner.dispose(), throwsA(isA()), - ); - }); - - test('multiple owners', () { - expect(mockFinalizationRegistry.registeredPairs, hasLength(0)); - final TestSkDeletable nativeObject = TestSkDeletable(); - final TestCountedRefOwner owner1 = TestCountedRefOwner(nativeObject); - expect(owner1.ref.debugReferrers, hasLength(1)); - expect(owner1.ref.debugReferrers.single, owner1); - expect(owner1.ref.refCount, 1); - expect(owner1.ref.nativeObject, nativeObject); - expect(TestSkDeletableMock.deleteCount, 0); - expect(mockFinalizationRegistry.registeredPairs, hasLength(1)); - - final TestCountedRefOwner owner2 = owner1.clone(); - expect(owner2.ref, same(owner1.ref)); - expect(owner2.ref.debugReferrers, hasLength(2)); - expect(owner2.ref.debugReferrers.first, owner1); - expect(owner2.ref.debugReferrers.last, owner2); - expect(owner2.ref.refCount, 2); - expect(owner2.ref.nativeObject, nativeObject); - expect(TestSkDeletableMock.deleteCount, 0); - expect( - reason: 'Second owner does not add more native object owners. ' - 'The underlying shared UniqueRef is the only one.', - mockFinalizationRegistry.registeredPairs, hasLength(1), - ); - - owner1.dispose(); - expect(owner2.ref.debugReferrers, hasLength(1)); - expect(owner2.ref.debugReferrers.single, owner2); - expect(owner2.ref.refCount, 1); - expect(owner2.ref.nativeObject, nativeObject); - expect(TestSkDeletableMock.deleteCount, 0); - expect( - reason: 'The same owner cannot dispose its CountedRef more than once, even when CountedRef is still alive.', - () => owner1.dispose(), throwsA(isA()), - ); - - owner2.dispose(); - expect(owner2.ref.debugReferrers, isEmpty); - expect(owner2.ref.refCount, 0); - expect( - reason: 'Cannot access object that was disposed', - () => owner2.ref.nativeObject, throwsA(isA()), - ); - expect(TestSkDeletableMock.deleteCount, 1); - - expect( - reason: 'The same owner cannot dispose its CountedRef more than once.', - () => owner2.dispose(), throwsA(isA()), - ); - - // Simulate a GC - mockFinalizationRegistry.registeredPairs.single.ref.collect(); - expect( - reason: 'Manually disposed object should not be deleted again by GC.', - TestSkDeletableMock.deleteCount, 1, - ); - }); - }); -} - -class TestSkDeletableMock { - static int deleteCount = 0; - - bool isDeleted() => _isDeleted; - bool _isDeleted = false; - - void delete() { - expect(_isDeleted, isFalse, - reason: - 'CanvasKit does not allow deleting the same object more than once.'); - _isDeleted = true; - deleteCount++; - } - - JsConstructor get constructor => TestJsConstructor(name: 'TestSkDeletable'); -} - -@JS() -@anonymous -@staticInterop -class TestSkDeletable implements SkDeletable { - factory TestSkDeletable() { - final TestSkDeletableMock mock = TestSkDeletableMock(); - return TestSkDeletable._( - isDeleted: () { return mock.isDeleted(); }.toJS, - delete: () { return mock.delete(); }.toJS, - constructor: mock.constructor); - } - - external factory TestSkDeletable._({ - JSFunction isDeleted, - JSFunction delete, - JsConstructor constructor}); -} - -@JS() -@anonymous -@staticInterop -class TestJsConstructor implements JsConstructor { - external factory TestJsConstructor({String name}); -} - -class TestCountedRefOwner implements StackTraceDebugger { - TestCountedRefOwner(TestSkDeletable nativeObject) { - if (assertionsEnabled) { - _debugStackTrace = StackTrace.current; - } - ref = CountedRef( - nativeObject, this, 'TestCountedRefOwner'); - } - - TestCountedRefOwner.cloneOf(this.ref) { - if (assertionsEnabled) { - _debugStackTrace = StackTrace.current; - } - ref.ref(this); - } - - @override - StackTrace get debugStackTrace => _debugStackTrace; - late StackTrace _debugStackTrace; - - late final CountedRef ref; - - void dispose() { - ref.unref(this); - } - - TestCountedRefOwner clone() => TestCountedRefOwner.cloneOf(ref); -} - -class _MockNativeMemoryFinalizationRegistry implements NativeMemoryFinalizationRegistry { - final List<_MockPair> registeredPairs = <_MockPair>[]; - - @override - void register(Object owner, UniqueRef ref) { - registeredPairs.add(_MockPair(owner, ref)); - } -} - -class _MockPair { - _MockPair(this.owner, this.ref); - - Object owner; - UniqueRef ref; -} diff --git a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart index 14df75d7e709c..9e7def11ca9ba 100644 --- a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart +++ b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart @@ -12,6 +12,8 @@ import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart'; +import '../common/matchers.dart'; +import '../common/spy.dart'; import 'common.dart'; void main() { @@ -111,6 +113,242 @@ void _tests() { expect(SkiaObjects.expensiveCache.debugContains(object2), isFalse); }); }); + + group(SkiaObjectBox, () { + test('Records stack traces and respects refcounts', () async { + final ZoneSpy spy = ZoneSpy(); + spy.run(() { + Instrumentation.enabled = true; + TestSkDeletableMock.deleteCount = 0; + TestBoxWrapper.resurrectCount = 0; + final TestBoxWrapper original = TestBoxWrapper(); + + expect(original.box.debugGetStackTraces().length, 1); + expect(original.box.refCount, 1); + expect(original.box.isDeletedPermanently, isFalse); + + final TestBoxWrapper clone = original.clone(); + expect(clone.box, same(original.box)); + expect(clone.box.debugGetStackTraces().length, 2); + expect(clone.box.refCount, 2); + expect(original.box.debugGetStackTraces().length, 2); + expect(original.box.refCount, 2); + expect(original.box.isDeletedPermanently, isFalse); + + original.dispose(); + + testCollector.collectNow(); + expect(TestSkDeletableMock.deleteCount, 0); + + spy.fakeAsync.elapse(const Duration(seconds: 2)); + expect( + spy.printLog, + [ + 'Engine counters:\n TestSkDeletable created: 1\n' + ], + ); + + expect(clone.box.debugGetStackTraces().length, 1); + expect(clone.box.refCount, 1); + expect(original.box.debugGetStackTraces().length, 1); + expect(original.box.refCount, 1); + + clone.dispose(); + expect(clone.box.debugGetStackTraces().length, 0); + expect(clone.box.refCount, 0); + expect(original.box.debugGetStackTraces().length, 0); + expect(original.box.refCount, 0); + expect(original.box.isDeletedPermanently, isTrue); + + testCollector.collectNow(); + expect(TestSkDeletableMock.deleteCount, 1); + expect(TestBoxWrapper.resurrectCount, 0); + + expect(() => clone.box.unref(clone), throwsAssertionError); + spy.printLog.clear(); + spy.fakeAsync.elapse(const Duration(seconds: 2)); + expect( + spy.printLog, + [ + 'Engine counters:\n TestSkDeletable created: 1\n TestSkDeletable deleted: 1\n' + ], + ); + Instrumentation.enabled = false; + }); + }); + + test('Can resurrect Skia objects', () async { + TestSkDeletableMock.deleteCount = 0; + TestBoxWrapper.resurrectCount = 0; + final TestBoxWrapper object = TestBoxWrapper(); + expect(TestSkDeletableMock.deleteCount, 0); + expect(TestBoxWrapper.resurrectCount, 0); + + // Test 3 cycles of delete/resurrect. + for (int i = 0; i < 3; i++) { + object.box.delete(); + object.box.didDelete(); + expect(TestSkDeletableMock.deleteCount, i + 1); + expect(TestBoxWrapper.resurrectCount, i); + expect(object.box.isDeletedTemporarily, isTrue); + expect(object.box.isDeletedPermanently, isFalse); + + expect(object.box.skiaObject, isNotNull); + expect(TestSkDeletableMock.deleteCount, i + 1); + expect(TestBoxWrapper.resurrectCount, i + 1); + expect(object.box.isDeletedTemporarily, isFalse); + expect(object.box.isDeletedPermanently, isFalse); + } + + object.dispose(); + expect(object.box.isDeletedPermanently, isTrue); + }); + + test('Can dispose temporarily deleted object', () async { + TestSkDeletableMock.deleteCount = 0; + TestBoxWrapper.resurrectCount = 0; + final TestBoxWrapper object = TestBoxWrapper(); + expect(TestSkDeletableMock.deleteCount, 0); + expect(TestBoxWrapper.resurrectCount, 0); + + object.box.delete(); + object.box.didDelete(); + expect(TestSkDeletableMock.deleteCount, 1); + expect(TestBoxWrapper.resurrectCount, 0); + expect(object.box.isDeletedTemporarily, isTrue); + expect(object.box.isDeletedPermanently, isFalse); + + object.dispose(); + expect(object.box.isDeletedPermanently, isTrue); + }); + }); + + group('$SynchronousSkiaObjectCache', () { + test('is initialized empty', () { + expect(SynchronousSkiaObjectCache(10), hasLength(0)); + }); + + test('adds objects', () { + final SynchronousSkiaObjectCache cache = SynchronousSkiaObjectCache(2); + cache.add(TestSelfManagedObject()); + expect(cache, hasLength(1)); + cache.add(TestSelfManagedObject()); + expect(cache, hasLength(2)); + }); + + test('forbids adding the same object twice', () { + final SynchronousSkiaObjectCache cache = SynchronousSkiaObjectCache(2); + final TestSelfManagedObject object = TestSelfManagedObject(); + cache.add(object); + expect(cache, hasLength(1)); + expect(() => cache.add(object), throwsAssertionError); + }); + + void expectObjectInCache( + SynchronousSkiaObjectCache cache, + TestSelfManagedObject object, + ) { + expect(cache.debugContains(object), isTrue); + expect(object._skiaObject, isNotNull); + } + + void expectObjectNotInCache( + SynchronousSkiaObjectCache cache, + TestSelfManagedObject object, + ) { + expect(cache.debugContains(object), isFalse); + expect(object._skiaObject, isNull); + } + + test('respects maximumSize', () { + final SynchronousSkiaObjectCache cache = SynchronousSkiaObjectCache(2); + final TestSelfManagedObject object1 = TestSelfManagedObject(); + final TestSelfManagedObject object2 = TestSelfManagedObject(); + final TestSelfManagedObject object3 = TestSelfManagedObject(); + final TestSelfManagedObject object4 = TestSelfManagedObject(); + + cache.add(object1); + expect(cache, hasLength(1)); + expectObjectInCache(cache, object1); + + cache.add(object2); + expect(cache, hasLength(2)); + expectObjectInCache(cache, object1); + expectObjectInCache(cache, object2); + + cache.add(object3); + expect(cache, hasLength(2)); + expectObjectNotInCache(cache, object1); + expectObjectInCache(cache, object2); + expectObjectInCache(cache, object3); + + cache.add(object4); + expect(cache, hasLength(2)); + expectObjectNotInCache(cache, object1); + expectObjectNotInCache(cache, object2); + expectObjectInCache(cache, object3); + expectObjectInCache(cache, object4); + }); + + test('uses RLU strategy', () { + final SynchronousSkiaObjectCache cache = SynchronousSkiaObjectCache(2); + final TestSelfManagedObject object1 = TestSelfManagedObject(); + final TestSelfManagedObject object2 = TestSelfManagedObject(); + final TestSelfManagedObject object3 = TestSelfManagedObject(); + final TestSelfManagedObject object4 = TestSelfManagedObject(); + + cache.add(object1); + expectObjectInCache(cache, object1); + cache.add(object2); + expectObjectInCache(cache, object2); + cache.add(object3); + expectObjectInCache(cache, object3); + expectObjectNotInCache(cache, object1); + + cache.markUsed(object2); + cache.add(object4); + expectObjectInCache(cache, object2); + expectObjectNotInCache(cache, object3); + expectObjectInCache(cache, object4); + }); + }); +} + +/// A simple class that wraps a [SkiaObjectBox]. +/// +/// Can be [clone]d such that the clones share the same ref counted box. +class TestBoxWrapper implements StackTraceDebugger { + TestBoxWrapper() { + if (assertionsEnabled) { + _debugStackTrace = StackTrace.current; + } + box = SkiaObjectBox.resurrectable( + this, TestSkDeletable(), () { + resurrectCount += 1; + return TestSkDeletable(); + }); + } + + TestBoxWrapper.cloneOf(this.box) { + if (assertionsEnabled) { + _debugStackTrace = StackTrace.current; + } + box.ref(this); + } + + static int resurrectCount = 0; + + @override + StackTrace get debugStackTrace => _debugStackTrace; + late StackTrace _debugStackTrace; + + late SkiaObjectBox box; + + void dispose() { + box.unref(this); + } + + TestBoxWrapper clone() => TestBoxWrapper.cloneOf(box); } class TestSkDeletableMock {