Implement Image.clone for CanvasKit (#21555)

Author: dnfield

lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart

@@ -683,6 +683,8 @@ class SkAnimatedImage {
   ///
   /// This object is no longer usable after calling this method.
   external void delete();
+  external bool isAliasOf(SkAnimatedImage other);
+  external bool isDeleted();
 }
 
 @JS()
@@ -698,6 +700,8 @@ class SkImage {
   );
   external Uint8List readPixels(SkImageInfo imageInfo, int srcX, int srcY);
   external SkData encodeToData();
+  external bool isAliasOf(SkImage other);
+  external bool isDeleted();
 }
 
 @JS()

lib/web_ui/lib/src/engine/canvaskit/image.dart

@@ -43,8 +43,15 @@ class CkAnimatedImage implements ui.Image {
   // being garbage-collected, or by an explicit call to [delete].
   late final SkiaObjectBox box;
 
-  CkAnimatedImage(this._skAnimatedImage) {
-    box = SkiaObjectBox(this, _skAnimatedImage as SkDeletable);
+  CkAnimatedImage(SkAnimatedImage skAnimatedImage) : this._(skAnimatedImage, null);
+
+  CkAnimatedImage._(this._skAnimatedImage, SkiaObjectBox? boxToClone) {
+    if (boxToClone != null) {
+      assert(boxToClone.skObject == _skAnimatedImage);
+      box = boxToClone.clone(this);
+    } else {
+      box = SkiaObjectBox(this, _skAnimatedImage as SkDeletable);
+    }
   }
 
   @override
@@ -53,13 +60,17 @@ class CkAnimatedImage implements ui.Image {
   }
 
   @override
-  ui.Image clone() => this;
+  ui.Image clone() => CkAnimatedImage._(_skAnimatedImage, box);
 
   @override
-  bool isCloneOf(ui.Image other) => other == this;
+  bool isCloneOf(ui.Image other) {
+    return other is CkAnimatedImage
+        && other._skAnimatedImage.isAliasOf(_skAnimatedImage);
+  }
 
   @override
-  List<StackTrace>? debugGetOpenHandleStackTraces() => null;
+  List<StackTrace>? debugGetOpenHandleStackTraces() => box.debugGetStackTraces();
+
   int get frameCount => _skAnimatedImage.getFrameCount();
 
   /// Decodes the next frame and returns the frame duration.
@@ -116,8 +127,15 @@ class CkImage implements ui.Image {
   // being garbage-collected, or by an explicit call to [delete].
   late final SkiaObjectBox box;
 
-  CkImage(this.skImage) {
-    box = SkiaObjectBox(this, skImage as SkDeletable);
+  CkImage(SkImage skImage) : this._(skImage, null);
+
+  CkImage._(this.skImage, SkiaObjectBox? boxToClone) {
+    if (boxToClone != null) {
+      assert(boxToClone.skObject == skImage);
+      box = boxToClone.clone(this);
+    } else {
+      box = SkiaObjectBox(this, skImage as SkDeletable);
+    }
   }
 
   @override
@@ -126,13 +144,16 @@ class CkImage implements ui.Image {
   }
 
   @override
-  ui.Image clone() => this;
+  ui.Image clone() => CkImage._(skImage, box);
 
   @override
-  bool isCloneOf(ui.Image other) => other == this;
+  bool isCloneOf(ui.Image other) {
+    return other is CkImage
+        && other.skImage.isAliasOf(skImage);
+  }
 
   @override
-  List<StackTrace>? debugGetOpenHandleStackTraces() => null;
+  List<StackTrace>? debugGetOpenHandleStackTraces() => box.debugGetStackTraces();
 
   @override
   int get width => skImage.width();

lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart

@@ -239,23 +239,46 @@ abstract class OneShotSkiaObject<T extends Object> extends SkiaObject<T> {
   }
 }
 
-/// Manages the lifecycle of a Skia object owned by a wrapper object.
+/// Uses reference counting to manage the lifecycle of a Skia object owned by a
+/// wrapper object.
 ///
-/// When the wrapper is garbage collected, deletes the corresponding
-/// [skObject] (only in browsers that support weak references).
+/// When the wrapper is garbage collected, decrements the refcount (only in
+/// browsers that support weak references).
 ///
-/// The [delete] method can be used to eagerly delete the [skObject]
-/// before the wrapper is garbage collected.
+/// The [delete] method can be used to eagerly decrement the refcount before the
+/// wrapper is garbage collected.
 ///
 /// The [delete] method may be called any number of times. The box
 /// will only delete the object once.
 class SkiaObjectBox {
-  SkiaObjectBox(Object wrapper, this.skObject) {
+  SkiaObjectBox(Object wrapper, SkDeletable skObject)
+      : this._(wrapper, skObject, <SkiaObjectBox>{});
+
+  SkiaObjectBox._(Object wrapper, this.skObject, this._refs) {
+    if (assertionsEnabled) {
+      _debugStackTrace = StackTrace.current;
+    }
+    _refs.add(this);
     if (browserSupportsFinalizationRegistry) {
       boxRegistry.register(wrapper, this);
     }
   }
 
+  /// Reference handles to the same underlying [skObject].
+  final Set<SkiaObjectBox> _refs;
+
+  late final StackTrace? _debugStackTrace;
+  /// If asserts are enabled, the [StackTrace]s representing when a reference
+  /// was created.
+  List<StackTrace>? debugGetStackTraces() {
+    if (assertionsEnabled) {
+      return _refs
+          .map<StackTrace>((SkiaObjectBox box) => box._debugStackTrace!)
+          .toList();
+    }
+    return null;
+  }
+
   /// The Skia object whose lifecycle is being managed.
   final SkDeletable skObject;
 
@@ -269,16 +292,33 @@ class SkiaObjectBox {
     box.delete();
   }));
 
-  /// Deletes the [skObject].
+  /// Returns a clone of this object, which increases its reference count.
+  ///
+  /// Clones must be [dispose]d when finished.
+  SkiaObjectBox clone(Object wrapper) {
+    assert(!_isDeleted, 'Cannot clone from a deleted handle.');
+    assert(_refs.isNotEmpty);
+    return SkiaObjectBox._(wrapper, skObject, _refs);
+  }
+
+  /// 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 delete() {
     if (_isDeleted) {
+      assert(!_refs.contains(this));
       return;
     }
+    final bool removed = _refs.remove(this);
+    assert(removed);
     _isDeleted = true;
-    _skObjectDeleteQueue.add(skObject);
-    _skObjectCollector ??= _scheduleSkObjectCollection();
+    if (_refs.isEmpty) {
+      _skObjectDeleteQueue.add(skObject);
+      _skObjectCollector ??= _scheduleSkObjectCollection();
+    }
   }
 }
 

lib/web_ui/test/canvaskit/image_test.dart

@@ -38,6 +38,27 @@ void testMain() {
       expect(image.box.isDeleted, true);
     });
 
