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

[web] JSConfig: Add multiViewEnabled value. #47939

Merged
merged 3 commits into from
Nov 15, 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
29 changes: 14 additions & 15 deletions lib/web_ui/lib/src/engine/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -267,17 +267,6 @@ class FlutterConfiguration {
'FLUTTER_WEB_CANVASKIT_FORCE_CPU_ONLY',
);

/// This is deprecated. The CanvasKit renderer will only ever create one
/// WebGL context, obviating the problem this configuration was meant to
/// solve originally.
@Deprecated('Setting canvasKitMaximumSurfaces has no effect')
int get canvasKitMaximumSurfaces =>
_configuration?.canvasKitMaximumSurfaces?.toInt() ?? _defaultCanvasKitMaximumSurfaces;
static const int _defaultCanvasKitMaximumSurfaces = int.fromEnvironment(
'FLUTTER_WEB_MAXIMUM_SURFACES',
defaultValue: 8,
);

/// Set this flag to `true` to cause the engine to visualize the semantics tree
/// on the screen for debugging.
///
Expand All @@ -298,6 +287,16 @@ class FlutterConfiguration {
/// to render, or `null` if the user hasn't specified anything.
DomElement? get hostElement => _configuration?.hostElement;

/// Sets Flutter Web in "multi-view" mode.
///
/// Multi-view mode allows apps to:
///
/// * Start without a `hostElement`.
/// * Add/remove views (`hostElements`) from JS while the application is running.
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

/// * ...
/// * PROFIT?
bool get multiViewEnabled => _configuration?.multiViewEnabled ?? false;

/// Returns a `nonce` to allowlist the inline styles that Flutter web needs.
///
/// See: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/nonce
Expand Down Expand Up @@ -347,16 +346,16 @@ extension JsFlutterConfigurationExtension on JsFlutterConfiguration {
external JSBoolean? get _canvasKitForceCpuOnly;
bool? get canvasKitForceCpuOnly => _canvasKitForceCpuOnly?.toDart;

@JS('canvasKitMaximumSurfaces')
external JSNumber? get _canvasKitMaximumSurfaces;
double? get canvasKitMaximumSurfaces => _canvasKitMaximumSurfaces?.toDartDouble;

@JS('debugShowSemanticsNodes')
external JSBoolean? get _debugShowSemanticsNodes;
bool? get debugShowSemanticsNodes => _debugShowSemanticsNodes?.toDart;

external DomElement? get hostElement;

@JS('multiViewEnabled')
Copy link
Member Author

Choose a reason for hiding this comment

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

We can give any "external" (JS) name we want to this property, like "multiViewEnabledExperimental" or something scary like that. Do we need that, or do we just call it "multiViewEnabled"? @mdebbar?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine to keep the name multiViewEnabled to avoid a little bit of churn down the line. Anyone who makes use of this flag ought to know that this is experimental and could break at any time.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I agree with this!

external JSBoolean? get _multiViewEnabled;
bool? get multiViewEnabled => _multiViewEnabled?.toDart;

@JS('nonce')
external JSString? get _nonce;
String? get nonce => _nonce?.toDart;
Expand Down
15 changes: 13 additions & 2 deletions lib/web_ui/test/canvaskit/initialization/stores_config_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,23 @@ void testMain() {
group('initializeEngineServices', () {
test('stores user configuration', () async {
final JsFlutterConfiguration config = JsFlutterConfiguration();
js_util.setProperty(config, 'canvasKitMaximumSurfaces', 32.0);
// `canvasKitBaseUrl` is required for the test to actually run.
js_util.setProperty(config, 'canvasKitBaseUrl', '/canvaskit/');
// A property under test, that we'll try to read later.
js_util.setProperty(config, 'nonce', 'some_nonce');
// A non-existing property to verify our js-interop doesn't crash.
js_util.setProperty(config, 'canvasKitMaximumSurfaces', 32.0);

// Remove window.flutterConfiguration (if it's there)
js_util.setProperty(domWindow, 'flutterConfiguration', null);

// TODO(web): Replace the above nullification by the following assertion
// when wasm and JS tests initialize their config the same way:
// assert(js_util.getProperty<Object?>(domWindow, 'flutterConfiguration') == null);

await initializeEngineServices(jsConfiguration: config);

expect(configuration.canvasKitMaximumSurfaces, 32);
expect(configuration.nonce, 'some_nonce');
});
});
}
114 changes: 71 additions & 43 deletions lib/web_ui/test/engine/configuration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,30 +22,30 @@ void testMain() {
test('initializes with null', () async {
final FlutterConfiguration config = FlutterConfiguration.legacy(null);

expect(config.canvasKitMaximumSurfaces, 8); // _defaultCanvasKitMaximumSurfaces
expect(config.canvasKitBaseUrl, 'canvaskit/'); // _defaultCanvasKitBaseUrl
});

test('legacy constructor initializes with a Js Object', () async {
final FlutterConfiguration config = FlutterConfiguration.legacy(
js_util.jsify(<String, Object?>{
'canvasKitMaximumSurfaces': 16,
'canvasKitBaseUrl': 'some_other_url/',
}) as JsFlutterConfiguration);

expect(config.canvasKitMaximumSurfaces, 16);
expect(config.canvasKitBaseUrl, 'some_other_url/');
});
});

group('setUserConfiguration', () {
test('throws assertion error if already initialized from JS', () async {
final FlutterConfiguration config = FlutterConfiguration.legacy(
js_util.jsify(<String, Object?>{
'canvasKitMaximumSurfaces': 12,
'canvasKitBaseUrl': 'some_other_url/',
}) as JsFlutterConfiguration);

expect(() {
config.setUserConfiguration(
js_util.jsify(<String, Object?>{
'canvasKitMaximumSurfaces': 16,
'canvasKitBaseUrl': 'yet_another_url/',
}) as JsFlutterConfiguration);
}, throwsAssertionError);
});
Expand All @@ -55,70 +55,98 @@ void testMain() {

config.setUserConfiguration(
js_util.jsify(<String, Object?>{
'canvasKitMaximumSurfaces': 16,
'canvasKitBaseUrl': 'one_more_url/',
}) as JsFlutterConfiguration);

expect(config.canvasKitMaximumSurfaces, 16);
expect(config.canvasKitBaseUrl, 'one_more_url/');
});
});

