From 39928d0d58faf4eaac75efb238c04d36b8086f72 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Mon, 11 Sep 2023 13:06:35 -0700 Subject: [PATCH] Revert "Remove some of our hacks around JSPromise now that we have better APIs. (#45591)" This reverts commit f192cc949269c5f8671685bbb8ed20c53cfa2118. --- lib/web_ui/lib/src/engine/app_bootstrap.dart | 26 +++++++--- .../engine/canvaskit/image_web_codecs.dart | 2 +- .../lib/src/engine/js_interop/js_loader.dart | 50 ++++++++----------- .../lib/src/engine/js_interop/js_promise.dart | 41 +++++++++------ .../lib/src/engine/safe_browser_api.dart | 22 ++++++-- .../test/engine/app_bootstrap_test.dart | 14 ++---- lib/web_ui/test/engine/window_test.dart | 20 ++++---- 7 files changed, 97 insertions(+), 78 deletions(-) diff --git a/lib/web_ui/lib/src/engine/app_bootstrap.dart b/lib/web_ui/lib/src/engine/app_bootstrap.dart index dc0bf098f4a05..f3d5a70cc3424 100644 --- a/lib/web_ui/lib/src/engine/app_bootstrap.dart +++ b/lib/web_ui/lib/src/engine/app_bootstrap.dart @@ -2,8 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:js/js_util.dart' show allowInterop; + import 'configuration.dart'; import 'js_interop/js_loader.dart'; +import 'js_interop/js_promise.dart'; /// The type of a function that initializes an engine (in Dart). typedef InitEngineFn = Future Function([JsFlutterConfiguration? params]); @@ -37,26 +40,33 @@ class AppBootstrap { // This is a convenience method that lets the programmer call "autoStart" // from JavaScript immediately after the main.dart.js has loaded. // Returns a promise that resolves to the Flutter app that was started. - autoStart: () async { + autoStart: allowInterop(() => futureToPromise(() async { await autoStart(); // Return the App that was just started return _prepareFlutterApp(); - }, + }())), // Calls [_initEngine], and returns a JS Promise that resolves to an // app runner object. - initializeEngine: ([JsFlutterConfiguration? configuration]) async { + initializeEngine: allowInterop(([JsFlutterConfiguration? configuration]) => futureToPromise(() async { await _initializeEngine(configuration); return _prepareAppRunner(); - } + }())) ); } /// Creates an appRunner that runs our encapsulated runApp function. FlutterAppRunner _prepareAppRunner() { - return FlutterAppRunner(runApp: ([RunAppFnParameters? params]) async { - await _runApp(); - return _prepareFlutterApp(); - }); + return FlutterAppRunner(runApp: allowInterop(([RunAppFnParameters? params]) { + // `params` coming from JS may be used to configure the run app method. + return Promise(allowInterop(( + PromiseResolver resolve, + PromiseRejecter _, + ) async { + await _runApp(); + // Return the App that was just started + resolve.resolve(_prepareFlutterApp()); + })); + })); } /// Represents the App that was just started, and its JS API. diff --git a/lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart b/lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart index 74a06d6cd4d6d..13563d0b52902 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart @@ -142,7 +142,7 @@ Future readVideoFramePixelsUnmodified(VideoFrame videoFrame) async { // In dart2wasm, Uint8List is not the same as a JS Uint8Array. So we // explicitly construct the JS object here. final JSUint8Array destination = createUint8ArrayFromLength(size); - final JSPromise copyPromise = videoFrame.copyTo(destination); + final JsPromise copyPromise = videoFrame.copyTo(destination); await promiseToFuture(copyPromise); // In dart2wasm, `toDart` incurs a copy here. On JS backends, this is a diff --git a/lib/web_ui/lib/src/engine/js_interop/js_loader.dart b/lib/web_ui/lib/src/engine/js_interop/js_loader.dart index 283f3e1655fa0..2e65f2aeb767c 100644 --- a/lib/web_ui/lib/src/engine/js_interop/js_loader.dart +++ b/lib/web_ui/lib/src/engine/js_interop/js_loader.dart @@ -8,7 +8,9 @@ library js_loader; import 'dart:js_interop'; import 'package:js/js_util.dart' as js_util; -import 'package:ui/src/engine.dart'; + +import '../configuration.dart'; +import 'js_promise.dart'; @JS() @staticInterop @@ -32,17 +34,6 @@ extension FlutterLoaderExtension on FlutterLoader { bool get isAutoStart => !js_util.hasProperty(this, 'didCreateEngineInitializer'); } -/// Typedef for the function that initializes the flutter engine. -/// /// -/// [JsFlutterConfiguration] comes from `../configuration.dart`. It is the same -/// object that can be used to configure flutter "inline", through the -/// (to be deprecated) `window.flutterConfiguration` object. -typedef InitializeEngineFn = Future Function([JsFlutterConfiguration?]); - -/// Typedef for the `autoStart` function that can be called straight from an engine initializer instance. -/// (Similar to [RunAppFn], but taking no specific "runApp" parameters). -typedef ImmediateRunAppFn = Future Function(); - // FlutterEngineInitializer /// An object that allows the user to initialize the Engine of a Flutter App. @@ -53,19 +44,23 @@ typedef ImmediateRunAppFn = Future Function(); @anonymous @staticInterop abstract class FlutterEngineInitializer{ - factory FlutterEngineInitializer({ + external factory FlutterEngineInitializer({ required InitializeEngineFn initializeEngine, required ImmediateRunAppFn autoStart, - }) => FlutterEngineInitializer._( - initializeEngine: (() => futureToPromise(initializeEngine())).toJS, - autoStart: (() => futureToPromise(autoStart())).toJS, - ); - external factory FlutterEngineInitializer._({ - required JSFunction initializeEngine, - required JSFunction autoStart, }); } +/// Typedef for the function that initializes the flutter engine. +/// +/// [JsFlutterConfiguration] comes from `../configuration.dart`. It is the same +/// object that can be used to configure flutter "inline", through the +/// (to be deprecated) `window.flutterConfiguration` object. +typedef InitializeEngineFn = Promise Function([JsFlutterConfiguration?]); + +/// Typedef for the `autoStart` function that can be called straight from an engine initializer instance. +/// (Similar to [RunAppFn], but taking no specific "runApp" parameters). +typedef ImmediateRunAppFn = Promise Function(); + // FlutterAppRunner /// A class that exposes a function that runs the Flutter app, @@ -73,14 +68,10 @@ abstract class FlutterEngineInitializer{ @JS() @anonymous @staticInterop -abstract class FlutterAppRunner extends JSObject { - factory FlutterAppRunner({required RunAppFn runApp,}) => FlutterAppRunner._( - runApp: ((RunAppFnParameters args) => futureToPromise(runApp(args))).toJS - ); - +abstract class FlutterAppRunner { /// Runs a flutter app - external factory FlutterAppRunner._({ - required JSFunction runApp, // Returns an App + external factory FlutterAppRunner({ + required RunAppFn runApp, // Returns an App }); } @@ -90,11 +81,10 @@ abstract class FlutterAppRunner extends JSObject { @anonymous @staticInterop abstract class RunAppFnParameters { - external factory RunAppFnParameters(); } /// Typedef for the function that runs the flutter app main entrypoint. -typedef RunAppFn = Future Function([RunAppFnParameters?]); +typedef RunAppFn = Promise Function([RunAppFnParameters?]); // FlutterApp @@ -102,7 +92,7 @@ typedef RunAppFn = Future Function([RunAppFnParameters?]); @JS() @anonymous @staticInterop -abstract class FlutterApp extends JSObject { +abstract class FlutterApp { /// Cleans a Flutter app external factory FlutterApp(); } diff --git a/lib/web_ui/lib/src/engine/js_interop/js_promise.dart b/lib/web_ui/lib/src/engine/js_interop/js_promise.dart index 6ad06b21566b4..5f7e61dd3217e 100644 --- a/lib/web_ui/lib/src/engine/js_interop/js_promise.dart +++ b/lib/web_ui/lib/src/engine/js_interop/js_promise.dart @@ -11,27 +11,40 @@ import 'package:js/js_util.dart' as js_util; import '../util.dart'; -extension CallExtension on JSFunction { - external void call(JSAny? this_, JSAny? object); +@JS() +@staticInterop +class PromiseResolver {} + +extension PromiseResolverExtension on PromiseResolver { + void resolve(T result) => js_util.callMethod(this, 'call', [this, if (result != null) result]); } -@JS('Promise') -external JSAny get _promiseConstructor; +@JS() +@staticInterop +class PromiseRejecter {} -JSPromise createPromise(JSFunction executor) => - js_util.callConstructor( - _promiseConstructor, - [executor], - ); +extension PromiseRejecterExtension on PromiseRejecter { + void reject(Object? error) => js_util.callMethod(this, 'call', [this, if (error != null) error]); +} + +/// Type-safe JS Promises +@JS('Promise') +@staticInterop +abstract class Promise { + /// A constructor for a JS promise + external factory Promise(PromiseExecutor executor); +} +/// The type of function that is used to create a Promise +typedef PromiseExecutor = void Function(PromiseResolver resolve, PromiseRejecter reject); -JSPromise futureToPromise(Future future) { - return createPromise((JSFunction resolver, JSFunction rejecter) { +Promise futureToPromise(Future future) { + return Promise(js_util.allowInterop((PromiseResolver resolver, PromiseRejecter rejecter) { future.then( - (T value) => resolver.call(null, value), + (T value) => resolver.resolve(value), onError: (Object? error) { printWarning('Rejecting promise with error: $error'); - rejecter.call(null, null); + rejecter.reject(error); }); - }.toJS); + })); } diff --git a/lib/web_ui/lib/src/engine/safe_browser_api.dart b/lib/web_ui/lib/src/engine/safe_browser_api.dart index 73ba4775ac169..a8359793fe53e 100644 --- a/lib/web_ui/lib/src/engine/safe_browser_api.dart +++ b/lib/web_ui/lib/src/engine/safe_browser_api.dart @@ -196,6 +196,20 @@ bool get _defaultBrowserSupportsImageDecoder => // enable it explicitly. bool get _isBrowserImageDecoderStable => browserEngine == BrowserEngine.blink; +/// The signature of the function passed to the constructor of JavaScript `Promise`. +typedef JsPromiseCallback = void Function(void Function(Object? value) resolve, void Function(Object? error) reject); + +/// Corresponds to JavaScript's `Promise`. +/// +/// This type doesn't need any members. Instead, it should be first converted +/// to Dart's [Future] using [promiseToFuture] then interacted with through the +/// [Future] API. +@JS('window.Promise') +@staticInterop +class JsPromise { + external factory JsPromise(JsPromiseCallback callback); +} + /// Corresponds to the browser's `ImageDecoder` type. /// /// See also: @@ -214,7 +228,7 @@ extension ImageDecoderExtension on ImageDecoder { external JSBoolean get _complete; bool get complete => _complete.toDart; - external JSPromise decode(DecodeOptions options); + external JsPromise decode(DecodeOptions options); external JSVoid close(); } @@ -288,8 +302,8 @@ extension VideoFrameExtension on VideoFrame { double allocationSize() => _allocationSize().toDartDouble; @JS('copyTo') - external JSPromise _copyTo(JSAny destination); - JSPromise copyTo(Object destination) => _copyTo(destination.toJSAnyShallow); + external JsPromise _copyTo(JSAny destination); + JsPromise copyTo(Object destination) => _copyTo(destination.toJSAnyShallow); @JS('format') external JSString? get _format; @@ -330,7 +344,7 @@ extension VideoFrameExtension on VideoFrame { class ImageTrackList {} extension ImageTrackListExtension on ImageTrackList { - external JSPromise get ready; + external JsPromise get ready; external ImageTrack? get selectedTrack; } diff --git a/lib/web_ui/test/engine/app_bootstrap_test.dart b/lib/web_ui/test/engine/app_bootstrap_test.dart index 14af5490dc9e0..609c569883bb6 100644 --- a/lib/web_ui/test/engine/app_bootstrap_test.dart +++ b/lib/web_ui/test/engine/app_bootstrap_test.dart @@ -88,17 +88,9 @@ void testMain() { final FlutterEngineInitializer engineInitializer = bootstrap.prepareEngineInitializer(); - final Object appInitializer = await promiseToFuture(callMethod( - engineInitializer, - 'initializeEngine', - [] - )); - expect(appInitializer, isA()); - final Object maybeApp = await promiseToFuture(callMethod( - appInitializer, - 'runApp', - [RunAppFnParameters()] - )); + final Object appInitializer = await promiseToFuture(callMethod(engineInitializer, 'initializeEngine', [])); + final Object maybeApp = await promiseToFuture(callMethod(appInitializer, 'runApp', [])); + expect(maybeApp, isA()); expect(initCalled, 1, reason: 'initEngine should have been called.'); expect(runCalled, 2, reason: 'runApp should have been called.'); diff --git a/lib/web_ui/test/engine/window_test.dart b/lib/web_ui/test/engine/window_test.dart index df39afb17adde..85f40e1662c3a 100644 --- a/lib/web_ui/test/engine/window_test.dart +++ b/lib/web_ui/test/engine/window_test.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:js_interop'; import 'dart:js_util' as js_util; import 'dart:typed_data'; @@ -332,18 +331,19 @@ Future testMain() async { // The `orientation` property cannot be overridden, so this test overrides the entire `screen`. js_util.setProperty(domWindow, 'screen', js_util.jsify({ 'orientation': { - 'lock': (String lockType) { + 'lock': allowInterop((String lockType) { lockCalls.add(lockType); - return futureToPromise(() async { - if (simulateError) { - throw Error(); + return Promise(allowInterop((PromiseResolver resolve, PromiseRejecter reject) { + if (!simulateError) { + resolve.resolve(null); + } else { + reject.reject('Simulating error'); } - return 0.toJS; - }()); - }.toJS, - 'unlock': () { + })); + }), + 'unlock': allowInterop(() { unlockCount += 1; - }.toJS, + }), }, }));