+    test('CkAnimatedImage can be cloned and explicitly disposed of', () async {
+      final SkAnimatedImage skAnimatedImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage);
+      final CkAnimatedImage image = CkAnimatedImage(skAnimatedImage);
+      final CkAnimatedImage imageClone = image.clone();
+
+      expect(image.isCloneOf(imageClone), true);
+      expect(image.box.isDeleted, false);
+      await Future<void>.delayed(Duration.zero);
+      expect(skAnimatedImage.isDeleted(), false);
+      image.dispose();
+      expect(image.box.isDeleted, true);
+      expect(imageClone.box.isDeleted, false);
+      await Future<void>.delayed(Duration.zero);
+      expect(skAnimatedImage.isDeleted(), false);
+      imageClone.dispose();
+      expect(image.box.isDeleted, true);
+      expect(imageClone.box.isDeleted, true);
+      await Future<void>.delayed(Duration.zero);
+      expect(skAnimatedImage.isDeleted(), true);
+    });
+
     test('CkImage toString', () {
       final SkImage skImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage).getCurrentFrame();
       final CkImage image = CkImage(skImage);
@@ -54,6 +75,27 @@ void testMain() {
       image.dispose();
       expect(image.box.isDeleted, true);
     });
+
+    test('CkImage can be explicitly disposed of when cloned', () async {
+      final SkImage skImage = canvasKit.MakeAnimatedImageFromEncoded(kTransparentImage).getCurrentFrame();
+      final CkImage image = CkImage(skImage);
+      final CkImage imageClone = image.clone();
+
+      expect(image.isCloneOf(imageClone), true);
+      expect(image.box.isDeleted, false);
+      await Future<void>.delayed(Duration.zero);
+      expect(skImage.isDeleted(), false);
+      image.dispose();
+      expect(image.box.isDeleted, true);
+      expect(imageClone.box.isDeleted, false);
+      await Future<void>.delayed(Duration.zero);
+      expect(skImage.isDeleted(), false);
+      imageClone.dispose();
+      expect(image.box.isDeleted, true);
+      expect(imageClone.box.isDeleted, true);
+      await Future<void>.delayed(Duration.zero);
+      expect(skImage.isDeleted(), true);
+    });
   // TODO: https://github.com/flutter/flutter/issues/60040
   }, skip: isIosSafari);
 }

