Revert "Reland "Create root isolate asynchronously (#20142)" (#21747)"

This reverts commit 190fd8e.

Author: Jonah Williams

runtime/runtime_controller.cc

@@ -18,10 +18,7 @@ namespace flutter {
 
 RuntimeController::RuntimeController(RuntimeDelegate& client,
                                      TaskRunners p_task_runners)
-    : client_(client),
-      vm_(nullptr),
-      task_runners_(p_task_runners),
-      weak_factory_(this) {}
+    : client_(client), vm_(nullptr), task_runners_(p_task_runners) {}
 
 RuntimeController::RuntimeController(
     RuntimeDelegate& p_client,
@@ -55,81 +52,49 @@ RuntimeController::RuntimeController(
       platform_data_(std::move(p_platform_data)),
       isolate_create_callback_(p_isolate_create_callback),
       isolate_shutdown_callback_(p_isolate_shutdown_callback),
-      persistent_isolate_data_(std::move(p_persistent_isolate_data)),
-      weak_factory_(this) {
-  // Create the root isolate as soon as the runtime controller is initialized,
-  // but not using a synchronous way to avoid blocking the platform thread a
-  // long time as it is waiting while creating `Shell` on that platform thread.
+      persistent_isolate_data_(std::move(p_persistent_isolate_data)) {
+  // Create the root isolate as soon as the runtime controller is initialized.
   // It will be run at a later point when the engine provides a run
   // configuration and then runs the isolate.
-  create_and_config_root_isolate_ =
-      std::async(std::launch::deferred, [self = weak_factory_.GetWeakPtr()]() {
-        if (!self) {
-          return;
-        }
-
-        auto strong_root_isolate =
-            DartIsolate::CreateRootIsolate(
-                self->vm_->GetVMData()->GetSettings(),                //
-                self->isolate_snapshot_,                              //
-                self->task_runners_,                                  //
-                std::make_unique<PlatformConfiguration>(self.get()),  //
-                self->snapshot_delegate_,                             //
-                self->hint_freed_delegate_,                           //
-                self->io_manager_,                                    //
-                self->unref_queue_,                                   //
-                self->image_decoder_,                                 //
-                self->advisory_script_uri_,                           //
-                self->advisory_script_entrypoint_,                    //
-                nullptr,                                              //
-                self->isolate_create_callback_,                       //
-                self->isolate_shutdown_callback_                      //
-                )
-                .lock();
-
-        FML_CHECK(strong_root_isolate) << "Could not create root isolate.";
-
-        // The root isolate ivar is weak.
-        self->root_isolate_ = strong_root_isolate;
-
-        strong_root_isolate->SetReturnCodeCallback([self](uint32_t code) {
-          if (!self) {
-            return;
-          }
-
-          self->root_isolate_return_code_ = {true, code};
-        });
-
-        if (auto* platform_configuration =
-                self->GetPlatformConfigurationIfAvailable()) {
-          tonic::DartState::Scope scope(strong_root_isolate);
-          platform_configuration->DidCreateIsolate();
-          if (!self->FlushRuntimeStateToIsolate()) {
-            FML_DLOG(ERROR) << "Could not setup initial isolate state.";
-          }
-        } else {
-          FML_DCHECK(false)
-              << "RuntimeController created without window binding.";
-        }
-
-        FML_DCHECK(Dart_CurrentIsolate() == nullptr);
-
-        self->client_.OnRootIsolateCreated();
-        return;
-      });
-
-  // We're still trying to create the root isolate as soon as possible here on
-  // the UI thread although it's deferred a little bit by
-  // std::async(std::launch::deferred, ...). So the callers of `GetRootIsolate`
-  // should get a quick return after this UI thread task.
-  task_runners_.GetUITaskRunner()->PostTask(
-      [self = weak_factory_.GetWeakPtr()]() {
-        if (!self) {
-          return;
-        }
-
-        self->GetRootIsolate();
-      });
+  auto strong_root_isolate =
+      DartIsolate::CreateRootIsolate(
+          vm_->GetVMData()->GetSettings(),                //
+          isolate_snapshot_,                              //
+          task_runners_,                                  //
+          std::make_unique<PlatformConfiguration>(this),  //
+          snapshot_delegate_,                             //
+          hint_freed_delegate_,                           //
+          io_manager_,                                    //
+          unref_queue_,                                   //
+          image_decoder_,                                 //
+          p_advisory_script_uri,                          //
+          p_advisory_script_entrypoint,                   //
+          nullptr,                                        //
+          isolate_create_callback_,                       //
+          isolate_shutdown_callback_                      //
+          )
+          .lock();
+
+  FML_CHECK(strong_root_isolate) << "Could not create root isolate.";
+
+  // The root isolate ivar is weak.
+  root_isolate_ = strong_root_isolate;
+
+  strong_root_isolate->SetReturnCodeCallback([this](uint32_t code) {
+    root_isolate_return_code_ = {true, code};
+  });
+
+  if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
+    tonic::DartState::Scope scope(strong_root_isolate);
+    platform_configuration->DidCreateIsolate();
+    if (!FlushRuntimeStateToIsolate()) {
+      FML_DLOG(ERROR) << "Could not setup initial isolate state.";
+    }
+  } else {
+    FML_DCHECK(false) << "RuntimeController created without window binding.";
+  }
+
+  FML_DCHECK(Dart_CurrentIsolate() == nullptr);
 }
 
 RuntimeController::~RuntimeController() {
@@ -145,8 +110,8 @@ RuntimeController::~RuntimeController() {
   }
 }
 
-bool RuntimeController::IsRootIsolateRunning() {
-  std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
+bool RuntimeController::IsRootIsolateRunning() const {
+  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
   if (root_isolate) {
     return root_isolate->GetPhase() == DartIsolate::Phase::Running;
   }
@@ -273,7 +238,7 @@ bool RuntimeController::ReportTimings(std::vector<int64_t> timings) {
 }
 
 bool RuntimeController::NotifyIdle(int64_t deadline, size_t freed_hint) {
-  std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
+  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
   if (!root_isolate) {
     return false;
   }
@@ -333,7 +298,7 @@ bool RuntimeController::DispatchSemanticsAction(int32_t id,
 
 PlatformConfiguration*
 RuntimeController::GetPlatformConfigurationIfAvailable() {
-  std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
+  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
   return root_isolate ? root_isolate->platform_configuration() : nullptr;
 }
 
@@ -395,17 +360,17 @@ RuntimeController::ComputePlatformResolvedLocale(
 }
 
 Dart_Port RuntimeController::GetMainPort() {
-  std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
+  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
   return root_isolate ? root_isolate->main_port() : ILLEGAL_PORT;
 }
 
 std::string RuntimeController::GetIsolateName() {
-  std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
+  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
   return root_isolate ? root_isolate->debug_name() : "";
 }
 
 bool RuntimeController::HasLivePorts() {
-  std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
+  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
   if (!root_isolate) {
     return false;
   }
@@ -414,20 +379,11 @@ bool RuntimeController::HasLivePorts() {
 }
 
 tonic::DartErrorHandleType RuntimeController::GetLastError() {
-  std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
+  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
   return root_isolate ? root_isolate->GetLastError() : tonic::kNoError;
 }
 
 std::weak_ptr<DartIsolate> RuntimeController::GetRootIsolate() {
-  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
-  if (root_isolate) {
-    return root_isolate_;
-  }
-
-  // Root isolate is not yet created, get it and do some configuration.
-  FML_DCHECK(create_and_config_root_isolate_.valid());
-  create_and_config_root_isolate_.get();
-
   return root_isolate_;
 }
 

runtime/runtime_controller.h

@@ -5,7 +5,6 @@
 #ifndef FLUTTER_RUNTIME_RUNTIME_CONTROLLER_H_
 #define FLUTTER_RUNTIME_RUNTIME_CONTROLLER_H_
 
-#include <future>
 #include <memory>
 #include <vector>
 
@@ -349,7 +348,7 @@ class RuntimeController : public PlatformConfigurationClient {
   ///
   /// @return     True if root isolate running, False otherwise.
   ///
-  virtual bool IsRootIsolateRunning();
+  virtual bool IsRootIsolateRunning() const;
 
   //----------------------------------------------------------------------------
   /// @brief      Dispatch the specified platform message to running root
@@ -431,10 +430,7 @@ class RuntimeController : public PlatformConfigurationClient {
   /// @brief      Get a weak pointer to the root Dart isolate. This isolate may
   ///             only be locked on the UI task runner. Callers use this
   ///             accessor to transition to the root isolate to the running
-  ///             phase. Note that it might take times if the isolate is not yet
-  ///             created, which should be done in a subsequence task after
-  ///             constructing `RuntimeController`, or it should get a quick
-  ///             return otherwise.
+  ///             phase.
   ///
   /// @return     The root isolate reference.
   ///
@@ -484,16 +480,11 @@ class RuntimeController : public PlatformConfigurationClient {
   std::string advisory_script_entrypoint_;
   std::function<void(int64_t)> idle_notification_callback_;
   PlatformData platform_data_;
-  std::future<void> create_and_config_root_isolate_;
-  // Note that `root_isolate_` is created asynchronously from the constructor of
-  // `RuntimeController`, be careful to use it directly while it might have not
-  // been created yet. Call `GetRootIsolate()` instead which guarantees that.
   std::weak_ptr<DartIsolate> root_isolate_;
   std::pair<bool, uint32_t> root_isolate_return_code_ = {false, 0};
   const fml::closure isolate_create_callback_;
   const fml::closure isolate_shutdown_callback_;
   std::shared_ptr<const fml::Mapping> persistent_isolate_data_;
-  fml::WeakPtrFactory<RuntimeController> weak_factory_;
 
   PlatformConfiguration* GetPlatformConfigurationIfAvailable();
 

runtime/runtime_delegate.h

@@ -32,8 +32,6 @@ class RuntimeDelegate {
 
   virtual FontCollection& GetFontCollection() = 0;
 
-  virtual void OnRootIsolateCreated() = 0;
-
   virtual void UpdateIsolateDescription(const std::string isolate_name,
                                         int64_t isolate_port) = 0;
 

shell/common/engine.cc

@@ -501,10 +501,6 @@ void Engine::HandlePlatformMessage(fml::RefPtr<PlatformMessage> message) {
   }
 }
 
-void Engine::OnRootIsolateCreated() {
-  delegate_.OnRootIsolateCreated();
-}
-
 void Engine::UpdateIsolateDescription(const std::string isolate_name,
                                       int64_t isolate_port) {
   delegate_.UpdateIsolateDescription(isolate_name, isolate_port);

shell/common/engine.h

@@ -189,15 +189,6 @@ class Engine final : public RuntimeDelegate,
     ///
     virtual void OnPreEngineRestart() = 0;
 
-    //--------------------------------------------------------------------------
-    /// @brief      Notifies the shell that the root isolate is created.
-    ///             Currently, this information is to add to the service
-    ///             protocol list of available root isolates running in the VM
-    ///             and their names so that the appropriate isolate can be
-    ///             selected in the tools for debugging and instrumentation.
-    ///
-    virtual void OnRootIsolateCreated() = 0;
-
     //--------------------------------------------------------------------------
     /// @brief      Notifies the shell of the name of the root isolate and its
     ///             port when that isolate is launched, restarted (in the
@@ -808,9 +799,6 @@ class Engine final : public RuntimeDelegate,
   // |RuntimeDelegate|
   void HandlePlatformMessage(fml::RefPtr<PlatformMessage> message) override;
 
-  // |RuntimeDelegate|
-  void OnRootIsolateCreated() override;
-
   // |RuntimeDelegate|
   void UpdateIsolateDescription(const std::string isolate_name,
                                 int64_t isolate_port) override;

shell/common/engine_unittests.cc

@@ -27,7 +27,6 @@ class MockDelegate : public Engine::Delegate {
   MOCK_METHOD1(OnEngineHandlePlatformMessage,
                void(fml::RefPtr<PlatformMessage>));
   MOCK_METHOD0(OnPreEngineRestart, void());
-  MOCK_METHOD0(OnRootIsolateCreated, void());
   MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
   MOCK_METHOD1(SetNeedsReportTimings, void(bool));
   MOCK_METHOD1(ComputePlatformResolvedLocale,
@@ -50,7 +49,6 @@ class MockRuntimeDelegate : public RuntimeDelegate {
                void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates));
   MOCK_METHOD1(HandlePlatformMessage, void(fml::RefPtr<PlatformMessage>));
   MOCK_METHOD0(GetFontCollection, FontCollection&());
-  MOCK_METHOD0(OnRootIsolateCreated, void());
   MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
   MOCK_METHOD1(SetNeedsReportTimings, void(bool));
   MOCK_METHOD1(ComputePlatformResolvedLocale,
@@ -62,7 +60,7 @@ class MockRuntimeController : public RuntimeController {
  public:
   MockRuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners)
       : RuntimeController(client, p_task_runners) {}
-  MOCK_METHOD0(IsRootIsolateRunning, bool());
+  MOCK_CONST_METHOD0(IsRootIsolateRunning, bool());
   MOCK_METHOD1(DispatchPlatformMessage, bool(fml::RefPtr<PlatformMessage>));
 };
 

shell/common/shell.cc

@@ -564,6 +564,8 @@ bool Shell::Setup(std::unique_ptr<PlatformView> platform_view,
 
   is_setup_ = true;
 
+  vm_->GetServiceProtocol()->AddHandler(this, GetServiceProtocolDescription());
+
   PersistentCache::GetCacheForProcess()->AddWorkerTaskRunner(
       task_runners_.GetIOTaskRunner());
 
@@ -1148,23 +1150,6 @@ void Shell::OnPreEngineRestart() {
   latch.Wait();
 }
 
-// |Engine::Delegate|
-void Shell::OnRootIsolateCreated() {
-  if (is_added_to_service_protocol_) {
-    return;
-  }
-  auto description = GetServiceProtocolDescription();
-  fml::TaskRunner::RunNowOrPostTask(
-      task_runners_.GetPlatformTaskRunner(),
-      [self = weak_factory_.GetWeakPtr(),
-       description = std::move(description)]() {
-        if (self) {
-          self->vm_->GetServiceProtocol()->AddHandler(self.get(), description);
-        }
-      });
-  is_added_to_service_protocol_ = true;
-}
-
 // |Engine::Delegate|
 void Shell::UpdateIsolateDescription(const std::string isolate_name,
                                      int64_t isolate_port) {
@@ -1310,15 +1295,9 @@ bool Shell::HandleServiceProtocolMessage(
 // |ServiceProtocol::Handler|
 ServiceProtocol::Handler::Description Shell::GetServiceProtocolDescription()
     const {
-  FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread());
-
-  if (!weak_engine_) {
-    return ServiceProtocol::Handler::Description();
-  }
-
   return {
-      weak_engine_->GetUIIsolateMainPort(),
-      weak_engine_->GetUIIsolateName(),
+      engine_->GetUIIsolateMainPort(),
+      engine_->GetUIIsolateName(),
   };
 }
 

shell/common/shell.h

@@ -411,7 +411,6 @@ class Shell final : public PlatformView::Delegate,
                      >
       service_protocol_handlers_;
   bool is_setup_ = false;
-  bool is_added_to_service_protocol_ = false;
   uint64_t next_pointer_flow_id_ = 0;
 
   bool first_frame_rasterized_ = false;
@@ -538,9 +537,6 @@ class Shell final : public PlatformView::Delegate,
   // |Engine::Delegate|
   void OnPreEngineRestart() override;
 
-  // |Engine::Delegate|
-  void OnRootIsolateCreated() override;
-
   // |Engine::Delegate|
   void UpdateIsolateDescription(const std::string isolate_name,
                                 int64_t isolate_port) override;