Re-land "Wrap the user entrypoint function in a zone with native exception...

Re-land "Wrap the user entrypoint function in a zone with native exception callback. (#7512)" (#7551)

This reverts commit 4c135c29 and applies relevant fixes.

DEPS

@@ -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',

ci/licenses_golden/licenses_third_party

-Signature: 120dae0c7996ecd90362d836b86989e5
+Signature: 04fddafc00218848f4cab97933808ac8
 
 UNUSED LICENSES:
 

common/settings.h

@@ -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.
@@ -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;

lib/ui/hooks.dart

@@ -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)

lib/ui/ui_dart_state.cc

@@ -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)),
@@ -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 */);
 }
@@ -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

lib/ui/ui_dart_state.h

@@ -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) {
@@ -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;
@@ -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);

lib/ui/window/window.cc

@@ -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) {
@@ -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,
@@ -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;
@@ -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) {
@@ -157,20 +175,21 @@ 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) {
@@ -178,10 +197,11 @@ void Window::UpdateLocales(const std::vector<std::string>& locales) {
   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) {
@@ -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) {
@@ -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) {
@@ -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) {
@@ -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) {
@@ -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,
@@ -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) {
@@ -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) {
@@ -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},
   });
 }
 

runtime/dart_isolate.cc

@@ -22,6 +22,7 @@
 #include "third_party/tonic/dart_message_handler.h"
 #include "third_party/tonic/dart_state.h"
 #include "third_party/tonic/file_loader/file_loader.h"
+#include "third_party/tonic/logging/dart_invoke.h"
 #include "third_party/tonic/scopes/dart_api_scope.h"
 #include "third_party/tonic/scopes/dart_isolate_scope.h"
 
@@ -109,6 +110,7 @@ DartIsolate::DartIsolate(DartVM* vm,
                   advisory_script_uri,
                   advisory_script_entrypoint,
                   vm->GetSettings().log_tag,
+                  vm->GetSettings().unhandled_exception_callback,
                   vm->GetIsolateNameServer()),
       vm_(vm),
       isolate_snapshot_(std::move(isolate_snapshot)),
@@ -391,33 +393,44 @@ bool DartIsolate::MarkIsolateRunnable() {
 }
 
 FML_WARN_UNUSED_RESULT
-bool DartIsolate::Run(const std::string& entrypoint_name) {
-  TRACE_EVENT0("flutter", "DartIsolate::Run");
-  if (phase_ != Phase::Ready) {
+static bool InvokeMainEntrypoint(Dart_Handle user_entrypoint_function) {
+  if (tonic::LogIfError(user_entrypoint_function)) {
+    FML_LOG(ERROR) << "Could not resolve main entrypoint function.";
     return false;
   }
 
-  tonic::DartState::Scope scope(this);
+  Dart_Handle start_main_isolate_function =
+      tonic::DartInvokeField(Dart_LookupLibrary(tonic::ToDart("dart:isolate")),
+                             "_getStartMainIsolateFunction", {});
 
-  Dart_Handle entrypoint =
-      Dart_GetField(Dart_RootLibrary(), tonic::ToDart(entrypoint_name.c_str()));
-  if (tonic::LogIfError(entrypoint)) {
+  if (tonic::LogIfError(start_main_isolate_function)) {
+    FML_LOG(ERROR) << "Could not resolve main entrypoint trampoline.";
     return false;
   }
 
-  Dart_Handle isolate_lib = Dart_LookupLibrary(tonic::ToDart("dart:isolate"));
-  if (tonic::LogIfError(isolate_lib)) {
+  if (tonic::LogIfError(tonic::DartInvokeField(
+          Dart_LookupLibrary(tonic::ToDart("dart:ui")), "_runMainZoned",
+          {start_main_isolate_function, user_entrypoint_function}))) {
+    FML_LOG(ERROR) << "Could not invoke the main entrypoint.";
     return false;
   }
 
-  Dart_Handle isolate_args[] = {
-      entrypoint,
-      Dart_Null(),
-  };
+  return true;
+}
+
+FML_WARN_UNUSED_RESULT
+bool DartIsolate::Run(const std::string& entrypoint_name) {
+  TRACE_EVENT0("flutter", "DartIsolate::Run");
+  if (phase_ != Phase::Ready) {
+    return false;
+  }
 
-  if (tonic::LogIfError(Dart_Invoke(
-          isolate_lib, tonic::ToDart("_startMainIsolate"),
-          sizeof(isolate_args) / sizeof(isolate_args[0]), isolate_args))) {
+  tonic::DartState::Scope scope(this);
+
+  auto user_entrypoint_function =
+      Dart_GetField(Dart_RootLibrary(), tonic::ToDart(entrypoint_name.c_str()));
+
+  if (!InvokeMainEntrypoint(user_entrypoint_function)) {
     return false;
   }
 
@@ -436,30 +449,11 @@ bool DartIsolate::RunFromLibrary(const std::string& library_name,
 
   tonic::DartState::Scope scope(this);
 
-  Dart_Handle library = Dart_LookupLibrary(tonic::ToDart(library_name.c_str()));
-  if (tonic::LogIfError(library)) {
-    return false;
-  }
-
-  Dart_Handle entrypoint =
-      Dart_GetField(library, tonic::ToDart(entrypoint_name.c_str()));
-  if (tonic::LogIfError(entrypoint)) {
-    return false;
-  }
-
-  Dart_Handle isolate_lib = Dart_LookupLibrary(tonic::ToDart("dart:isolate"));
-  if (tonic::LogIfError(isolate_lib)) {
-    return false;
-  }
-
-  Dart_Handle isolate_args[] = {
-      entrypoint,
-      Dart_Null(),
-  };
+  auto user_entrypoint_function =
+      Dart_GetField(Dart_LookupLibrary(tonic::ToDart(library_name.c_str())),
+                    tonic::ToDart(entrypoint_name.c_str()));
 
-  if (tonic::LogIfError(Dart_Invoke(
-          isolate_lib, tonic::ToDart("_startMainIsolate"),
-          sizeof(isolate_args) / sizeof(isolate_args[0]), isolate_args))) {
+  if (!InvokeMainEntrypoint(user_entrypoint_function)) {
     return false;
   }
 

runtime/dart_isolate.h

@@ -88,6 +88,7 @@ class DartIsolate : public UIDartState {
   DartVM* GetDartVM() const;
 
   fml::RefPtr<DartSnapshot> GetIsolateSnapshot() const;
+
   fml::RefPtr<DartSnapshot> GetSharedSnapshot() const;
 
   std::weak_ptr<DartIsolate> GetWeakIsolatePtr();