[web] Don't close image source too early (#57200)
A `CkImage` instance holds a reference to `ImageSource?`. When that `CkImage` gets cloned, the `ImageSource` instance becomes shared between the original `CkImage` and its new clone. Then when one of the `CkImage`s gets disposed of, it closes the shared `ImageSource` leaving other live `CkImage`s holding on to a closed `ImageSource`.
The quick solution to this is to have a ref count on the `ImageSource` to count how many `CkImage`s are referencing it. The `ImageSource` will only be closed if its ref count reaches 0.
Fixes https://github.com/flutter/flutter/issues/160199
Fixes https://github.com/flutter/flutter/issues/158093
diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart
index 812c98f..5b4edb6 100644
--- a/lib/web_ui/lib/src/engine/canvaskit/image.dart
+++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart
@@ -7,6 +7,7 @@
import 'dart:math' as math;
import 'dart:typed_data';
+import 'package:meta/meta.dart';
import 'package:ui/src/engine.dart';
import 'package:ui/ui.dart' as ui;
import 'package:ui/ui_web/src/ui_web.dart' as ui_web;
@@ -403,11 +404,13 @@
CkImage(SkImage skImage, {this.imageSource}) {
box = CountedRef<CkImage, SkImage>(skImage, this, 'SkImage');
_init();
+ imageSource?.refCount++;
}
CkImage.cloneOf(this.box, {this.imageSource}) {
_init();
box.ref(this);
+ imageSource?.refCount++;
}
void _init() {
@@ -454,6 +457,8 @@
ui.Image.onDispose?.call(this);
_disposed = true;
box.unref(this);
+
+ imageSource?.refCount--;
imageSource?.close();
}
@@ -645,7 +650,26 @@
DomCanvasImageSource get canvasImageSource;
int get width;
int get height;
- void close();
+
+ /// The number of references to this image source.
+ ///
+ /// Calling [close] is a no-op if [refCount] is greater than 0.
+ ///
+ /// Only when [refCount] is 0 will the [close] method actually close the
+ /// image source.
+ int refCount = 0;
+
+ @visibleForTesting
+ bool debugIsClosed = false;
+
+ void close() {
+ if (refCount == 0) {
+ _doClose();
+ debugIsClosed = true;
+ }
+ }
+
+ void _doClose();
}
class VideoFrameImageSource extends ImageSource {
@@ -654,7 +678,7 @@
final VideoFrame videoFrame;
@override
- void close() {
+ void _doClose() {
// Do nothing. Skia will close the VideoFrame when the SkImage is disposed.
}
@@ -674,7 +698,7 @@
final DomHTMLImageElement imageElement;
@override
- void close() {
+ void _doClose() {
// There's no way to immediately close the <img> element. Just let the
// browser garbage collect it.
}
@@ -695,7 +719,7 @@
final DomImageBitmap imageBitmap;
@override
- void close() {
+ void _doClose() {
imageBitmap.close();
}
diff --git a/lib/web_ui/lib/src/engine/dom.dart b/lib/web_ui/lib/src/engine/dom.dart
index 5fb335f..caf4888 100644
--- a/lib/web_ui/lib/src/engine/dom.dart
+++ b/lib/web_ui/lib/src/engine/dom.dart
@@ -169,6 +169,12 @@
/// The Trusted Types API (when available).
/// See: https://developer.mozilla.org/en-US/docs/Web/API/Trusted_Types_API
external DomTrustedTypePolicyFactory? get trustedTypes;
+
+ @JS('createImageBitmap')
+ external JSPromise<JSAny?> _createImageBitmap(DomImageData source);
+ Future<DomImageBitmap> createImageBitmap(DomImageData source) {
+ return js_util.promiseToFuture<DomImageBitmap>(_createImageBitmap(source));
+ }
}
typedef DomRequestAnimationFrameCallback = void Function(JSNumber highResTime);
diff --git a/lib/web_ui/test/canvaskit/image_test.dart b/lib/web_ui/test/canvaskit/image_test.dart
index c23ae19..9a30849 100644
--- a/lib/web_ui/test/canvaskit/image_test.dart
+++ b/lib/web_ui/test/canvaskit/image_test.dart
@@ -7,12 +7,11 @@
import 'package:test/bootstrap/browser.dart';
import 'package:test/test.dart';
-import 'package:ui/src/engine/canvaskit/image.dart';
-import 'package:ui/src/engine/image_decoder.dart';
-import 'package:ui/src/engine/util.dart';
+import 'package:ui/src/engine.dart';
import 'package:ui/ui.dart' as ui;
import 'common.dart';
+import 'test_data.dart';
void main() {
internalBootstrapBrowserTest(() => testMain);
@@ -131,6 +130,31 @@
expect(activeImages.length, 0);
});
+
+ test('CkImage does not close image source too early', () async {
+ final ImageSource imageSource = ImageBitmapImageSource(
+ await domWindow.createImageBitmap(createBlankDomImageData(4, 4)),
+ );
+
+ final SkImage skImage1 = canvasKit.MakeAnimatedImageFromEncoded(k4x4PngImage)!.makeImageAtCurrentFrame();
+ final CkImage image1 = CkImage(skImage1, imageSource: imageSource);
+
+ final SkImage skImage2 = canvasKit.MakeAnimatedImageFromEncoded(k4x4PngImage)!.makeImageAtCurrentFrame();
+ final CkImage image2 = CkImage(skImage2, imageSource: imageSource);
+
+ final CkImage image3 = image1.clone();
+
+ expect(imageSource.debugIsClosed, isFalse);
+
+ image1.dispose();
+ expect(imageSource.debugIsClosed, isFalse);
+
+ image2.dispose();
+ expect(imageSource.debugIsClosed, isFalse);
+
+ image3.dispose();
+ expect(imageSource.debugIsClosed, isTrue);
+ });
}
Future<ui.Image> _createImage() => _createPicture().toImage(10, 10);
