Revert "Wrap the user entrypoint function in a zone with native excep…

…tion callback. (flutter#7512)" (flutter#7522) This reverts commit 25559ed. Reason for revert: broken in AOT mode. @pragma('vm:entry-point') placed on a function only instructs the compiler to retain the function itself, but does not tell compiler to generate and retain tear-off for this function. In this PR _runMainZoned was marked as an entry-point but C++ code was trying to tear it off and use a closure, instead of invoking it directly, which is not supported.
yunyu · Jan 17, 2019 · 4c135c2 · 4c135c2
1 parent 0a080a1
commit 4c135c2
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 113 deletions.
diff --git a/common/settings.h b/common/settings.h
@@ -20,9 +20,6 @@ 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 */)>;
 
 struct Settings {
   Settings();
@@ -83,11 +80,6 @@ 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;

diff --git a/lib/ui/hooks.dart b/lib/ui/hooks.dart
@@ -162,20 +162,6 @@ 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)

diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc
@@ -13,17 +13,15 @@ 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,
-    UnhandledExceptionCallback unhandled_exception_callback,
-    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,
+                         IsolateNameServer* isolate_name_server)
     : task_runners_(std::move(task_runners)),
       add_callback_(std::move(add_callback)),
       remove_callback_(std::move(remove_callback)),
@@ -32,7 +30,6 @@ UIDartState::UIDartState(
       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 */);
 }
@@ -136,17 +133,4 @@ 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
diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h
@@ -57,9 +57,6 @@ 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) {
@@ -80,7 +77,6 @@ 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;
@@ -106,7 +102,6 @@ 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);

diff --git a/lib/ui/window/window.cc b/lib/ui/window/window.cc
@@ -65,27 +65,6 @@ 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,
@@ -339,7 +318,6 @@ 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},
   });
 }
 

diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc
@@ -22,7 +22,6 @@
 #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"
 
@@ -110,7 +109,6 @@ 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)),
@@ -393,49 +391,33 @@ bool DartIsolate::MarkIsolateRunnable() {
 }
 
 FML_WARN_UNUSED_RESULT
-static bool InvokeMainEntrypoint(Dart_Handle user_entrypoint_function) {
-  if (tonic::LogIfError(user_entrypoint_function)) {
-    FML_LOG(ERROR) << "Could not resolve main entrypoint function.";
+bool DartIsolate::Run(const std::string& entrypoint_name) {
+  TRACE_EVENT0("flutter", "DartIsolate::Run");
+  if (phase_ != Phase::Ready) {
     return false;
   }
 
-  auto run_main_zoned_function =
-      Dart_GetField(Dart_LookupLibrary(tonic::ToDart("dart:ui")),
-                    tonic::ToDart("_runMainZoned"));
-
-  auto start_main_isolate_function =
-      Dart_GetField(Dart_LookupLibrary(tonic::ToDart("dart:isolate")),
-                    tonic::ToDart("_startMainIsolate"));
-
-  if (tonic::LogIfError(run_main_zoned_function) ||
-      tonic::LogIfError(start_main_isolate_function)) {
-    FML_LOG(ERROR) << "Could not resolve main entrypoint trampolines.";
-    return false;
-  }
+  tonic::DartState::Scope scope(this);
 
-  if (tonic::LogIfError(tonic::DartInvoke(
-          run_main_zoned_function,
-          {start_main_isolate_function, user_entrypoint_function}))) {
-    FML_LOG(ERROR) << "Could not invoke the main entrypoint.";
+  Dart_Handle entrypoint =
+      Dart_GetField(Dart_RootLibrary(), tonic::ToDart(entrypoint_name.c_str()));
+  if (tonic::LogIfError(entrypoint)) {
     return false;
   }
 
-  return true;
-}
-
-FML_WARN_UNUSED_RESULT
-bool DartIsolate::Run(const std::string& entrypoint_name) {
-  TRACE_EVENT0("flutter", "DartIsolate::Run");
-  if (phase_ != Phase::Ready) {
+  Dart_Handle isolate_lib = Dart_LookupLibrary(tonic::ToDart("dart:isolate"));
+  if (tonic::LogIfError(isolate_lib)) {
     return false;
   }
 
-  tonic::DartState::Scope scope(this);
-
-  auto user_entrypoint_function =
-      Dart_GetField(Dart_RootLibrary(), tonic::ToDart(entrypoint_name.c_str()));
+  Dart_Handle isolate_args[] = {
+      entrypoint,
+      Dart_Null(),
+  };
 
-  if (!InvokeMainEntrypoint(user_entrypoint_function)) {
+  if (tonic::LogIfError(Dart_Invoke(
+          isolate_lib, tonic::ToDart("_startMainIsolate"),
+          sizeof(isolate_args) / sizeof(isolate_args[0]), isolate_args))) {
     return false;
   }
 
@@ -454,11 +436,30 @@ bool DartIsolate::RunFromLibrary(const std::string& library_name,
 
   tonic::DartState::Scope scope(this);
 
-  auto user_entrypoint_function =
-      Dart_GetField(Dart_LookupLibrary(tonic::ToDart(library_name.c_str())),
-                    tonic::ToDart(entrypoint_name.c_str()));
+  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(),
+  };
 
-  if (!InvokeMainEntrypoint(user_entrypoint_function)) {
+  if (tonic::LogIfError(Dart_Invoke(
+          isolate_lib, tonic::ToDart("_startMainIsolate"),
+          sizeof(isolate_args) / sizeof(isolate_args[0]), isolate_args))) {
     return false;
   }
 

diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h
@@ -88,7 +88,6 @@ class DartIsolate : public UIDartState {
   DartVM* GetDartVM() const;
 
   fml::RefPtr<DartSnapshot> GetIsolateSnapshot() const;
-
   fml::RefPtr<DartSnapshot> GetSharedSnapshot() const;
 
   std::weak_ptr<DartIsolate> GetWeakIsolatePtr();