group('CanvasKit config', () {
test('default canvasKitVariant', () {
final FlutterConfiguration config = FlutterConfiguration();
test('can receive non-existing properties without crashing', () async {
final FlutterConfiguration config = FlutterConfiguration.legacy(null);

expect(config.canvasKitVariant, CanvasKitVariant.auto);
expect(() {
config.setUserConfiguration(
js_util.jsify(<String, Object?>{
'canvasKitMaximumSurfaces': 32.0,
}) as JsFlutterConfiguration);
}, returnsNormally);
});
});

test('default canvasKitVariant when it is undefined', () {
final FlutterConfiguration config = FlutterConfiguration();
config.setUserConfiguration(
// With an empty map, the canvasKitVariant is undefined in JS.
group('Default configuration values', () {
late FlutterConfiguration defaultConfig;
setUp(() {
defaultConfig = FlutterConfiguration();
defaultConfig.setUserConfiguration(
js_util.jsify(<String, Object?>{}) as JsFlutterConfiguration,
);
});

expect(config.canvasKitVariant, CanvasKitVariant.auto);
test('canvasKitVariant', () {
expect(defaultConfig.canvasKitVariant, CanvasKitVariant.auto);
});

test('validates canvasKitVariant', () {
final FlutterConfiguration config = FlutterConfiguration();
test('useColorEmoji', () {
expect(defaultConfig.useColorEmoji, isFalse);
});

config.setUserConfiguration(
js_util.jsify(<String, Object?>{'canvasKitVariant': 'foo'}) as JsFlutterConfiguration,
);
expect(() => config.canvasKitVariant, throwsArgumentError);
test('multiViewEnabled', () {
expect(defaultConfig.multiViewEnabled, isFalse);
});

config.setUserConfiguration(
js_util.jsify(<String, Object?>{'canvasKitVariant': 'auto'}) as JsFlutterConfiguration,
);
expect(config.canvasKitVariant, CanvasKitVariant.auto);
});

config.setUserConfiguration(
js_util.jsify(<String, Object?>{'canvasKitVariant': 'full'}) as JsFlutterConfiguration,
);
expect(config.canvasKitVariant, CanvasKitVariant.full);
group('setUserConfiguration (values)', () {
group('canvasKitVariant', () {
test('value undefined - defaults to "auto"', () {
final FlutterConfiguration config = FlutterConfiguration();
config.setUserConfiguration(
// With an empty map, the canvasKitVariant is undefined in JS.
js_util.jsify(<String, Object?>{}) as JsFlutterConfiguration,
);

config.setUserConfiguration(
js_util.jsify(<String, Object?>{'canvasKitVariant': 'chromium'}) as JsFlutterConfiguration,
);
expect(config.canvasKitVariant, CanvasKitVariant.chromium);
expect(config.canvasKitVariant, CanvasKitVariant.auto);
});

test('value - converts to CanvasKitVariant enum (or throw)', () {
final FlutterConfiguration config = FlutterConfiguration();

config.setUserConfiguration(
js_util.jsify(<String, Object?>{'canvasKitVariant': 'foo'}) as JsFlutterConfiguration,
);
expect(() => config.canvasKitVariant, throwsArgumentError);

config.setUserConfiguration(
js_util.jsify(<String, Object?>{'canvasKitVariant': 'auto'}) as JsFlutterConfiguration,
);
expect(config.canvasKitVariant, CanvasKitVariant.auto);

config.setUserConfiguration(
js_util.jsify(<String, Object?>{'canvasKitVariant': 'full'}) as JsFlutterConfiguration,
);
expect(config.canvasKitVariant, CanvasKitVariant.full);

config.setUserConfiguration(
js_util.jsify(<String, Object?>{'canvasKitVariant': 'chromium'}) as JsFlutterConfiguration,
);
expect(config.canvasKitVariant, CanvasKitVariant.chromium);
});
});
});

group('useColorEmoji', () {
test('defaults to false', () {
test('useColorEmoji', () {
final FlutterConfiguration config = FlutterConfiguration();
config.setUserConfiguration(
js_util.jsify(<String, Object?>{}) as JsFlutterConfiguration,
js_util.jsify(<String, Object?>{'useColorEmoji': true}) as JsFlutterConfiguration,
);
expect(config.useColorEmoji, isFalse);
expect(config.useColorEmoji, isTrue);
});

test('can be set to true', () {
test('multiViewEnabled', () {
final FlutterConfiguration config = FlutterConfiguration();
config.setUserConfiguration(
js_util.jsify(<String, Object?>{'useColorEmoji': true}) as JsFlutterConfiguration,
js_util.jsify(<String, Object?>{'multiViewEnabled': true}) as JsFlutterConfiguration,
);
expect(config.useColorEmoji, isTrue);
expect(config.multiViewEnabled, isTrue);
});
});
}