Revert "Create root isolate asynchronously (#20142)" (#20937)

This reverts commit 95f2b727.

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,
@@ -53,80 +50,48 @@ 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->io_manager_,                                    //
-                self->unref_queue_,                                   //
-                self->image_decoder_,                                 //
-                self->advisory_script_uri_,                           //
-                self->advisory_script_entrypoint_,                    //
+          vm_->GetVMData()->GetSettings(),                //
+          isolate_snapshot_,                              //
+          task_runners_,                                  //
+          std::make_unique<PlatformConfiguration>(this),  //
+          snapshot_delegate_,                             //
+          io_manager_,                                    //
+          unref_queue_,                                   //
+          image_decoder_,                                 //
+          p_advisory_script_uri,                          //
+          p_advisory_script_entrypoint,                   //
           nullptr,                                        //
-                self->isolate_create_callback_,                       //
-                self->isolate_shutdown_callback_                      //
+          isolate_create_callback_,                       //
+          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;
-          }
+  root_isolate_ = strong_root_isolate;
 
-          self->root_isolate_return_code_ = {true, code};
+  strong_root_isolate->SetReturnCodeCallback([this](uint32_t code) {
+    root_isolate_return_code_ = {true, code};
   });
 
-        if (auto* platform_configuration =
-                self->GetPlatformConfigurationIfAvailable()) {
+  if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
     tonic::DartState::Scope scope(strong_root_isolate);
     platform_configuration->DidCreateIsolate();
-          if (!self->FlushRuntimeStateToIsolate()) {
+    if (!FlushRuntimeStateToIsolate()) {
       FML_DLOG(ERROR) << "Could not setup initial isolate state.";
     }
   } else {
-          FML_DCHECK(false)
-              << "RuntimeController created without window binding.";
+    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();
-      });
 }
 
 RuntimeController::~RuntimeController() {
@@ -142,8 +107,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;
   }
@@ -269,7 +234,7 @@ bool RuntimeController::ReportTimings(std::vector<int64_t> timings) {
 }
 
 bool RuntimeController::NotifyIdle(int64_t deadline) {
-  std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
+  std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
   if (!root_isolate) {
     return false;
   }
@@ -326,7 +291,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;
 }
 
@@ -388,17 +353,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;
   }
@@ -407,20 +372,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>
 
@@ -341,7 +340,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
@@ -423,10 +422,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.
   ///
@@ -475,16 +471,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

@@ -495,10 +495,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

@@ -185,15 +185,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
     ///
     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
@@ -821,9 +812,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
   // |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

@@ -24,7 +24,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,
@@ -47,7 +46,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,
@@ -59,7 +57,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

@@ -562,6 +562,8 @@ bool Shell::Setup(std::unique_ptr<PlatformView> platform_view,
 
   is_setup_ = true;
 
+  vm_->GetServiceProtocol()->AddHandler(this, GetServiceProtocolDescription());
+
   PersistentCache::GetCacheForProcess()->AddWorkerTaskRunner(
       task_runners_.GetIOTaskRunner());
 
@@ -1131,19 +1133,6 @@ void Shell::OnPreEngineRestart() {
   latch.Wait();
 }
 
-// |Engine::Delegate|
-void Shell::OnRootIsolateCreated() {
-  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);
-        }
-      });
-}
-
 // |Engine::Delegate|
 void Shell::UpdateIsolateDescription(const std::string isolate_name,
                                      int64_t isolate_port) {
@@ -1288,15 +1277,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

@@ -513,9 +513,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;