Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove some of our hacks around JSPromise now that we have better APIs. #45591

Merged
merged 3 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 8 additions & 18 deletions lib/web_ui/lib/src/engine/app_bootstrap.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 '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<void> Function([JsFlutterConfiguration? params]);
Expand Down Expand Up @@ -40,33 +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: allowInterop(() => futureToPromise(() async {
autoStart: () 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: allowInterop(([JsFlutterConfiguration? configuration]) => futureToPromise(() async {
initializeEngine: ([JsFlutterConfiguration? configuration]) async {
await _initializeEngine(configuration);
return _prepareAppRunner();
}()))
}
);
}

/// 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<FlutterApp>(allowInterop((
PromiseResolver<FlutterApp> resolve,
PromiseRejecter _,
) async {
await _runApp();
// Return the App that was just started
resolve.resolve(_prepareFlutterApp());
}));
}));
return FlutterAppRunner(runApp: ([RunAppFnParameters? params]) async {
await _runApp();
return _prepareFlutterApp();
});
}

/// Represents the App that was just started, and its JS API.
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/canvaskit/image_web_codecs.dart
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ Future<ByteBuffer> 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<void>(copyPromise);

// In dart2wasm, `toDart` incurs a copy here. On JS backends, this is a
Expand Down
50 changes: 30 additions & 20 deletions lib/web_ui/lib/src/engine/js_interop/js_loader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ library js_loader;
import 'dart:js_interop';

import 'package:js/js_util.dart' as js_util;

import '../configuration.dart';
import 'js_promise.dart';
import 'package:ui/src/engine.dart';

@JS()
@staticInterop
Expand All @@ -34,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<FlutterAppRunner> 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<FlutterApp> Function();

// FlutterEngineInitializer

/// An object that allows the user to initialize the Engine of a Flutter App.
Expand All @@ -44,34 +53,34 @@ 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,
});
}

/// Typedef for the function that initializes the flutter engine.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's up to you if you want to make this typed a bit better by having a non-external function that takes in the typedefs, does the toJS conversions, and then forwards them to a (private) external factory function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I guess maybe it's better to keep a little bit of type safety here.

///
/// [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<FlutterAppRunner> 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<FlutterApp> Function();

// FlutterAppRunner

/// A class that exposes a function that runs the Flutter app,
/// and returns a promise of a FlutterAppCleaner.
@JS()
@anonymous
@staticInterop
abstract class FlutterAppRunner {
abstract class FlutterAppRunner extends JSObject {
factory FlutterAppRunner({required RunAppFn runApp,}) => FlutterAppRunner._(
runApp: ((RunAppFnParameters args) => futureToPromise(runApp(args))).toJS
);

/// Runs a flutter app
external factory FlutterAppRunner({
required RunAppFn runApp, // Returns an App
external factory FlutterAppRunner._({
required JSFunction runApp, // Returns an App
});
}

Expand All @@ -81,18 +90,19 @@ abstract class FlutterAppRunner {
@anonymous
@staticInterop
abstract class RunAppFnParameters {
external factory RunAppFnParameters();
}

/// Typedef for the function that runs the flutter app main entrypoint.
typedef RunAppFn = Promise<FlutterApp> Function([RunAppFnParameters?]);
typedef RunAppFn = Future<FlutterApp> 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();
}
41 changes: 14 additions & 27 deletions lib/web_ui/lib/src/engine/js_interop/js_promise.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,40 +11,27 @@ import 'package:js/js_util.dart' as js_util;

import '../util.dart';

@JS()
@staticInterop
class PromiseResolver<T extends Object?> {}

extension PromiseResolverExtension<T extends Object?> on PromiseResolver<T> {
void resolve(T result) => js_util.callMethod(this, 'call', <Object>[this, if (result != null) result]);
}

@JS()
@staticInterop
class PromiseRejecter {}

extension PromiseRejecterExtension on PromiseRejecter {
void reject(Object? error) => js_util.callMethod(this, 'call', <Object>[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<T extends Object?> {
/// A constructor for a JS promise
external factory Promise(PromiseExecutor<T> executor);
}
external JSAny get _promiseConstructor;

JSPromise createPromise(JSFunction executor) =>
srujzs marked this conversation as resolved.
Show resolved Hide resolved
js_util.callConstructor(
_promiseConstructor,
<Object>[executor],
);

/// The type of function that is used to create a Promise<T>
typedef PromiseExecutor<T extends Object?> = void Function(PromiseResolver<T> resolve, PromiseRejecter reject);

Promise<T> futureToPromise<T extends Object>(Future<T> future) {
return Promise<T>(js_util.allowInterop((PromiseResolver<T> resolver, PromiseRejecter rejecter) {
JSPromise futureToPromise<T extends JSAny>(Future<T> 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);
}
22 changes: 4 additions & 18 deletions lib/web_ui/lib/src/engine/safe_browser_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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();
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
14 changes: 11 additions & 3 deletions lib/web_ui/test/engine/app_bootstrap_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,17 @@ void testMain() {

final FlutterEngineInitializer engineInitializer = bootstrap.prepareEngineInitializer();

final Object appInitializer = await promiseToFuture<Object>(callMethod<Object>(engineInitializer, 'initializeEngine', <Object?>[]));
final Object maybeApp = await promiseToFuture<Object>(callMethod<Object>(appInitializer, 'runApp', <Object?>[]));

final Object appInitializer = await promiseToFuture<Object>(callMethod<Object>(
engineInitializer,
'initializeEngine',
<Object?>[]
));
expect(appInitializer, isA<FlutterAppRunner>());
final Object maybeApp = await promiseToFuture<Object>(callMethod<Object>(
appInitializer,
'runApp',
<Object?>[RunAppFnParameters()]
));
expect(maybeApp, isA<FlutterApp>());
expect(initCalled, 1, reason: 'initEngine should have been called.');
expect(runCalled, 2, reason: 'runApp should have been called.');
Expand Down
20 changes: 10 additions & 10 deletions lib/web_ui/test/engine/window_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -331,19 +332,18 @@ Future<void> testMain() async {
// The `orientation` property cannot be overridden, so this test overrides the entire `screen`.
js_util.setProperty(domWindow, 'screen', js_util.jsify(<Object?, Object?>{
'orientation': <Object?, Object?>{
'lock': allowInterop((String lockType) {
'lock': (String lockType) {
lockCalls.add(lockType);
return Promise<Object?>(allowInterop((PromiseResolver<Object?> 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,
},
}));

Expand Down