Skip to content

Commit

Permalink
Re-land "Wrap the user entrypoint function in a zone with native exce…
Browse files Browse the repository at this point in the history
…ption callback. (flutter#7512)" (flutter#7551)

This reverts commit 4c135c2 and applies relevant fixes.
  • Loading branch information
chinmaygarde authored Jan 24, 2019
1 parent 31c7917 commit 23b7e29
Show file tree
Hide file tree
Showing 9 changed files with 159 additions and 96 deletions.
2 changes: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ deps = {
# and not have to specific specific hashes.

'src/third_party/tonic':
Var('fuchsia_git') + '/tonic' + '@' + '4634b29a24ccfc0fcfafcc8196ef30131185ad88',
Var('fuchsia_git') + '/tonic' + '@' + '02f9d8dd18dd259e3c5efe1fbe713819a730b6e0',

'src/third_party/benchmark':
Var('fuchsia_git') + '/third_party/benchmark' + '@' + '21f1eb3fe269ea43eba862bf6b699cde46587ade',
Expand Down
2 changes: 1 addition & 1 deletion ci/licenses_golden/licenses_third_party
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Signature: 120dae0c7996ecd90362d836b86989e5
Signature: 04fddafc00218848f4cab97933808ac8

UNUSED LICENSES:

Expand Down
8 changes: 8 additions & 0 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ namespace blink {
using TaskObserverAdd =
std::function<void(intptr_t /* key */, fml::closure /* callback */)>;
using TaskObserverRemove = std::function<void(intptr_t /* key */)>;
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
// callback that generates the mapping from these paths.
Expand Down Expand Up @@ -91,6 +94,11 @@ struct Settings {
// as fast as possible in returning from this callback. Long running
// operations in this callback do have the capability of introducing jank.
std::function<void(int64_t)> idle_notification_callback;
// A callback given to the embedder to react to unhandled exceptions in the
// running Flutter application. This callback is made on an internal engine
// managed thread and embedders must thread as necessary. Performing blocking
// calls in this callback will cause applications to jank.
UnhandledExceptionCallback unhandled_exception_callback;
bool enable_software_rendering = false;
bool skia_deterministic_rendering_on_cpu = false;
bool verbose_logging = false;
Expand Down
14 changes: 14 additions & 0 deletions lib/ui/hooks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,20 @@ void _drawFrame() {
_invoke(window.onDrawFrame, window._onDrawFrameZone);
}

@pragma('vm:entry-point')
// ignore: unused_element
void _runMainZoned(Function startMainIsolateFunction, Function userMainFunction) {
startMainIsolateFunction((){
runZoned<Future<void>>(() {
userMainFunction();
}, onError: (Object error, StackTrace stackTrace) {
_reportUnhandledException(error.toString(), stackTrace.toString());
});
}, null);
}

void _reportUnhandledException(String error, String stackTrace) native 'Window_reportUnhandledException';

/// Invokes [callback] inside the given [zone].
void _invoke(void callback(), Zone zone) {
if (callback == null)
Expand Down
34 changes: 25 additions & 9 deletions lib/ui/ui_dart_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@ using tonic::ToDart;

namespace blink {

UIDartState::UIDartState(TaskRunners task_runners,
TaskObserverAdd add_callback,
TaskObserverRemove remove_callback,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<IOManager> io_manager,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
std::string logger_prefix,
IsolateNameServer* isolate_name_server)
UIDartState::UIDartState(
TaskRunners task_runners,
TaskObserverAdd add_callback,
TaskObserverRemove remove_callback,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<IOManager> io_manager,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
std::string logger_prefix,
UnhandledExceptionCallback unhandled_exception_callback,
IsolateNameServer* isolate_name_server)
: task_runners_(std::move(task_runners)),
add_callback_(std::move(add_callback)),
remove_callback_(std::move(remove_callback)),
Expand All @@ -30,6 +32,7 @@ UIDartState::UIDartState(TaskRunners task_runners,
advisory_script_uri_(std::move(advisory_script_uri)),
advisory_script_entrypoint_(std::move(advisory_script_entrypoint)),
logger_prefix_(std::move(logger_prefix)),
unhandled_exception_callback_(unhandled_exception_callback),
isolate_name_server_(isolate_name_server) {
AddOrRemoveTaskObserver(true /* add */);
}
Expand Down Expand Up @@ -133,4 +136,17 @@ tonic::DartErrorHandleType UIDartState::GetLastError() {
return error;
}

void UIDartState::ReportUnhandledException(const std::string& error,
const std::string& stack_trace) {
if (unhandled_exception_callback_ &&
unhandled_exception_callback_(error, stack_trace)) {
return;
}

// Either the exception handler was not set or it could not handle the error,
// just log the exception.
FML_LOG(ERROR) << "Unhandled Exception: " << error << std::endl
<< stack_trace;
}

} // namespace blink
5 changes: 5 additions & 0 deletions lib/ui/ui_dart_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ class UIDartState : public tonic::DartState {

tonic::DartErrorHandleType GetLastError();

void ReportUnhandledException(const std::string& error,
const std::string& stack_trace);

template <class T>
static flow::SkiaGPUObject<T> CreateGPUObject(sk_sp<T> object) {
if (!object) {
Expand All @@ -77,6 +80,7 @@ class UIDartState : public tonic::DartState {
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
std::string logger_prefix,
UnhandledExceptionCallback unhandled_exception_callback,
IsolateNameServer* isolate_name_server);

~UIDartState() override;
Expand All @@ -102,6 +106,7 @@ class UIDartState : public tonic::DartState {
std::string debug_name_;
std::unique_ptr<Window> window_;
tonic::DartMicrotaskQueue microtask_queue_;
UnhandledExceptionCallback unhandled_exception_callback_;
IsolateNameServer* isolate_name_server_;

void AddOrRemoveTaskObserver(bool add);
Expand Down
117 changes: 71 additions & 46 deletions lib/ui/window/window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,13 @@
#include "third_party/tonic/logging/dart_invoke.h"
#include "third_party/tonic/typed_data/dart_byte_data.h"

using tonic::DartInvokeField;
using tonic::DartState;
using tonic::StdStringToDart;
using tonic::ToDart;

namespace blink {
namespace {

void DefaultRouteName(Dart_NativeArguments args) {
std::string routeName =
UIDartState::Current()->window()->client()->DefaultRouteName();
Dart_SetReturnValue(args, StdStringToDart(routeName));
Dart_SetReturnValue(args, tonic::StdStringToDart(routeName));
}

void ScheduleFrame(Dart_NativeArguments args) {
Expand Down Expand Up @@ -65,6 +60,27 @@ void SetIsolateDebugName(Dart_NativeArguments args) {
UIDartState::Current()->SetDebugName(name);
}

void ReportUnhandledException(Dart_NativeArguments args) {
Dart_Handle exception = nullptr;

auto error_name =
tonic::DartConverter<std::string>::FromArguments(args, 0, exception);
if (exception) {
Dart_ThrowException(exception);
return;
}

auto stack_trace =
tonic::DartConverter<std::string>::FromArguments(args, 1, exception);
if (exception) {
Dart_ThrowException(exception);
return;
}

UIDartState::Current()->ReportUnhandledException(std::move(error_name),
std::move(stack_trace));
}

Dart_Handle SendPlatformMessage(Dart_Handle window,
const std::string& name,
Dart_Handle callback,
Expand All @@ -74,7 +90,8 @@ Dart_Handle SendPlatformMessage(Dart_Handle window,
if (!dart_state->window()) {
// Must release the TypedData buffer before allocating other Dart objects.
data.Release();
return ToDart("Platform messages can only be sent from the main isolate");
return tonic::ToDart(
"Platform messages can only be sent from the main isolate");
}

fml::RefPtr<PlatformMessageResponse> response;
Expand Down Expand Up @@ -147,7 +164,8 @@ Window::Window(WindowClient* client) : client_(client) {}
Window::~Window() {}

void Window::DidCreateIsolate() {
library_.Set(DartState::Current(), Dart_LookupLibrary(ToDart("dart:ui")));
library_.Set(tonic::DartState::Current(),
Dart_LookupLibrary(tonic::ToDart("dart:ui")));
}

void Window::UpdateWindowMetrics(const ViewportMetrics& metrics) {
Expand All @@ -157,31 +175,33 @@ void Window::UpdateWindowMetrics(const ViewportMetrics& metrics) {
if (!dart_state)
return;
tonic::DartState::Scope scope(dart_state);
DartInvokeField(library_.value(), "_updateWindowMetrics",
{
ToDart(metrics.device_pixel_ratio),
ToDart(metrics.physical_width),
ToDart(metrics.physical_height),
ToDart(metrics.physical_padding_top),
ToDart(metrics.physical_padding_right),
ToDart(metrics.physical_padding_bottom),
ToDart(metrics.physical_padding_left),
ToDart(metrics.physical_view_inset_top),
ToDart(metrics.physical_view_inset_right),
ToDart(metrics.physical_view_inset_bottom),
ToDart(metrics.physical_view_inset_left),
});
tonic::LogIfError(tonic::DartInvokeField(
library_.value(), "_updateWindowMetrics",
{
tonic::ToDart(metrics.device_pixel_ratio),
tonic::ToDart(metrics.physical_width),
tonic::ToDart(metrics.physical_height),
tonic::ToDart(metrics.physical_padding_top),
tonic::ToDart(metrics.physical_padding_right),
tonic::ToDart(metrics.physical_padding_bottom),
tonic::ToDart(metrics.physical_padding_left),
tonic::ToDart(metrics.physical_view_inset_top),
tonic::ToDart(metrics.physical_view_inset_right),
tonic::ToDart(metrics.physical_view_inset_bottom),
tonic::ToDart(metrics.physical_view_inset_left),
}));
}

void Window::UpdateLocales(const std::vector<std::string>& locales) {
std::shared_ptr<tonic::DartState> dart_state = library_.dart_state().lock();
if (!dart_state)
return;
tonic::DartState::Scope scope(dart_state);
DartInvokeField(library_.value(), "_updateLocales",
{
tonic::ToDart<std::vector<std::string>>(locales),
});
tonic::LogIfError(tonic::DartInvokeField(
library_.value(), "_updateLocales",
{
tonic::ToDart<std::vector<std::string>>(locales),
}));
}

void Window::UpdateUserSettingsData(const std::string& data) {
Expand All @@ -190,10 +210,11 @@ void Window::UpdateUserSettingsData(const std::string& data) {
return;
tonic::DartState::Scope scope(dart_state);

DartInvokeField(library_.value(), "_updateUserSettingsData",
{
StdStringToDart(data),
});
tonic::LogIfError(tonic::DartInvokeField(library_.value(),
"_updateUserSettingsData",
{
tonic::StdStringToDart(data),
}));
}

void Window::UpdateSemanticsEnabled(bool enabled) {
Expand All @@ -202,8 +223,8 @@ void Window::UpdateSemanticsEnabled(bool enabled) {
return;
tonic::DartState::Scope scope(dart_state);

DartInvokeField(library_.value(), "_updateSemanticsEnabled",
{ToDart(enabled)});
tonic::LogIfError(tonic::DartInvokeField(
library_.value(), "_updateSemanticsEnabled", {tonic::ToDart(enabled)}));
}

void Window::UpdateAccessibilityFeatures(int32_t values) {
Expand All @@ -212,8 +233,9 @@ void Window::UpdateAccessibilityFeatures(int32_t values) {
return;
tonic::DartState::Scope scope(dart_state);

DartInvokeField(library_.value(), "_updateAccessibilityFeatures",
{ToDart(values)});
tonic::LogIfError(tonic::DartInvokeField(library_.value(),
"_updateAccessibilityFeatures",
{tonic::ToDart(values)}));
}

void Window::DispatchPlatformMessage(fml::RefPtr<PlatformMessage> message) {
Expand All @@ -232,9 +254,10 @@ void Window::DispatchPlatformMessage(fml::RefPtr<PlatformMessage> message) {
pending_responses_[response_id] = response;
}

DartInvokeField(
library_.value(), "_dispatchPlatformMessage",
{ToDart(message->channel()), data_handle, ToDart(response_id)});
tonic::LogIfError(
tonic::DartInvokeField(library_.value(), "_dispatchPlatformMessage",
{tonic::ToDart(message->channel()), data_handle,
tonic::ToDart(response_id)}));
}

void Window::DispatchPointerDataPacket(const PointerDataPacket& packet) {
Expand All @@ -246,8 +269,8 @@ void Window::DispatchPointerDataPacket(const PointerDataPacket& packet) {
Dart_Handle data_handle = ToByteData(packet.data());
if (Dart_IsError(data_handle))
return;
DartInvokeField(library_.value(), "_dispatchPointerDataPacket",
{data_handle});
tonic::LogIfError(tonic::DartInvokeField(
library_.value(), "_dispatchPointerDataPacket", {data_handle}));
}

void Window::DispatchSemanticsAction(int32_t id,
Expand All @@ -263,9 +286,10 @@ void Window::DispatchSemanticsAction(int32_t id,
if (Dart_IsError(args_handle))
return;

DartInvokeField(
tonic::LogIfError(tonic::DartInvokeField(
library_.value(), "_dispatchSemanticsAction",
{ToDart(id), ToDart(static_cast<int32_t>(action)), args_handle});
{tonic::ToDart(id), tonic::ToDart(static_cast<int32_t>(action)),
args_handle}));
}

void Window::BeginFrame(fml::TimePoint frameTime) {
Expand All @@ -276,14 +300,14 @@ void Window::BeginFrame(fml::TimePoint frameTime) {

int64_t microseconds = (frameTime - fml::TimePoint()).ToMicroseconds();

DartInvokeField(library_.value(), "_beginFrame",
{
Dart_NewInteger(microseconds),
});
tonic::LogIfError(tonic::DartInvokeField(library_.value(), "_beginFrame",
{
Dart_NewInteger(microseconds),
}));

UIDartState::Current()->FlushMicrotasksNow();

DartInvokeField(library_.value(), "_drawFrame", {});
tonic::LogIfError(tonic::DartInvokeField(library_.value(), "_drawFrame", {}));
}

void Window::CompletePlatformMessageEmptyResponse(int response_id) {
Expand Down Expand Up @@ -318,6 +342,7 @@ void Window::RegisterNatives(tonic::DartLibraryNatives* natives) {
{"Window_render", Render, 2, true},
{"Window_updateSemantics", UpdateSemantics, 2, true},
{"Window_setIsolateDebugName", SetIsolateDebugName, 2, true},
{"Window_reportUnhandledException", ReportUnhandledException, 2, true},
});
}

Expand Down
Loading

0 comments on commit 23b7e29

Please sign in to comment.