lib/web_ui/test/canvaskit/skia_objects_cache_test.dart

@@ -12,6 +12,7 @@ import 'package:ui/ui.dart' as ui;
 import 'package:ui/src/engine.dart';
 
 import 'common.dart';
+import '../matchers.dart';
 
 void main() {
   internalBootstrapBrowserTest(() => testMain);
@@ -153,9 +154,49 @@ void _tests() {
       expect(SkiaObjects.oneShotCache.debugContains(object2), isFalse);
     });
   });
+
+
+  group(SkiaObjectBox, () {
+    test('Records stack traces and respects refcounts', () async {
+      TestOneShotSkiaObject.deleteCount = 0;
+      final TestOneShotSkiaObject skObject = TestOneShotSkiaObject();
+      final Object wrapper = Object();
+      final SkiaObjectBox box = SkiaObjectBox(wrapper, skObject);
+
+      expect(box.debugGetStackTraces().length, 1);
+
+      final SkiaObjectBox clone = box.clone(wrapper);
+      expect(clone, isNot(same(box)));
+      expect(clone.debugGetStackTraces().length, 2);
+      expect(box.debugGetStackTraces().length, 2);
+
+      box.delete();
+
+      expect(() => box.clone(wrapper), throwsAssertionError);
+
+      expect(box.isDeleted, true);
+
+      // Let any timers elapse.
+      await Future<void>.delayed(Duration.zero);
+      expect(TestOneShotSkiaObject.deleteCount, 0);
+
+      expect(clone.debugGetStackTraces().length, 1);
+      expect(box.debugGetStackTraces().length, 1);
+
+      clone.delete();
+      expect(() => clone.clone(wrapper), throwsAssertionError);
+
+      // Let any timers elapse.
+      await Future<void>.delayed(Duration.zero);
+      expect(TestOneShotSkiaObject.deleteCount, 1);
+
+      expect(clone.debugGetStackTraces().length, 0);
+      expect(box.debugGetStackTraces().length, 0);
+    });
+  });
 }
 
-class TestOneShotSkiaObject extends OneShotSkiaObject<SkPaint> {
+class TestOneShotSkiaObject extends OneShotSkiaObject<SkPaint> implements SkDeletable {
   static int deleteCount = 0;
 
   TestOneShotSkiaObject() : super(SkPaint());