From ad9cbe1fa7bed11116b516a0d18f47fb2f98be35 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Fri, 26 May 2023 10:13:23 -0700 Subject: [PATCH] Don't use a factory constructor on the finalization registry. --- .../src/engine/canvaskit/native_memory.dart | 2 +- lib/web_ui/lib/src/engine/dom.dart | 19 +++++++++++++------ .../src/engine/skwasm/skwasm_impl/memory.dart | 2 +- .../test/canvaskit/canvaskit_api_test.dart | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart b/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart index bf78968562ecc..73a5946a965b5 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart @@ -20,7 +20,7 @@ import 'package:ui/src/engine.dart'; /// 4. GC decides to perform a GC cycle and collects CkPaint. /// 5. The finalizer function is called with the SkPaint as the sole argument. /// 6. We call `delete` on SkPaint. -DomFinalizationRegistry _finalizationRegistry = DomFinalizationRegistry( +DomFinalizationRegistry _finalizationRegistry = createDomFinalizationRegistry( (UniqueRef uniq) { uniq.collect(); }.toJS diff --git a/lib/web_ui/lib/src/engine/dom.dart b/lib/web_ui/lib/src/engine/dom.dart index 45dc726999479..82aeb6ea7df3b 100644 --- a/lib/web_ui/lib/src/engine/dom.dart +++ b/lib/web_ui/lib/src/engine/dom.dart @@ -3438,9 +3438,19 @@ extension DomTextDecoderExtension on DomTextDecoder { @JS('window.FinalizationRegistry') @staticInterop -class DomFinalizationRegistry { - external factory DomFinalizationRegistry(JSFunction cleanup); -} +class DomFinalizationRegistry {} + +@JS('window.FinalizationRegistry') +external JSAny? get _finalizationRegistryConstructor; + +// Note: We don't use a factory constructor here because there is an issue in +// dart2js that causes a crash in the Google3 build if we do use a factory +// constructor. See b/284478971 +DomFinalizationRegistry createDomFinalizationRegistry(JSFunction cleanup) => + js_util.callConstructor( + _finalizationRegistryConstructor!.toObjectShallow, + [cleanup] + ); extension DomFinalizationRegistryExtension on DomFinalizationRegistry { @JS('register') @@ -3461,9 +3471,6 @@ extension DomFinalizationRegistryExtension on DomFinalizationRegistry { void unregister(Object token) => _unregister(token.toJSAnyShallow); } -@JS('window.FinalizationRegistry') -external JSAny? get _finalizationRegistryConstructor; - /// Whether the current browser supports `FinalizationRegistry`. bool browserSupportsFinalizationRegistry = _finalizationRegistryConstructor != null; diff --git a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/memory.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/memory.dart index 4b0f04cc3b148..2497f83490ff0 100644 --- a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/memory.dart +++ b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/memory.dart @@ -28,7 +28,7 @@ typedef DisposeFunction = void Function(Pointer); class SkwasmFinalizationRegistry { SkwasmFinalizationRegistry(this.dispose) - : registry = DomFinalizationRegistry(((JSNumber address) => + : registry = createDomFinalizationRegistry(((JSNumber address) => dispose(Pointer.fromAddress(address.toDart.toInt())) ).toJS); diff --git a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart index ed918d46148db..220f78dc4bd19 100644 --- a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart +++ b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart @@ -1907,7 +1907,7 @@ void _paragraphTests() { // 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 DomFinalizationRegistry registry = DomFinalizationRegistry((String arg) {}.toJS); + final DomFinalizationRegistry registry = createDomFinalizationRegistry((String arg) {}.toJS); registry.register(Object(), Object()); }); }