From bff2987bbd909ee453ec350cdef03388ac7f1beb Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Fri, 8 Sep 2023 14:15:30 -0700 Subject: [PATCH 1/3] Remove some of our hacks around JSPromise now that we have better APIs. --- lib/web_ui/lib/src/engine/app_bootstrap.dart | 29 +++++-------- .../engine/canvaskit/image_web_codecs.dart | 2 +- .../lib/src/engine/js_interop/js_loader.dart | 23 ++--------- .../lib/src/engine/js_interop/js_promise.dart | 41 +++++++------------ .../lib/src/engine/safe_browser_api.dart | 22 ++-------- lib/web_ui/test/engine/window_test.dart | 20 ++++----- 6 files changed, 43 insertions(+), 94 deletions(-) diff --git a/lib/web_ui/lib/src/engine/app_bootstrap.dart b/lib/web_ui/lib/src/engine/app_bootstrap.dart index f3d5a70cc3424..7579d138645fd 100644 --- a/lib/web_ui/lib/src/engine/app_bootstrap.dart +++ b/lib/web_ui/lib/src/engine/app_bootstrap.dart @@ -2,7 +2,7 @@ // 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 'dart:js_interop'; import 'configuration.dart'; import 'js_interop/js_loader.dart'; @@ -40,33 +40,26 @@ 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: allowInterop(() => futureToPromise(() async { + autoStart: (() => futureToPromise(() async { await autoStart(); // Return the App that was just started - return _prepareFlutterApp(); - }())), + return _prepareFlutterApp() as JSAny; + }())).toJS, // Calls [_initEngine], and returns a JS Promise that resolves to an // app runner object. - initializeEngine: allowInterop(([JsFlutterConfiguration? configuration]) => futureToPromise(() async { + initializeEngine: (([JsFlutterConfiguration? configuration]) => futureToPromise(() async { await _initializeEngine(configuration); - return _prepareAppRunner(); - }())) + return _prepareAppRunner() as JSAny; + }())).toJS ); } /// Creates an appRunner that runs our encapsulated runApp function. FlutterAppRunner _prepareAppRunner() { - 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()); - })); - })); + return FlutterAppRunner(runApp: (([RunAppFnParameters? params]) => futureToPromise(() async { + await _runApp(); + return _prepareFlutterApp() as JSAny; + }())).toJS); } /// 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 13563d0b52902..74a06d6cd4d6d 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 2e65f2aeb767c..bb24adff27c15 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 @@ -9,9 +9,6 @@ import 'dart:js_interop'; import 'package:js/js_util.dart' as js_util; -import '../configuration.dart'; -import 'js_promise.dart'; - @JS() @staticInterop class FlutterJS {} @@ -45,22 +42,11 @@ extension FlutterLoaderExtension on FlutterLoader { @staticInterop abstract class FlutterEngineInitializer{ external factory FlutterEngineInitializer({ - required InitializeEngineFn initializeEngine, - required ImmediateRunAppFn autoStart, + 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, @@ -71,7 +57,7 @@ typedef ImmediateRunAppFn = Promise Function(); abstract class FlutterAppRunner { /// Runs a flutter app external factory FlutterAppRunner({ - required RunAppFn runApp, // Returns an App + required JSFunction runApp, // Returns an App }); } @@ -83,9 +69,6 @@ abstract class FlutterAppRunner { abstract class RunAppFnParameters { } -/// Typedef for the function that runs the flutter app main entrypoint. -typedef RunAppFn = Promise Function([RunAppFnParameters?]); - // FlutterApp /// A class that exposes the public API of a running Flutter Web App running. 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 5f7e61dd3217e..6ad06b21566b4 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,40 +11,27 @@ import 'package:js/js_util.dart' as js_util; import '../util.dart'; -@JS() -@staticInterop -class PromiseResolver {} - -extension PromiseResolverExtension on PromiseResolver { - void resolve(T result) => js_util.callMethod(this, 'call', [this, if (result != null) result]); -} - -@JS() -@staticInterop -class PromiseRejecter {} - -extension PromiseRejecterExtension on PromiseRejecter { - void reject(Object? error) => js_util.callMethod(this, 'call', [this, if (error != null) error]); +extension CallExtension on JSFunction { + external void call(JSAny? this_, JSAny? object); } -/// Type-safe JS Promises @JS('Promise') -@staticInterop -abstract class Promise { - /// A constructor for a JS promise - external factory Promise(PromiseExecutor executor); -} +external JSAny get _promiseConstructor; + +JSPromise createPromise(JSFunction executor) => + js_util.callConstructor( + _promiseConstructor, + [executor], + ); -/// The type of function that is used to create a Promise -typedef PromiseExecutor = void Function(PromiseResolver resolve, PromiseRejecter reject); -Promise futureToPromise(Future future) { - return Promise(js_util.allowInterop((PromiseResolver resolver, PromiseRejecter rejecter) { +JSPromise futureToPromise(Future future) { + return createPromise((JSFunction resolver, JSFunction rejecter) { future.then( - (T value) => resolver.resolve(value), + (T value) => resolver.call(null, value), onError: (Object? error) { printWarning('Rejecting promise with error: $error'); - rejecter.reject(error); + rejecter.call(null, null); }); - })); + }.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 a8359793fe53e..73ba4775ac169 100644 --- a/lib/web_ui/lib/src/engine/safe_browser_api.dart +++ b/lib/web_ui/lib/src/engine/safe_browser_api.dart @@ -196,20 +196,6 @@ 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: @@ -228,7 +214,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(); } @@ -302,8 +288,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; @@ -344,7 +330,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/window_test.dart b/lib/web_ui/test/engine/window_test.dart index 85f40e1662c3a..df39afb17adde 100644 --- a/lib/web_ui/test/engine/window_test.dart +++ b/lib/web_ui/test/engine/window_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:js_interop'; import 'dart:js_util' as js_util; import 'dart:typed_data'; @@ -331,19 +332,18 @@ 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': allowInterop((String lockType) { + 'lock': (String lockType) { lockCalls.add(lockType); - return Promise(allowInterop((PromiseResolver resolve, PromiseRejecter reject) { - if (!simulateError) { - resolve.resolve(null); - } else { - reject.reject('Simulating error'); + return futureToPromise(() async { + if (simulateError) { + throw Error(); } - })); - }), - 'unlock': allowInterop(() { + return 0.toJS; + }()); + }.toJS, + 'unlock': () { unlockCount += 1; - }), + }.toJS, }, })); From 041f8cd9c799a5d1e3f994cad7bba6361176fca0 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Fri, 8 Sep 2023 15:04:57 -0700 Subject: [PATCH 2/3] Some fixes as per Srujan's suggestions. --- lib/web_ui/lib/src/engine/app_bootstrap.dart | 21 +++++------- .../lib/src/engine/js_interop/js_loader.dart | 34 ++++++++++++++++--- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/lib/web_ui/lib/src/engine/app_bootstrap.dart b/lib/web_ui/lib/src/engine/app_bootstrap.dart index 7579d138645fd..dc0bf098f4a05 100644 --- a/lib/web_ui/lib/src/engine/app_bootstrap.dart +++ b/lib/web_ui/lib/src/engine/app_bootstrap.dart @@ -2,11 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:js_interop'; - 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]); @@ -40,26 +37,26 @@ 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: (() => futureToPromise(() async { + autoStart: () async { await autoStart(); // Return the App that was just started - return _prepareFlutterApp() as JSAny; - }())).toJS, + return _prepareFlutterApp(); + }, // Calls [_initEngine], and returns a JS Promise that resolves to an // app runner object. - initializeEngine: (([JsFlutterConfiguration? configuration]) => futureToPromise(() async { + initializeEngine: ([JsFlutterConfiguration? configuration]) async { await _initializeEngine(configuration); - return _prepareAppRunner() as JSAny; - }())).toJS + return _prepareAppRunner(); + } ); } /// Creates an appRunner that runs our encapsulated runApp function. FlutterAppRunner _prepareAppRunner() { - return FlutterAppRunner(runApp: (([RunAppFnParameters? params]) => futureToPromise(() async { + return FlutterAppRunner(runApp: ([RunAppFnParameters? params]) async { await _runApp(); - return _prepareFlutterApp() as JSAny; - }())).toJS); + return _prepareFlutterApp(); + }); } /// Represents the App that was just started, and its JS API. 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 bb24adff27c15..b149f28c31681 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,6 +8,7 @@ library js_loader; import 'dart:js_interop'; import 'package:js/js_util.dart' as js_util; +import 'package:ui/src/engine.dart'; @JS() @staticInterop @@ -31,6 +32,17 @@ 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. @@ -41,7 +53,14 @@ extension FlutterLoaderExtension on FlutterLoader { @anonymous @staticInterop abstract class FlutterEngineInitializer{ - external factory FlutterEngineInitializer({ + 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, }); @@ -54,9 +73,13 @@ abstract class FlutterEngineInitializer{ @JS() @anonymous @staticInterop -abstract class FlutterAppRunner { +abstract class FlutterAppRunner extends JSObject { + factory FlutterAppRunner({required RunAppFn runApp,}) => FlutterAppRunner._( + runApp: ((RunAppFnParameters args) => runApp(args)).toJS + ); + /// Runs a flutter app - external factory FlutterAppRunner({ + external factory FlutterAppRunner._({ required JSFunction runApp, // Returns an App }); } @@ -69,13 +92,16 @@ abstract class FlutterAppRunner { abstract class RunAppFnParameters { } +/// Typedef for the function that runs the flutter app main entrypoint. +typedef RunAppFn = Future Function([RunAppFnParameters?]); + // FlutterApp /// A class that exposes the public API of a running Flutter Web App running. @JS() @anonymous @staticInterop -abstract class FlutterApp { +abstract class FlutterApp extends JSObject { /// Cleans a Flutter app external factory FlutterApp(); } From b474cd924afcafcc1d8921cfd889c6ca94d8ee31 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Mon, 11 Sep 2023 09:43:37 -0700 Subject: [PATCH 3/3] Fix a few issues with the app bootstrap tests. --- .../lib/src/engine/js_interop/js_loader.dart | 3 ++- lib/web_ui/test/engine/app_bootstrap_test.dart | 14 +++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) 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 b149f28c31681..283f3e1655fa0 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 @@ -75,7 +75,7 @@ abstract class FlutterEngineInitializer{ @staticInterop abstract class FlutterAppRunner extends JSObject { factory FlutterAppRunner({required RunAppFn runApp,}) => FlutterAppRunner._( - runApp: ((RunAppFnParameters args) => runApp(args)).toJS + runApp: ((RunAppFnParameters args) => futureToPromise(runApp(args))).toJS ); /// Runs a flutter app @@ -90,6 +90,7 @@ abstract class FlutterAppRunner extends JSObject { @anonymous @staticInterop abstract class RunAppFnParameters { + external factory RunAppFnParameters(); } /// Typedef for the function that runs the flutter app main entrypoint. diff --git a/lib/web_ui/test/engine/app_bootstrap_test.dart b/lib/web_ui/test/engine/app_bootstrap_test.dart index 609c569883bb6..14af5490dc9e0 100644 --- a/lib/web_ui/test/engine/app_bootstrap_test.dart +++ b/lib/web_ui/test/engine/app_bootstrap_test.dart @@ -88,9 +88,17 @@ void testMain() { final FlutterEngineInitializer engineInitializer = bootstrap.prepareEngineInitializer(); - final Object appInitializer = await promiseToFuture(callMethod(engineInitializer, 'initializeEngine', [])); - final Object maybeApp = await promiseToFuture(callMethod(appInitializer, 'runApp', [])); - + final Object appInitializer = await promiseToFuture(callMethod( + engineInitializer, + 'initializeEngine', + [] + )); + expect(appInitializer, isA()); + final Object maybeApp = await promiseToFuture(callMethod( + appInitializer, + 'runApp', + [RunAppFnParameters()] + )); expect(maybeApp, isA()); expect(initCalled, 1, reason: 'initEngine should have been called.'); expect(runCalled, 2, reason: 'runApp should have been called.');