[web] Don't close image source too early (flutter/engine#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
This commit is contained in:
parent
26aec9b3ec
commit
262b472592
@ -7,6 +7,7 @@ import 'dart:js_interop';
|
||||
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 @@ class CkImage implements ui.Image, StackTraceDebugger {
|
||||
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 @@ class CkImage implements ui.Image, StackTraceDebugger {
|
||||
ui.Image.onDispose?.call(this);
|
||||
_disposed = true;
|
||||
box.unref(this);
|
||||
|
||||
imageSource?.refCount--;
|
||||
imageSource?.close();
|
||||
}
|
||||
|
||||
@ -645,7 +650,26 @@ sealed class ImageSource {
|
||||
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 @@ class VideoFrameImageSource extends ImageSource {
|
||||
final VideoFrame videoFrame;
|
||||
|
||||
@override
|
||||
void close() {
|
||||
void _doClose() {
|
||||
// Do nothing. Skia will close the VideoFrame when the SkImage is disposed.
|
||||
}
|
||||
|
||||
@ -674,7 +698,7 @@ class ImageElementImageSource extends ImageSource {
|
||||
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 @@ class ImageBitmapImageSource extends ImageSource {
|
||||
final DomImageBitmap imageBitmap;
|
||||
|
||||
@override
|
||||
void close() {
|
||||
void _doClose() {
|
||||
imageBitmap.close();
|
||||
}
|
||||
|
||||
|
@ -169,6 +169,12 @@ extension DomWindowExtension on DomWindow {
|
||||
/// 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);
|
||||
|
@ -7,12 +7,11 @@ import 'dart:typed_data';
|
||||
|
||||
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 @@ void testMain() {
|
||||
|
||||
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);
|
||||
|
Loading…
x
Reference in New Issue
Block a user