Skip to content

Commit

Permalink
Enable loading snapshots with sound null safety enabled. (flutter#21820)
Browse files Browse the repository at this point in the history
Snapshots compiled with sound null-safety enabled require changes to the way in
which isolates are launched. Specifically, the `Dart_IsolateFlags::null_safety`
field needs to be known upfront. The value of this field can only be determined
once the kernel snapshot is available. This poses a problem in the engine
because the engine used to launch the isolate at shell initialization and only
need the kernel mappings later at isolate launch (when transitioning the root
isolate to the `DartIsolate::Phase::Running` phase). This patch delays launch of
the isolate on the UI task runner till a kernel mapping is available. The side
effects of this delay (callers no longer having access to the non-running
isolate handle) have been addressed in this patch. The DartIsolate API has also
been amended to hide the method that could return a non-running isolate to the
caller.  Instead, it has been replaced with a method that requires a valid
isolate configuration that returns a running root isolate. The isolate will be
launched by asking the isolate configuration for its null-safety
characteristics.

A side effect of enabling null-safety is that Dart APIs that work with legacy
types will now terminate the process if used with an isolate that has sound
null-safety enabled. These APIs may no longer be used in the engine. This
primarily affects the Dart Convertors in Tonic that convert certain C++ objects
into the Dart counterparts. All known Dart Converters have been updated to
convert C++ objects to non-nullable Dart types inferred using type traits of the
corresponding C++ object. The few spots in the engine that used the old Dart
APIs directly have been manually updated. To ensure that no usage of the legacy
APIs remain in the engine (as these would cause runtime process terminations),
the legacy APIs were prefixed with the `DART_LEGACY_API` macro and the macro
defined to `[[deprecated]]` in all engine translation units. While the engine
now primarily works with non-nullable Dart types, callers can still use
`Dart_TypeToNonNullableType` to acquire nullable types for use directly or with
Tonic. One use case that is not addressed with the Tonic Dart Convertors is the
creation of non-nullable lists of nullable types. This hasn’t come up so far in
the engine.

A minor related change is reworking tonic to define a single library target.
This allows the various tonic subsystems to depend on one another. Primarily,
this is used to make the Dart convertors use the logging utilities. This now
allows errors to be more descriptive as the presence of error handles is caught
(and logged) earlier.

Fixes flutter/flutter#59879
  • Loading branch information
chinmaygarde authored Oct 16, 2020
1 parent 977537b commit 5bd7260
Show file tree
Hide file tree
Showing 54 changed files with 1,096 additions and 740 deletions.
1 change: 1 addition & 0 deletions common/config.gni
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ feature_defines_list = [
"FLUTTER_RUNTIME_MODE_PROFILE=2",
"FLUTTER_RUNTIME_MODE_RELEASE=3",
"FLUTTER_RUNTIME_MODE_JIT_RELEASE=4",
"DART_LEGACY_API=[[deprecated]]",
]

if (flutter_runtime_mode == "debug") {
Expand Down
8 changes: 4 additions & 4 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <chrono>
#include <cstdint>
#include <memory>
#include <optional>
#include <string>
#include <vector>

Expand Down Expand Up @@ -50,12 +51,11 @@ using UnhandledExceptionCallback =
std::function<bool(const std::string& /* error */,
const std::string& /* stack trace */)>;

// TODO(chinmaygarde): Deprecate all the "path" struct members in favor of the
// TODO(26783): Deprecate all the "path" struct members in favor of the
// callback that generates the mapping from these paths.
// https://github.com/flutter/flutter/issues/26783
using MappingCallback = std::function<std::unique_ptr<fml::Mapping>(void)>;
using MappingsCallback =
std::function<std::vector<std::unique_ptr<const fml::Mapping>>(void)>;
using Mappings = std::vector<std::unique_ptr<const fml::Mapping>>;
using MappingsCallback = std::function<Mappings(void)>;

using FrameRasterizedCallback = std::function<void(const FrameTiming&)>;

Expand Down
2 changes: 1 addition & 1 deletion fml/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ source_set("fml") {
"platform/darwin/string_range_sanitization.mm",
]

libs += [ "Foundation.framework" ]
frameworks = [ "Foundation.framework" ]
}

if (is_android) {
Expand Down
6 changes: 5 additions & 1 deletion fml/dart/dart_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ struct DartConverter<DartConverterMapping> {
return Dart_Null();
}

auto dart_list_handle = Dart_NewListOf(Dart_CoreType_Int, val->GetSize());
auto dart_list_handle = Dart_NewListOfTypeFilled(
ToDartTypeHandle<size_t>(), // type
CreateZeroInitializedDartObject<size_t>(), // sentinel
val->GetSize() // size
);

if (Dart_IsError(dart_list_handle)) {
FML_LOG(ERROR) << "Error while attempting to allocate a list: "
Expand Down
3 changes: 2 additions & 1 deletion lib/io/dart_io.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/lib/io/dart_io.h"

Expand All @@ -23,7 +24,7 @@ void DartIO::InitForIsolate(bool may_insecurely_connect_to_all_domains,
FML_CHECK(!LogIfError(result));

Dart_Handle embedder_config_type =
Dart_GetType(io_lib, ToDart("_EmbedderConfig"), 0, nullptr);
Dart_GetNonNullableType(io_lib, ToDart("_EmbedderConfig"), 0, nullptr);
FML_CHECK(!LogIfError(embedder_config_type));

Dart_Handle allow_insecure_connections_result = Dart_SetField(
Expand Down
20 changes: 13 additions & 7 deletions lib/ui/dart_runtime_hooks.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/lib/ui/dart_runtime_hooks.h"

Expand Down Expand Up @@ -62,17 +63,19 @@ void DartRuntimeHooks::RegisterNatives(tonic::DartLibraryNatives* natives) {

static void PropagateIfError(Dart_Handle result) {
if (Dart_IsError(result)) {
FML_LOG(ERROR) << "Dart Error: " << ::Dart_GetError(result);
Dart_PropagateError(result);
}
}

static Dart_Handle GetFunction(Dart_Handle builtin_library, const char* name) {
static Dart_Handle InvokeFunction(Dart_Handle builtin_library,
const char* name) {
Dart_Handle getter_name = ToDart(name);
return Dart_Invoke(builtin_library, getter_name, 0, nullptr);
}

static void InitDartInternal(Dart_Handle builtin_library, bool is_ui_isolate) {
Dart_Handle print = GetFunction(builtin_library, "_getPrintClosure");
Dart_Handle print = InvokeFunction(builtin_library, "_getPrintClosure");

Dart_Handle internal_library = Dart_LookupLibrary(ToDart("dart:_internal"));

Expand Down Expand Up @@ -112,7 +115,7 @@ static void InitDartAsync(Dart_Handle builtin_library, bool is_ui_isolate) {
Dart_Handle schedule_microtask;
if (is_ui_isolate) {
schedule_microtask =
GetFunction(builtin_library, "_getScheduleMicrotaskClosure");
InvokeFunction(builtin_library, "_getScheduleMicrotaskClosure");
} else {
Dart_Handle isolate_lib = Dart_LookupLibrary(ToDart("dart:isolate"));
Dart_Handle method_name =
Expand All @@ -130,21 +133,24 @@ static void InitDartIO(Dart_Handle builtin_library,
const std::string& script_uri) {
Dart_Handle io_lib = Dart_LookupLibrary(ToDart("dart:io"));
Dart_Handle platform_type =
Dart_GetType(io_lib, ToDart("_Platform"), 0, nullptr);
Dart_GetNonNullableType(io_lib, ToDart("_Platform"), 0, nullptr);
if (!script_uri.empty()) {
Dart_Handle result = Dart_SetField(platform_type, ToDart("_nativeScript"),
ToDart(script_uri));
PropagateIfError(result);
}
Dart_Handle locale_closure =
GetFunction(builtin_library, "_getLocaleClosure");
// typedef _LocaleClosure = String Function();
Dart_Handle /* _LocaleClosure? */ locale_closure =
InvokeFunction(builtin_library, "_getLocaleClosure");
PropagateIfError(locale_closure);
// static String Function()? _localeClosure;
Dart_Handle result =
Dart_SetField(platform_type, ToDart("_localeClosure"), locale_closure);
PropagateIfError(result);

// Register dart:io service extensions used for network profiling.
Dart_Handle network_profiling_type =
Dart_GetType(io_lib, ToDart("_NetworkProfiling"), 0, nullptr);
Dart_GetNonNullableType(io_lib, ToDart("_NetworkProfiling"), 0, nullptr);
PropagateIfError(network_profiling_type);
result = Dart_Invoke(network_profiling_type,
ToDart("_registerServiceExtension"), 0, nullptr);
Expand Down
18 changes: 6 additions & 12 deletions lib/ui/hooks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ void _updateWindowMetrics(
_invoke(window.onMetricsChanged, window._onMetricsChangedZone);
}

typedef _LocaleClosure = String? Function();

String? _localeClosure() {
String _localeClosure() {
if (window.locale == null) {
return null;
return '';
}
return window.locale.toString();
}

typedef _LocaleClosure = String Function();

@pragma('vm:entry-point')
// ignore: unused_element
_LocaleClosure? _getLocaleClosure() => _localeClosure;
Expand Down Expand Up @@ -210,9 +210,7 @@ void _drawFrame() {
}

// ignore: always_declare_return_types, prefer_generic_function_type_aliases
typedef _UnaryFunction(Null args);
// ignore: always_declare_return_types, prefer_generic_function_type_aliases
typedef _BinaryFunction(Null args, Null message);
typedef _ListStringArgFunction(List<String> args);

@pragma('vm:entry-point')
// ignore: unused_element
Expand All @@ -221,11 +219,7 @@ void _runMainZoned(Function startMainIsolateFunction,
List<String> args) {
startMainIsolateFunction((){
runZonedGuarded<void>(() {
if (userMainFunction is _BinaryFunction) {
// This seems to be undocumented but supported by the command line VM.
// Let's do the same in case old entry-points are ported to Flutter.
(userMainFunction as dynamic)(args, '');
} else if (userMainFunction is _UnaryFunction) {
if (userMainFunction is _ListStringArgFunction) {
(userMainFunction as dynamic)(args);
} else {
userMainFunction();
Expand Down
22 changes: 10 additions & 12 deletions lib/ui/text/paragraph.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// FLUTTER_NOLINT

#include "flutter/lib/ui/text/paragraph.h"

Expand Down Expand Up @@ -132,20 +133,19 @@ tonic::Float32List Paragraph::getRectsForPlaceholders() {
}

Dart_Handle Paragraph::getPositionForOffset(double dx, double dy) {
Dart_Handle result = Dart_NewListOf(Dart_CoreType_Int, 2);
txt::Paragraph::PositionWithAffinity pos =
m_paragraph->GetGlyphPositionAtCoordinate(dx, dy);
Dart_ListSetAt(result, 0, ToDart(pos.position));
Dart_ListSetAt(result, 1, ToDart(static_cast<int>(pos.affinity)));
return result;
std::vector<size_t> result = {
pos.position, // size_t already
static_cast<size_t>(pos.affinity) // affinity (enum)
};
return tonic::DartConverter<decltype(result)>::ToDart(result);
}

Dart_Handle Paragraph::getWordBoundary(unsigned offset) {
txt::Paragraph::Range<size_t> point = m_paragraph->GetWordBoundary(offset);
Dart_Handle result = Dart_NewListOf(Dart_CoreType_Int, 2);
Dart_ListSetAt(result, 0, ToDart(point.start));
Dart_ListSetAt(result, 1, ToDart(point.end));
return result;
std::vector<size_t> result = {point.start, point.end};
return tonic::DartConverter<decltype(result)>::ToDart(result);
}

Dart_Handle Paragraph::getLineBoundary(unsigned offset) {
Expand All @@ -159,10 +159,8 @@ Dart_Handle Paragraph::getLineBoundary(unsigned offset) {
break;
}
}
Dart_Handle result = Dart_NewListOf(Dart_CoreType_Int, 2);
Dart_ListSetAt(result, 0, ToDart(line_start));
Dart_ListSetAt(result, 1, ToDart(line_end));
return result;
std::vector<int> result = {line_start, line_end};
return tonic::DartConverter<decltype(result)>::ToDart(result);
}

tonic::Float64List Paragraph::computeLineMetrics() {
Expand Down
14 changes: 0 additions & 14 deletions lib/ui/window/platform_configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,6 @@
#include "flutter/lib/ui/window/window.h"
#include "third_party/tonic/dart_persistent_value.h"

namespace tonic {
class DartLibraryNatives;

// So tonic::ToDart<std::vector<int64_t>> returns List<int> instead of
// List<dynamic>.
template <>
struct DartListFactory<int64_t> {
static Dart_Handle NewList(intptr_t length) {
return Dart_NewListOf(Dart_CoreType_Int, length);
}
};

} // namespace tonic

namespace flutter {
class FontCollection;
class PlatformMessage;
Expand Down
3 changes: 3 additions & 0 deletions runtime/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ source_set("runtime") {
"dart_vm_lifecycle.h",
"embedder_resources.cc",
"embedder_resources.h",
"isolate_configuration.cc",
"isolate_configuration.h",
"platform_data.cc",
"platform_data.h",
"ptrace_check.h",
Expand Down Expand Up @@ -112,6 +114,7 @@ if (enable_unittests) {
"dart_lifecycle_unittests.cc",
"dart_service_isolate_unittests.cc",
"dart_vm_unittests.cc",
"type_conversions_unittests.cc",
]

public_configs = [ "//flutter:export_dynamic_symbols" ]
Expand Down
Loading

0 comments on commit 5bd7260

Please sign in to comment.