Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

take web_ui to null safety #19027

Merged
merged 9 commits into from
Jun 16, 2020
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
2 changes: 1 addition & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ task:
script:
- cd $ENGINE_PATH/src/flutter/lib/web_ui
- $ENGINE_PATH/src/out/host_debug_unopt/dart-sdk/bin/pub get
- $ENGINE_PATH/src/out/host_debug_unopt/dart-sdk/bin/dartanalyzer --fatal-warnings --fatal-hints dev/ lib/ test/ tool/
- $ENGINE_PATH/src/out/host_debug_unopt/dart-sdk/bin/dartanalyzer --enable-experiment=non-nullable --fatal-warnings --fatal-hints dev/ lib/ test/ tool/

- name: format_and_dart_test
format_script: |
Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
# uncommented, we'll delete this file and simply inherit the root options.

analyzer:
enable-experiment:
- non-nullable
strong-mode:
# TODO(uncomment) implicit-casts: false
implicit-dynamic: false
Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/build.canvaskit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ targets:
dart2js_args:
- --no-minify
- --enable-asserts
- --enable-experiment=non-nullable
- --no-sound-null-safety
- -DFLUTTER_WEB_USE_SKIA=true
generate_for:
include:
Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/build.html.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ targets:
dart2js_args:
- --no-minify
- --enable-asserts
- --enable-experiment=non-nullable
- --no-sound-null-safety
generate_for:
include:
- test/**.dart
Expand Down
17 changes: 11 additions & 6 deletions lib/web_ui/dev/integration_tests_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart = 2.6

import 'dart:io' as io;
import 'package:path/path.dart' as pathlib;
import 'package:web_driver_installer/chrome_driver_installer.dart';
Expand Down Expand Up @@ -51,7 +53,7 @@ class IntegrationTestsManager {
// LUCI installs driver from CIPD, so we skip installing it on LUCI.
await _prepareDriver();
} else {
await _verifyDriverForLUCI();
_verifyDriverForLUCI();
}
await _startDriver(_browserDriverDir.path);
// TODO(nurhan): https://github.com/flutter/flutter/issues/52987
Expand Down Expand Up @@ -109,13 +111,13 @@ class IntegrationTestsManager {
}
}

void _startDriver(String workingDirectory) async {
Future<void> _startDriver(String workingDirectory) async {
await startProcess('./chromedriver/chromedriver', ['--port=4444'],
workingDirectory: workingDirectory);
print('INFO: Driver started');
}

void _prepareDriver() async {
Future<void> _prepareDriver() async {
if (_browserDriverDir.existsSync()) {
_browserDriverDir.deleteSync(recursive: true);
}
Expand All @@ -130,7 +132,10 @@ class IntegrationTestsManager {
final String chromeDriverVersion = await queryChromeDriverVersion();
ChromeDriverInstaller chromeDriverInstaller =
ChromeDriverInstaller.withVersion(chromeDriverVersion);
await chromeDriverInstaller.install(alwaysInstall: true);
// TODO(yjbanov): remove this dynamic hack when chromeDriverInstaller.install returns Future<void>
// https://github.com/flutter/flutter/issues/59376
final dynamic installationFuture = chromeDriverInstaller.install(alwaysInstall: true) as dynamic;
await installationFuture;
io.Directory.current = temp;
}

Expand Down Expand Up @@ -172,7 +177,7 @@ class IntegrationTestsManager {
.whereType<io.File>()
.toList();

final List<String> e2eTestsToRun = List<String>();
final List<String> e2eTestsToRun = <String>[];
final List<String> blockedTests =
blockedTestsListsMap[getBlockedTestsListMapKey(_browser)] ?? <String>[];

Expand Down Expand Up @@ -265,7 +270,7 @@ class IntegrationTestsManager {
// Whether the project has the pubspec.yaml file.
bool pubSpecFound = false;
// The test directory 'test_driver'.
io.Directory testDirectory = null;
io.Directory testDirectory;

for (io.FileSystemEntity e in entities) {
// The tests should be under this directories.
Expand Down
1 change: 1 addition & 0 deletions lib/web_ui/dev/test_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ class TestCommand extends Command<bool> with ArgUtils {
'run',
'build_runner',
'build',
'--enable-experiment=non-nullable',
'test',
'-o',
forCanvasKit ? canvasKitOutputRelativePath : 'build',
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/dev/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Future<int> runProcess(
}

/// Runs [executable]. Do not follow the exit code or the output.
void startProcess(
Future<void> startProcess(
String executable,
List<String> arguments, {
String workingDirectory,
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ void registerHotRestartListener(ui.VoidCallback listener) {
/// environment in the native embedder.
// TODO(yjbanov): we should refactor the code such that the framework does not
// call this method directly.
void webOnlyInitializeEngine() {
void initializeEngine() {
if (_engineInitialized) {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/compositor/embedded_views.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ class HtmlViewEmbedder {
return;
}

final PlatformViewFactory factory =
platformViewRegistry.registeredFactories[viewType];
final ui.PlatformViewFactory factory =
ui.platformViewRegistry.registeredFactories[viewType];
if (factory == null) {
callback(codec.encodeErrorEnvelope(
code: 'unregistered_view_type',
Expand Down
5 changes: 5 additions & 0 deletions lib/web_ui/lib/src/engine/compositor/initialization.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,10 @@ js.JsObject canvasKit;
/// The Skia font collection.
SkiaFontCollection skiaFontCollection;

/// Initializes [skiaFontCollection].
void ensureSkiaFontCollectionInitialized() {

Choose a reason for hiding this comment

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

Once this file is opted in, can this become late SkiaFontCollection = skiaFontCollection? Maybe final if it doesn't get reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hopefully. Filed flutter/flutter#59371 to revisit this.

skiaFontCollection ??= SkiaFontCollection();
}

/// The scene host, where the root canvas and overlay canvases are added to.
html.Element skiaSceneHost;
91 changes: 4 additions & 87 deletions lib/web_ui/lib/src/engine/platform_views.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,90 +5,7 @@
// @dart = 2.6
part of engine;

/// A registry for factories that create platform views.
class PlatformViewRegistry {
final Map<String, PlatformViewFactory> registeredFactories =
<String, PlatformViewFactory>{};

final Map<int, html.Element> _createdViews = <int, html.Element>{};

/// Private constructor so this class can be a singleton.
PlatformViewRegistry._();

/// Register [viewTypeId] as being creating by the given [factory].
bool registerViewFactory(String viewTypeId, PlatformViewFactory factory) {
if (registeredFactories.containsKey(viewTypeId)) {
return false;
}
registeredFactories[viewTypeId] = factory;
return true;
}

/// Returns the view that has been created with the given [id], or `null` if
/// no such view exists.
html.Element getCreatedView(int id) {
return _createdViews[id];
}
}

/// A function which takes a unique [id] and creates an HTML element.
typedef PlatformViewFactory = html.Element Function(int viewId);

/// The platform view registry for this app.
final PlatformViewRegistry platformViewRegistry = PlatformViewRegistry._();

/// Handles a platform call to `flutter/platform_views`.
///
/// Used to create platform views.
void handlePlatformViewCall(
ByteData data,
ui.PlatformMessageResponseCallback callback,
) {
const MethodCodec codec = StandardMethodCodec();
final MethodCall decoded = codec.decodeMethodCall(data);

switch (decoded.method) {
case 'create':
_createPlatformView(decoded, callback);
return;
case 'dispose':
_disposePlatformView(decoded, callback);
return;
}
callback(null);
}

void _createPlatformView(
MethodCall methodCall, ui.PlatformMessageResponseCallback callback) {
final Map<dynamic, dynamic> args = methodCall.arguments;
final int id = args['id'];
final String viewType = args['viewType'];
const MethodCodec codec = StandardMethodCodec();

// TODO(het): Use 'direction', 'width', and 'height'.
if (!platformViewRegistry.registeredFactories.containsKey(viewType)) {
callback(codec.encodeErrorEnvelope(
code: 'Unregistered factory',
message: "No factory registered for viewtype '$viewType'",
));
return;
}
// TODO(het): Use creation parameters.
final html.Element element =
platformViewRegistry.registeredFactories[viewType](id);

platformViewRegistry._createdViews[id] = element;
callback(codec.encodeSuccessEnvelope(null));
}

void _disposePlatformView(
MethodCall methodCall, ui.PlatformMessageResponseCallback callback) {
final int id = methodCall.arguments;
const MethodCodec codec = StandardMethodCodec();

// Remove the root element of the view from the DOM.
platformViewRegistry._createdViews[id]?.remove();
platformViewRegistry._createdViews.remove(id);

callback(codec.encodeSuccessEnvelope(null));
}
// TODO(yjbanov): The code in this file was temporarily moved to lib/web_ui/lib/ui.dart
// during the NNBD migration so that `dart:ui` does not have to export
// `dart:_engine`. NNBD does not allow exported non-migrated libraries
// from migrated libraries.
Copy link
Contributor

Choose a reason for hiding this comment

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

@leafpetersen - is this a bug or an intended restriction?

Choose a reason for hiding this comment

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

This was intentional. We could remove the restriction, but I was aiming to have the property that opted in packages had no legacy types in their public interface, so that if you migrated relative to an opted in package you could be confident that you were done.

4 changes: 0 additions & 4 deletions lib/web_ui/lib/src/engine/plugins.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,3 @@
part of engine;

Future<void> Function(String, ByteData, ui.PlatformMessageResponseCallback) pluginMessageCallHandler;

void webOnlySetPluginHandler(Future<void> Function(String, ByteData, ui.PlatformMessageResponseCallback) handler) {
pluginMessageCallHandler = handler;
}
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/semantics/incrementable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,13 @@ class Incrementable extends RoleManager {
_element.setAttribute('aria-valuenow', surrogateTextValue);
_element.setAttribute('aria-valuetext', semanticsObject.value);

final bool canIncrease = semanticsObject.increasedValue != null;
final bool canIncrease = semanticsObject.increasedValue.isNotEmpty;
final String surrogateMaxTextValue =
canIncrease ? '${_currentSurrogateValue + 1}' : surrogateTextValue;
_element.max = surrogateMaxTextValue;
_element.setAttribute('aria-valuemax', surrogateMaxTextValue);

final bool canDecrease = semanticsObject.decreasedValue != null;
final bool canDecrease = semanticsObject.decreasedValue.isNotEmpty;
final String surrogateMinTextValue =
canDecrease ? '${_currentSurrogateValue - 1}' : surrogateTextValue;
_element.min = surrogateMinTextValue;
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/surface/canvas.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class SurfaceCanvas implements ui.Canvas {

@override
void clipRect(ui.Rect/*!*/ rect,
{ui.ClipOp clipOp/*!*/ = ui.ClipOp.intersect, bool/*!*/ doAntiAlias = true}) {
{ui.ClipOp/*!*/ clipOp = ui.ClipOp.intersect, bool/*!*/ doAntiAlias = true}) {
assert(rectIsValid(rect));
assert(clipOp != null);
assert(doAntiAlias != null);
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/surface/platform_view.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class PersistedPlatformView extends PersistedLeafSurface {
}''';
_shadowRoot.append(_styleReset);
final html.Element platformView =
platformViewRegistry.getCreatedView(viewId);
ui.platformViewRegistry.getCreatedView(viewId);
if (platformView != null) {
_shadowRoot.append(platformView);
} else {
Expand All @@ -67,7 +67,7 @@ class PersistedPlatformView extends PersistedLeafSurface {
..height = '${height}px';
// Set size of the root element created by the PlatformView.
final html.Element platformView =
platformViewRegistry.getCreatedView(viewId);
ui.platformViewRegistry.getCreatedView(viewId);
if (platformView != null) {
platformView.style
..width = '${width}px'
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ typedef Callback<T> = void Function(T result);
///
/// Return value should be null on success, and a string error message on
/// failure.
typedef Callbacker<T> = String Function(Callback<T> callback);
typedef Callbacker<T> = String/*?*/ Function(Callback<T> callback);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean to keep comment syntax here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now. This PR only migrates dart:ui. dart:_engine will follow next.


/// Converts a method that receives a value-returning callback to a method that
/// returns a Future.
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/window.dart
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ class EngineWindow extends ui.Window {
if (experimentalUseSkia) {
rasterizer.viewEmbedder.handlePlatformViewCall(data, callback);
} else {
handlePlatformViewCall(data, callback);
ui.handlePlatformViewCall(data, callback);
}
return;

Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/ui/annotations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

// TODO(dnfield): Remove unused_import ignores when https://github.com/dart-lang/sdk/issues/35164 is resolved.

// @dart = 2.6
// @dart = 2.9
part of ui;

// TODO(dnfield): Update this if/when we default this to on in the tool,
Expand Down Expand Up @@ -39,7 +39,7 @@ part of ui;
/// }
/// }
/// ```
const _KeepToString/*!*/ keepToString = _KeepToString();
const _KeepToString keepToString = _KeepToString();

class _KeepToString {
const _KeepToString();
Expand Down
Loading