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 94b04f5c1ab..8c0ccee7da0 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -99,6 +99,23 @@ class CanvasKit { /// Creates an [SkPath] using commands obtained from [SkPath.toCmds]. // TODO(yjbanov): switch to CanvasKit.Path.MakeFromCmds when it's available. external SkPath MakePathFromCmds(List pathCommands); + + /// Creates an image from decoded pixels represented as a list of bytes. + /// + /// The pixel data must match the [width], [height], [alphaType], [colorType], + /// and [colorSpace]. + /// + /// Typically pixel data is obtained using [SkImage.readPixels]. The + /// parameters specified in [SkImageInfo] passed [SkImage.readPixels] much + /// match the arguments passed to this method. + external SkImage MakeImage( + Uint8List imageData, + int width, + int height, + SkAlphaType alphaType, + SkColorType colorType, + ColorSpace colorSpace, + ); } @JS('window.CanvasKitInit') @@ -1726,48 +1743,159 @@ class TypefaceFontProviderNamespace { external TypefaceFontProvider Make(); } -Timer? _skObjectCollector; -List _skObjectDeleteQueue = []; +/// Collects Skia objects that are no longer necessary. +abstract class Collector { + /// The production collector implementation. + static final Collector _productionInstance = ProductionCollector(); -final SkObjectFinalizationRegistry skObjectFinalizationRegistry = - SkObjectFinalizationRegistry(js.allowInterop((SkDeletable deletable) { - _scheduleSkObjectCollection(deletable); -})); + /// The collector implementation currently in use. + static Collector get instance => _instance; + static Collector _instance = _productionInstance; -/// Schedules a Skia object for deletion in an asap timer. + /// In tests overrides the collector implementation. + static void debugOverrideCollector(Collector override) { + _instance = override; + } + + /// In tests restores the collector to the production implementation. + static void debugRestoreCollector() { + _instance = _productionInstance; + } + + /// Registers a [deletable] for collection when the [wrapper] object is + /// garbage collected. + /// + /// The [debugLabel] is used to track the origin of the deletable. + void register(Object wrapper, SkDeletable deletable); + + /// Deletes the [deletable]. + /// + /// The exact timing of the deletion is implementation-specific. For example, + /// a production implementation may want to batch deletables and schedule a + /// timer to collect them instead of deleting right away. + /// + /// A test implementation may want a collection strategy that's less efficient + /// but more predictable. + void collect(SkDeletable deletable); +} + +/// Uses the browser's real `FinalizationRegistry` to collect objects. /// -/// A timer is used for the following reasons: -/// -/// - Deleting the object immediately may lead to dangling pointer as the Skia -/// object may still be used by a function in the current frame. For example, -/// a `CkPaint` + `SkPaint` pair may be created by the framework, passed to -/// the engine, and the `CkPaint` dropped immediately. Because GC can kick in -/// any time, including in the middle of the event, we may delete `SkPaint` -/// prematurely. -/// - A microtask, while solves the problem above, would prevent the event from -/// yielding to the graphics system to render the frame on the screen if there -/// is a large number of objects to delete, causing jank. -/// -/// Because scheduling a timer is expensive, the timer is shared by all objects -/// deleted this frame. No timer is created if no objects were scheduled for -/// deletion. -void _scheduleSkObjectCollection(SkDeletable deletable) { - _skObjectDeleteQueue.add(deletable); - _skObjectCollector ??= Timer(Duration.zero, () { +/// Uses timers to delete objects in batches and outside the animation frame. +class ProductionCollector implements Collector { + ProductionCollector() { + _skObjectFinalizationRegistry = SkObjectFinalizationRegistry(js.allowInterop((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 + // skip collection of this object. + if (!deletable.isDeleted()) { + collect(deletable); + } + })); + } + + late final SkObjectFinalizationRegistry _skObjectFinalizationRegistry; + List _skiaObjectCollectionQueue = []; + Timer? _skiaObjectCollectionTimer; + + @override + void register(Object wrapper, SkDeletable deletable) { + _skObjectFinalizationRegistry.register(wrapper, deletable); + } + + /// Schedules a Skia object for deletion in an asap timer. + /// + /// A timer is used for the following reasons: + /// + /// - Deleting the object immediately may lead to dangling pointer as the Skia + /// object may still be used by a function in the current frame. For example, + /// a `CkPaint` + `SkPaint` pair may be created by the framework, passed to + /// the engine, and the `CkPaint` dropped immediately. Because GC can kick in + /// any time, including in the middle of the event, we may delete `SkPaint` + /// prematurely. + /// - A microtask, while solves the problem above, would prevent the event from + /// yielding to the graphics system to render the frame on the screen if there + /// is a large number of objects to delete, causing jank. + /// + /// Because scheduling a timer is expensive, the timer is shared by all objects + /// deleted this frame. No timer is created if no objects were scheduled for + /// deletion. + @override + void collect(SkDeletable deletable) { + assert( + !deletable.isDeleted(), + 'Attempted to delete an already deleted Skia object.', + ); + _skiaObjectCollectionQueue.add(deletable); + + _skiaObjectCollectionTimer ??= Timer(Duration.zero, () { + // Null out the timer so we can schedule a new one next time objects are + // scheduled for deletion. + _skiaObjectCollectionTimer = null; + collectSkiaObjectsNow(); + }); + } + + /// Deletes all Skia objects pending deletion synchronously. + /// + /// After calling this method [_skiaObjectCollectionQueue] is empty. + /// + /// Throws a [SkiaObjectCollectionError] if CanvasKit fails to delete at least + /// one object. The error is populated with information about the first failed + /// object. Upon an error the collection continues and the collection queue is + /// emptied out to prevent memory leaks. This may happen, for example, when the + /// same object is deleted more than once. + void collectSkiaObjectsNow() { html.window.performance.mark('SkObject collection-start'); - final int length = _skObjectDeleteQueue.length; + final int length = _skiaObjectCollectionQueue.length; + dynamic firstError; + StackTrace? firstStackTrace; for (int i = 0; i < length; i++) { - _skObjectDeleteQueue[i].delete(); + final SkDeletable deletable = _skiaObjectCollectionQueue[i]; + if (deletable.isDeleted()) { + // Some Skia objects are ref counted and are deleted before GC and/or + // the collection timer begins collecting them. So we have to check + // again if the objects is worth collecting. + continue; + } + try { + deletable.delete(); + } catch (error, stackTrace) { + // Remember the error, but keep going. If for some reason CanvasKit fails + // to delete an object we still want to delete other objects and empty + // out the queue. Otherwise, the queue will never be flushed and keep + // accumulating objects, a.k.a. memory leak. + if (firstError == null) { + firstError = error; + firstStackTrace = stackTrace; + } + } } - _skObjectDeleteQueue = []; + _skiaObjectCollectionQueue = []; - // Null out the timer so we can schedule a new one next time objects are - // scheduled for deletion. - _skObjectCollector = null; html.window.performance.mark('SkObject collection-end'); html.window.performance.measure('SkObject collection', 'SkObject collection-start', 'SkObject collection-end'); - }); + + // It's safe to throw the error here, now that we've processed the queue. + if (firstError != null) { + throw SkiaObjectCollectionError(firstError, firstStackTrace); + } + } +} + +/// Thrown by [ProductionCollector] when Skia object collection fails. +class SkiaObjectCollectionError implements Error { + SkiaObjectCollectionError(this.error, this.stackTrace); + + final dynamic error; + + @override + final StackTrace? stackTrace; + + @override + String toString() => 'SkiaObjectCollectionError: $error\n$stackTrace'; } /// Any Skia object that has a `delete` method. @@ -1776,6 +1904,9 @@ void _scheduleSkObjectCollection(SkDeletable deletable) { class SkDeletable { /// Deletes the C++ side object. external void delete(); + + /// Returns whether the correcponding C++ object has been deleted. + external bool isDeleted(); } /// Attaches a weakly referenced object to another object and calls a finalizer diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index 5c558d5fda5..e4f64121383 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -41,24 +41,24 @@ Future skiaInstantiateWebImageCodec( /// The CanvasKit implementation of [ui.Codec]. /// /// Wraps `SkAnimatedImage`. -class CkAnimatedImage implements ui.Codec, StackTraceDebugger { +class CkAnimatedImage extends ManagedSkiaObject implements ui.Codec { /// Decodes an image from a list of encoded bytes. - CkAnimatedImage.decodeFromBytes(Uint8List bytes) { - if (assertionsEnabled) { - _debugStackTrace = StackTrace.current; - } - final SkAnimatedImage skAnimatedImage = - canvasKit.MakeAnimatedImageFromEncoded(bytes); - box = SkiaObjectBox(this, skAnimatedImage); - } + CkAnimatedImage.decodeFromBytes(this._bytes); - // Use a box because `CkAnimatedImage` may be deleted either due to this - // object being garbage-collected, or by an explicit call to [dispose]. - late final SkiaObjectBox box; + final Uint8List _bytes; @override - StackTrace get debugStackTrace => _debugStackTrace!; - StackTrace? _debugStackTrace; + SkAnimatedImage createDefault() { + return canvasKit.MakeAnimatedImageFromEncoded(_bytes); + } + + @override + SkAnimatedImage resurrect() => createDefault(); + + @override + void delete() { + rawSkiaObject?.delete(); + } bool _disposed = false; bool get debugDisposed => _disposed; @@ -75,29 +75,27 @@ class CkAnimatedImage implements ui.Codec, StackTraceDebugger { 'Cannot dispose a codec that has already been disposed.', ); _disposed = true; - - // This image is no longer usable. Bump the ref count. - box.unref(this); + delete(); } @override int get frameCount { assert(_debugCheckIsNotDisposed()); - return box.skiaObject.getFrameCount(); + return skiaObject.getFrameCount(); } @override int get repetitionCount { assert(_debugCheckIsNotDisposed()); - return box.skiaObject.getRepetitionCount(); + return skiaObject.getRepetitionCount(); } @override Future getNextFrame() { assert(_debugCheckIsNotDisposed()); - final int durationMillis = box.skiaObject.decodeNextFrame(); + final int durationMillis = skiaObject.decodeNextFrame(); final Duration duration = Duration(milliseconds: durationMillis); - final CkImage image = CkImage(box.skiaObject.getCurrentFrame()); + final CkImage image = CkImage(skiaObject.getCurrentFrame()); return Future.value(AnimatedImageFrameInfo(duration, image)); } } @@ -108,7 +106,35 @@ class CkImage implements ui.Image, StackTraceDebugger { if (assertionsEnabled) { _debugStackTrace = StackTrace.current; } - box = SkiaObjectBox(this, skImage); + 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, + ); + box = SkiaObjectBox.resurrectable(this, skImage, () { + return canvasKit.MakeImage( + originalBytes.buffer.asUint8List(), + width, + height, + canvasKit.AlphaType.Premul, + canvasKit.ColorType.RGBA_8888, + SkColorSpaceSRGB, + ); + }); + } } CkImage.cloneOf(this.box) { @@ -187,18 +213,35 @@ class CkImage implements ui.Image, StackTraceDebugger { } @override - Future toByteData( - {ui.ImageByteFormat format = ui.ImageByteFormat.rawRgba}) { + Future toByteData({ + ui.ImageByteFormat format = ui.ImageByteFormat.rawRgba, + }) { assert(_debugCheckIsNotDisposed()); + return Future.value(_encodeImage( + skImage: skImage, + format: format, + alphaType: canvasKit.AlphaType.Premul, + colorType: canvasKit.ColorType.RGBA_8888, + colorSpace: SkColorSpaceSRGB, + )); + } + + static ByteData _encodeImage({ + required SkImage skImage, + required ui.ImageByteFormat format, + required SkAlphaType alphaType, + required SkColorType colorType, + required ColorSpace colorSpace, + }) { Uint8List bytes; if (format == ui.ImageByteFormat.rawRgba) { final SkImageInfo imageInfo = SkImageInfo( - alphaType: canvasKit.AlphaType.Premul, - colorType: canvasKit.ColorType.RGBA_8888, - colorSpace: SkColorSpaceSRGB, - width: width, - height: height, + alphaType: alphaType, + colorType: colorType, + colorSpace: colorSpace, + width: skImage.width(), + height: skImage.height(), ); bytes = skImage.readPixels(imageInfo, 0, 0); } else { @@ -208,8 +251,7 @@ class CkImage implements ui.Image, StackTraceDebugger { skData.delete(); } - final ByteData data = bytes.buffer.asByteData(0, bytes.length); - return Future.value(data); + return bytes.buffer.asByteData(0, bytes.length); } @override 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 17ef95318ba..96b6ebf6605 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 @@ -143,7 +143,7 @@ abstract class ManagedSkiaObject extends SkiaObject { if (browserSupportsFinalizationRegistry) { // If FinalizationRegistry is supported we will only ever need the // default object, as we know precisely when to delete it. - skObjectFinalizationRegistry.register(this, defaultObject); + Collector.instance.register(this, defaultObject as SkDeletable); } else { // If FinalizationRegistry is _not_ supported we may need to delete // and resurrect the object multiple times before deleting it forever. @@ -228,7 +228,7 @@ abstract class OneShotSkiaObject extends SkiaObject { OneShotSkiaObject(T skObject) : this.rawSkiaObject = skObject { if (browserSupportsFinalizationRegistry) { - skObjectFinalizationRegistry.register(this, skObject); + Collector.instance.register(this, skObject as SkDeletable); } else { SkiaObjects.manageOneShot(this); } @@ -261,25 +261,51 @@ abstract class StackTraceDebugger { 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. /// -/// When the wrapper is garbage collected, decrements the refcount (only in -/// browsers that support weak references). +/// The [ref] method can be used to increment the refcount to tell this box to +/// keep the underlying Skia object alive. /// -/// The [delete] method can be used to eagerly decrement the refcount before the -/// wrapper is garbage collected. +/// 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]). /// -/// The [delete] method may be called any number of times. The box -/// will only delete the object once. -class SkiaObjectBox { - SkiaObjectBox(R debugReferrer, this.skiaObject) : _skDeletable = skiaObject as SkDeletable { +/// 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) : _resurrector = null { + assert(browserSupportsFinalizationRegistry); + _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); + SkiaObjects.manageExpensive(this); + } + + void _initialize(R debugReferrer, T initialValue) { + _update(initialValue); if (assertionsEnabled) { debugReferrers.add(debugReferrer); } - if (browserSupportsFinalizationRegistry) { - boxRegistry.register(this, _skDeletable); - } assert(refCount == debugReferrers.length); } @@ -312,25 +338,57 @@ class SkiaObjectBox { /// /// Do not store this value outside this box. It is memory-managed by /// [SkiaObjectBox]. Storing it may result in use-after-free bugs. - final T skiaObject; - final SkDeletable _skDeletable; + T? rawSkiaObject; + SkDeletable? _skDeletable; + Resurrector? _resurrector; - /// Whether this object has been deleted. - bool get isDeleted => _isDeleted; - bool _isDeleted = false; + void _update(T? newSkiaObject) { + rawSkiaObject = newSkiaObject; + _skDeletable = newSkiaObject as SkDeletable?; + } - /// Deletes Skia objects when their wrappers are garbage collected. - static final SkObjectFinalizationRegistry boxRegistry = - SkObjectFinalizationRegistry(js.allowInterop((SkDeletable deletable) { - deletable.delete(); - })); + @override + T get skiaObject => rawSkiaObject ?? _doResurrect(); + + T _doResurrect() { + assert(!browserSupportsFinalizationRegistry); + assert(_resurrector != null); + assert(!_isDeletedPermanently, 'Cannot use deleted object.'); + _update(_resurrector!()); + SkiaObjects.manageExpensive(this); + return skiaObject; + } + + @override + void delete() { + _skDeletable?.delete(); + } + + @override + void didDelete() { + 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(!_isDeleted, 'Cannot increment ref count on a deleted handle.'); + assert(!_isDeletedPermanently, 'Cannot increment ref count on a deleted handle.'); assert(_refCount > 0); assert( debugReferrers.add(debugReferrer), @@ -347,7 +405,7 @@ class SkiaObjectBox { /// If this causes the reference count to drop to zero, deletes the /// [skObject]. void unref(R debugReferrer) { - assert(!_isDeleted, 'Attempted to unref an already deleted Skia object.'); + 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.', @@ -355,8 +413,19 @@ class SkiaObjectBox { _refCount -= 1; assert(refCount == debugReferrers.length); if (_refCount == 0) { - _isDeleted = true; - _scheduleSkObjectCollection(_skDeletable); + // 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 { + _skDeletable!.delete(); + } + } + rawSkiaObject = null; + _skDeletable = null; + _resurrector = null; + _isDeletedPermanently = true; } } } diff --git a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart index 24981933048..89022a27dfc 100644 --- a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart +++ b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart @@ -20,13 +20,7 @@ void main() { void testMain() { group('CanvasKit API', () { - setUpAll(() async { - await ui.webOnlyInitializePlatform(); - }); - - test('Using CanvasKit', () { - expect(useCanvasKit, true); - }); + setUpCanvasKitTest(); _blendModeTests(); _paintStyleTests(); diff --git a/lib/web_ui/test/canvaskit/common.dart b/lib/web_ui/test/canvaskit/common.dart index 1d7731c9abd..46c319bc9cc 100644 --- a/lib/web_ui/test/canvaskit/common.dart +++ b/lib/web_ui/test/canvaskit/common.dart @@ -2,10 +2,161 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.6 +import 'package:test/test.dart'; + import 'package:ui/src/engine.dart'; +import 'package:ui/ui.dart' as ui; /// Whether we are running on iOS Safari. // TODO: https://github.com/flutter/flutter/issues/60040 bool get isIosSafari => browserEngine == BrowserEngine.webkit && operatingSystem == OperatingSystem.iOs; + +/// Used in tests instead of [ProductionCollector] to control Skia object +/// collection explicitly, and to prevent leaks across tests. +/// +/// See [TestCollector] for usage. +late TestCollector testCollector; + +/// Common test setup for all CanvasKit unit-tests. +void setUpCanvasKitTest() { + setUpAll(() async { + expect(useCanvasKit, true, + reason: 'This test must run in CanvasKit mode.'); + debugResetBrowserSupportsFinalizationRegistry(); + await ui.webOnlyInitializePlatform(); + }); + + setUp(() async { + testCollector = TestCollector(); + Collector.debugOverrideCollector(testCollector); + }); + + tearDown(() { + testCollector.cleanUpAfterTest(); + debugResetBrowserSupportsFinalizationRegistry(); + }); + + tearDownAll(() { + debugResetBrowserSupportsFinalizationRegistry(); + }); +} + +class _TestFinalizerRegistration { + _TestFinalizerRegistration(this.wrapper, this.deletable, this.stackTrace); + + final Object wrapper; + final SkDeletable deletable; + final StackTrace stackTrace; +} + +class _TestCollection { + _TestCollection(this.deletable, this.stackTrace); + + final SkDeletable deletable; + final StackTrace stackTrace; +} + +/// Provides explicit synchronous API for collecting Skia objects in tests. +/// +/// [ProductionCollector] relies on `FinalizationRegistry` and timers to +/// delete Skia objects, which makes it more precise and efficient. However, +/// it also makes it unpredictable. For example, an object created in one +/// test may be collected while running another test because the timing is +/// subject to browser-specific GC scheduling. +/// +/// Tests should use [collectNow] and [collectAfterTest] to trigger collections. +class TestCollector implements Collector { + final List<_TestFinalizerRegistration> _activeRegistrations = <_TestFinalizerRegistration>[]; + final List<_TestFinalizerRegistration> _collectedRegistrations = <_TestFinalizerRegistration>[]; + + final List<_TestCollection> _pendingCollections = <_TestCollection>[]; + final List<_TestCollection> _completedCollections = <_TestCollection>[]; + + @override + void register(Object wrapper, SkDeletable deletable) { + _activeRegistrations.add( + _TestFinalizerRegistration(wrapper, deletable, StackTrace.current), + ); + } + + @override + void collect(SkDeletable deletable) { + _pendingCollections.add( + _TestCollection(deletable, StackTrace.current), + ); + } + + /// Deletes all Skia objects scheduled for collection. + void collectNow() { + for (_TestCollection collection in _pendingCollections) { + late final _TestFinalizerRegistration? activeRegistration; + for (_TestFinalizerRegistration registration in _activeRegistrations) { + if (identical(registration.deletable, collection.deletable)) { + activeRegistration = registration; + break; + } + } + if (activeRegistration == null) { + late final _TestFinalizerRegistration? collectedRegistration; + for (_TestFinalizerRegistration registration in _collectedRegistrations) { + if (identical(registration.deletable, collection.deletable)) { + collectedRegistration = registration; + break; + } + } + if (collectedRegistration == null) { + fail( + 'Attempted to collect an object that was never registered for finalization.\n' + 'The collection was requested here:\n' + '${collection.stackTrace}' + ); + } else { + final _TestCollection firstCollection = _completedCollections.firstWhere( + (_TestCollection completedCollection) { + return identical(completedCollection.deletable, collection.deletable); + } + ); + fail( + 'Attempted to collect an object that was previously collected.\n' + 'The object was registered for finalization here:\n' + '${collection.stackTrace}\n\n' + 'The first collection was requested here:\n' + '${firstCollection.stackTrace}\n\n' + 'The second collection was requested here:\n' + '${collection.stackTrace}' + ); + } + } else { + _collectedRegistrations.add(activeRegistration); + _activeRegistrations.remove(activeRegistration); + _completedCollections.add(collection); + if (!collection.deletable.isDeleted()) { + collection.deletable.delete(); + } + } + } + _pendingCollections.clear(); + } + + /// Deletes all Skia objects with registered finalizers. + /// + /// This also deletes active objects that have not been scheduled for + /// collection, to prevent objects leaking across tests. + void cleanUpAfterTest() { + for (_TestCollection collection in _pendingCollections) { + if (!collection.deletable.isDeleted()) { + collection.deletable.delete(); + } + } + for (_TestFinalizerRegistration registration in _activeRegistrations) { + if (!registration.deletable.isDeleted()) { + registration.deletable.delete(); + } + } + _activeRegistrations.clear(); + _collectedRegistrations.clear(); + _pendingCollections.clear(); + _completedCollections.clear(); + } +} diff --git a/lib/web_ui/test/canvaskit/filter_test.dart b/lib/web_ui/test/canvaskit/filter_test.dart index e46dc7aad67..b548ed7a972 100644 --- a/lib/web_ui/test/canvaskit/filter_test.dart +++ b/lib/web_ui/test/canvaskit/filter_test.dart @@ -47,9 +47,7 @@ void testMain() { } group('ImageFilters', () { - setUpAll(() async { - await ui.webOnlyInitializePlatform(); - }); + setUpCanvasKitTest(); test('can be constructed', () { final CkImageFilter imageFilter = CkImageFilter.blur(sigmaX: 5, sigmaY: 10); diff --git a/lib/web_ui/test/canvaskit/frame_timings_test.dart b/lib/web_ui/test/canvaskit/frame_timings_test.dart index 38d775476be..59a57f8b8b8 100644 --- a/lib/web_ui/test/canvaskit/frame_timings_test.dart +++ b/lib/web_ui/test/canvaskit/frame_timings_test.dart @@ -5,8 +5,6 @@ // @dart = 2.6 import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; -import 'package:ui/src/engine.dart'; -import 'package:ui/ui.dart' as ui; import 'common.dart'; import '../frame_timings_common.dart'; @@ -17,13 +15,7 @@ void main() { void testMain() { group('frame timings', () { - setUpAll(() async { - await ui.webOnlyInitializePlatform(); - }); - - test('Using CanvasKit', () { - expect(useCanvasKit, true); - }); + setUpCanvasKitTest(); test('collects frame timings', () async { await runFrameTimingsTest(); diff --git a/lib/web_ui/test/canvaskit/image_test.dart b/lib/web_ui/test/canvaskit/image_test.dart index 5141464fb52..0f2073b0762 100644 --- a/lib/web_ui/test/canvaskit/image_test.dart +++ b/lib/web_ui/test/canvaskit/image_test.dart @@ -20,34 +20,22 @@ void main() { void testMain() { group('CanvasKit image', () { - setUpAll(() async { - await ui.webOnlyInitializePlatform(); - }); + setUpCanvasKitTest(); test('CkAnimatedImage can be explicitly disposed of', () { final CkAnimatedImage image = CkAnimatedImage.decodeFromBytes(kTransparentImage); - expect(image.box.isDeleted, false); expect(image.debugDisposed, false); image.dispose(); - expect(image.box.isDeleted, true); expect(image.debugDisposed, true); + // Disallow usage after disposal + expect(() => image.frameCount, throwsAssertionError); + expect(() => image.repetitionCount, throwsAssertionError); + expect(() => image.getNextFrame(), throwsAssertionError); + // Disallow double-dispose. expect(() => image.dispose(), throwsAssertionError); - }); - - test('CkAnimatedImage can be cloned and explicitly disposed of', () async { - final CkAnimatedImage image = CkAnimatedImage.decodeFromBytes(kTransparentImage); - final SkAnimatedImage skAnimatedImage = image.box.skiaObject; - final SkiaObjectBox box = image.box; - expect(box.refCount, 1); - expect(box.debugGetStackTraces().length, 1); - - image.dispose(); - expect(box.isDeleted, true); - await Future.delayed(Duration.zero); - expect(skAnimatedImage.isDeleted(), true); - expect(box.debugGetStackTraces().length, 0); + testCollector.collectNow(); }); test('CkImage toString', () { @@ -57,6 +45,7 @@ void testMain() { final CkImage image = CkImage(skImage); expect(image.toString(), '[1×1]'); image.dispose(); + testCollector.collectNow(); }); test('CkImage can be explicitly disposed of', () { @@ -65,13 +54,14 @@ void testMain() { .getCurrentFrame(); final CkImage image = CkImage(skImage); expect(image.debugDisposed, false); - expect(image.box.isDeleted, false); + expect(image.box.isDeletedPermanently, false); image.dispose(); expect(image.debugDisposed, true); - expect(image.box.isDeleted, true); + expect(image.box.isDeletedPermanently, true); // Disallow double-dispose. expect(() => image.dispose(), throwsAssertionError); + testCollector.collectNow(); }); test('CkImage can be explicitly disposed of when cloned', () async { @@ -83,29 +73,36 @@ void testMain() { expect(box.refCount, 1); expect(box.debugGetStackTraces().length, 1); - final CkImage imageClone = image.clone(); + final CkImage clone = image.clone(); expect(box.refCount, 2); expect(box.debugGetStackTraces().length, 2); - expect(image.isCloneOf(imageClone), true); - expect(box.isDeleted, false); - await Future.delayed(Duration.zero); + expect(image.isCloneOf(clone), true); + expect(box.isDeletedPermanently, false); + + testCollector.collectNow(); expect(skImage.isDeleted(), false); image.dispose(); - expect(box.isDeleted, false); - await Future.delayed(Duration.zero); + expect(box.refCount, 1); + expect(box.isDeletedPermanently, false); + + testCollector.collectNow(); expect(skImage.isDeleted(), false); - imageClone.dispose(); - expect(box.isDeleted, true); - await Future.delayed(Duration.zero); + clone.dispose(); + expect(box.refCount, 0); + expect(box.isDeletedPermanently, true); + + testCollector.collectNow(); expect(skImage.isDeleted(), true); expect(box.debugGetStackTraces().length, 0); + testCollector.collectNow(); }); test('skiaInstantiateWebImageCodec throws exception if given invalid URL', () async { expect(skiaInstantiateWebImageCodec('invalid-url', null), throwsA(isA())); + testCollector.collectNow(); }); test('CkImage toByteData', () async { @@ -115,6 +112,7 @@ void testMain() { final CkImage image = CkImage(skImage); expect((await image.toByteData()).lengthInBytes, greaterThan(0)); expect((await image.toByteData(format: ui.ImageByteFormat.png)).lengthInBytes, greaterThan(0)); + testCollector.collectNow(); }); // TODO: https://github.com/flutter/flutter/issues/60040 }, skip: isIosSafari); diff --git a/lib/web_ui/test/canvaskit/path_test.dart b/lib/web_ui/test/canvaskit/path_test.dart index b90f6d12fbb..151323f5303 100644 --- a/lib/web_ui/test/canvaskit/path_test.dart +++ b/lib/web_ui/test/canvaskit/path_test.dart @@ -17,14 +17,7 @@ void main() { void testMain() { group('CkPath', () { - setUpAll(() async { - debugResetBrowserSupportsFinalizationRegistry(); - await ui.webOnlyInitializePlatform(); - }); - - tearDown(() { - debugResetBrowserSupportsFinalizationRegistry(); - }); + setUpCanvasKitTest(); test('Using CanvasKit', () { expect(useCanvasKit, true); diff --git a/lib/web_ui/test/canvaskit/shader_test.dart b/lib/web_ui/test/canvaskit/shader_test.dart index dfa3f2dd12b..0036a390d96 100644 --- a/lib/web_ui/test/canvaskit/shader_test.dart +++ b/lib/web_ui/test/canvaskit/shader_test.dart @@ -19,9 +19,7 @@ void main() { void testMain() { group('CanvasKit shaders', () { - setUpAll(() async { - await ui.webOnlyInitializePlatform(); - }); + setUpCanvasKitTest(); test('Sweep gradient', () { final CkGradientSweep gradient = ui.Gradient.sweep( 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 aa1b808fa24..2ec933b23e8 100644 --- a/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart +++ b/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart @@ -8,7 +8,6 @@ import 'package:mockito/mockito.dart'; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; -import 'package:ui/ui.dart' as ui; import 'package:ui/src/engine.dart'; import '../matchers.dart'; @@ -27,21 +26,15 @@ void testMain() { void _tests() { SkiaObjects.maximumCacheSize = 4; - bool originalBrowserSupportsFinalizationRegistry; - setUpAll(() async { - await ui.webOnlyInitializePlatform(); + setUpCanvasKitTest(); + setUp(() async { // Pretend the browser does not support FinalizationRegistry so we can test the // resurrection logic. - originalBrowserSupportsFinalizationRegistry = browserSupportsFinalizationRegistry; browserSupportsFinalizationRegistry = false; }); - tearDownAll(() { - browserSupportsFinalizationRegistry = originalBrowserSupportsFinalizationRegistry; - }); - group(ManagedSkiaObject, () { test('implements create, cache, delete, resurrect, delete lifecycle', () { int addPostFrameCallbackCount = 0; @@ -158,11 +151,12 @@ void _tests() { group(SkiaObjectBox, () { test('Records stack traces and respects refcounts', () async { TestSkDeletable.deleteCount = 0; + TestBoxWrapper.resurrectCount = 0; final TestBoxWrapper original = TestBoxWrapper(); expect(original.box.debugGetStackTraces().length, 1); expect(original.box.refCount, 1); - expect(original.box.isDeleted, false); + expect(original.box.isDeletedPermanently, false); final TestBoxWrapper clone = original.clone(); expect(clone.box, same(original.box)); @@ -170,12 +164,11 @@ void _tests() { expect(clone.box.refCount, 2); expect(original.box.debugGetStackTraces().length, 2); expect(original.box.refCount, 2); - expect(original.box.isDeleted, false); + expect(original.box.isDeletedPermanently, false); original.dispose(); - // Let Skia object delete queue run. - await Future.delayed(Duration.zero); + testCollector.collectNow(); expect(TestSkDeletable.deleteCount, 0); expect(clone.box.debugGetStackTraces().length, 1); @@ -184,19 +177,63 @@ void _tests() { expect(original.box.refCount, 1); clone.dispose(); - - // Let Skia object delete queue run. - await Future.delayed(Duration.zero); - expect(TestSkDeletable.deleteCount, 1); - 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.isDeleted, true); + expect(original.box.isDeletedPermanently, true); + + testCollector.collectNow(); + expect(TestSkDeletable.deleteCount, 1); + expect(TestBoxWrapper.resurrectCount, 0); expect(() => clone.box.unref(clone), throwsAssertionError); }); + + test('Can resurrect Skia objects', () async { + TestSkDeletable.deleteCount = 0; + TestBoxWrapper.resurrectCount = 0; + final TestBoxWrapper object = TestBoxWrapper(); + expect(TestSkDeletable.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(TestSkDeletable.deleteCount, i + 1); + expect(TestBoxWrapper.resurrectCount, i); + expect(object.box.isDeletedTemporarily, true); + expect(object.box.isDeletedPermanently, false); + + expect(object.box.skiaObject, isNotNull); + expect(TestSkDeletable.deleteCount, i + 1); + expect(TestBoxWrapper.resurrectCount, i + 1); + expect(object.box.isDeletedTemporarily, false); + expect(object.box.isDeletedPermanently, false); + } + + object.dispose(); + expect(object.box.isDeletedPermanently, true); + }); + + test('Can dispose temporarily deleted object', () async { + TestSkDeletable.deleteCount = 0; + TestBoxWrapper.resurrectCount = 0; + final TestBoxWrapper object = TestBoxWrapper(); + expect(TestSkDeletable.deleteCount, 0); + expect(TestBoxWrapper.resurrectCount, 0); + + object.box.delete(); + object.box.didDelete(); + expect(TestSkDeletable.deleteCount, 1); + expect(TestBoxWrapper.resurrectCount, 0); + expect(object.box.isDeletedTemporarily, true); + expect(object.box.isDeletedPermanently, false); + + object.dispose(); + expect(object.box.isDeletedPermanently, true); + }); }); } @@ -204,11 +241,20 @@ void _tests() { /// /// Can be [clone]d such that the clones share the same ref counted box. class TestBoxWrapper implements StackTraceDebugger { + static int resurrectCount = 0; + TestBoxWrapper() { if (assertionsEnabled) { _debugStackTrace = StackTrace.current; } - box = SkiaObjectBox(this, TestSkDeletable()); + box = SkiaObjectBox.resurrectable( + this, + TestSkDeletable(), + () { + resurrectCount += 1; + return TestSkDeletable(); + } + ); } TestBoxWrapper.cloneOf(this.box) { @@ -235,8 +281,15 @@ class TestBoxWrapper implements StackTraceDebugger { class TestSkDeletable implements SkDeletable { static int deleteCount = 0; + @override + bool isDeleted() => _isDeleted; + bool _isDeleted = false; + @override void delete() { + expect(_isDeleted, isFalse, + reason: 'CanvasKit does not allow deleting the same object more than once.'); + _isDeleted = true; deleteCount++; } } @@ -246,8 +299,14 @@ class TestOneShotSkiaObject extends OneShotSkiaObject implements SkDele TestOneShotSkiaObject() : super(SkPaint()); + @override + bool isDeleted() => _isDeleted; + bool _isDeleted = false; + @override void delete() { + expect(_isDeleted, isFalse, + reason: 'CanvasKit does not allow deleting the same object more than once.'); rawSkiaObject?.delete(); deleteCount++; } diff --git a/lib/web_ui/test/canvaskit/vertices_test.dart b/lib/web_ui/test/canvaskit/vertices_test.dart index 0f0263a04eb..5f340fdacd4 100644 --- a/lib/web_ui/test/canvaskit/vertices_test.dart +++ b/lib/web_ui/test/canvaskit/vertices_test.dart @@ -16,9 +16,7 @@ void main() { void testMain() { group('Vertices', () { - setUpAll(() async { - await ui.webOnlyInitializePlatform(); - }); + setUpCanvasKitTest(); test('can be constructed, drawn, and deleted', () { final CkVertices vertices = _testVertices();