Simplify Libassistant startup (and fix crashes)

Currently starting Libassistant happens in 2 steps:
   1. (On background thread) ServiceController::Start() creates the
      |AssistantManager|.
   2. (On main thread) ServiceControllerProxy::FinishAssistantStart()
      sets some extra fields on |AssistantManager| and calls
      AssistantManager::Start().

This causes a race condition if Libassistant is immediately stopped,
as that happens on the background thread and can collide with step 2
above.

To prevent this from happening, this CL collapses both starting steps in
a single step that runs on the background thread. We do this by
registering a callback which is called by the background thread in step
1 above.

This callback is temporary but necessary as some parts of the
initialization code still use objects that have not been migrated to the
Libassistant mojom service yet.

Bug: 1157177, b/171748795, 1128032
Test: chromeos_unittests "Assistant*:ServiceController*"
Change-Id: Ifab9705839ad5d3e5c9f2b7f7fba4c336b9f7c9d
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2594090
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838277}

chromeos/services/assistant/BUILD.gn

@@ -124,7 +124,6 @@ component("lib") {
       "//chromeos/resources",
       "//chromeos/services/assistant/proxy",
       "//chromeos/services/assistant/public/cpp/migration",
-      "//chromeos/services/assistant/public/cpp/migration",
       "//chromeos/services/libassistant",
       "//chromeos/services/network_config/public/mojom",
       "//chromeos/strings",
@@ -235,6 +234,10 @@ static_library("test_support") {
     "//testing/gmock",
     "//testing/gtest",
   ]
+
+  if (enable_cros_libassistant) {
+    deps += [ "//chromeos/services/assistant/public/cpp/migration" ]
+  }
 }
 
 buildflag_header("buildflags") {

chromeos/services/assistant/assistant_manager_service_impl_unittest.cc

@@ -196,6 +196,13 @@ class FakeLibassistantServiceHost : public LibassistantServiceHost {
   }
   void Stop() override { service_->Unbind(); }
 
+  void SetInitializeCallback(
+      base::OnceCallback<void(assistant_client::AssistantManager*,
+                              assistant_client::AssistantManagerInternal*)>
+          callback) override {
+    service_->service_controller().SetInitializeCallback(std::move(callback));
+  }
+
  private:
   FakeLibassistantService* service_;
 };

chromeos/services/assistant/libassistant_service_host_impl.cc

@@ -32,5 +32,13 @@ void LibassistantServiceHostImpl::Stop() {
   libassistant_service_ = nullptr;
 }
 
+void LibassistantServiceHostImpl::SetInitializeCallback(
+    base::OnceCallback<void(assistant_client::AssistantManager*,
+                            assistant_client::AssistantManagerInternal*)>
+        callback) {
+  DCHECK_NE(libassistant_service_, nullptr);
+  libassistant_service_->SetInitializeCallback(std::move(callback));
+}
+
 }  // namespace assistant
 }  // namespace chromeos

chromeos/services/assistant/libassistant_service_host_impl.h

@@ -13,6 +13,12 @@ namespace assistant_client {
 class PlatformApi;
 }  // namespace assistant_client
 
+namespace chromeos {
+namespace libassistant {
+class LibassistantService;
+}  // namespace libassistant
+}  // namespace chromeos
+
 namespace chromeos {
 namespace assistant {
 
@@ -30,6 +36,10 @@ class LibassistantServiceHostImpl : public LibassistantServiceHost {
   void Launch(
       mojo::PendingReceiver<LibassistantServiceMojom> receiver) override;
   void Stop() override;
+  void SetInitializeCallback(
+      base::OnceCallback<void(assistant_client::AssistantManager*,
+                              assistant_client::AssistantManagerInternal*)>)
+      override;
 
  private:
   // Owned by |AssistantManagerServiceImpl| which also owns |this|.
@@ -37,7 +47,8 @@ class LibassistantServiceHostImpl : public LibassistantServiceHost {
   // Owned by |AssistantManagerServiceImpl| which also owns |this|.
   AssistantManagerServiceDelegate* const delegate_;
 
-  std::unique_ptr<LibassistantServiceMojom> libassistant_service_;
+  std::unique_ptr<chromeos::libassistant::LibassistantService>
+      libassistant_service_;
 };
 
 }  // namespace assistant

chromeos/services/assistant/proxy/assistant_proxy.cc

@@ -28,8 +28,8 @@ void AssistantProxy::Initialize(LibassistantServiceHost* host) {
   libassistant_service_host_ = host;
   LaunchLibassistantService();
 
-  service_controller_proxy_ = std::make_unique<ServiceControllerProxy>(
-      background_task_runner(), BindServiceController());
+  service_controller_proxy_ =
+      std::make_unique<ServiceControllerProxy>(host, BindServiceController());
 }
 
 void AssistantProxy::LaunchLibassistantService() {

chromeos/services/assistant/proxy/libassistant_service_host.h

@@ -9,6 +9,11 @@
 
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 
+namespace assistant_client {
+class AssistantManager;
+class AssistantManagerInternal;
+}  // namespace assistant_client
+
 namespace chromeos {
 namespace libassistant {
 namespace mojom {
@@ -36,9 +41,18 @@ class LibassistantServiceHost {
   // |Stop| is called.
   virtual void Launch(
       mojo::PendingReceiver<LibassistantServiceMojom> receiver) = 0;
-  // Stop the mojom service.
 
+  // Stop the mojom service.
   virtual void Stop() = 0;
+
+  // Set a callback to initialize |AssistantManager| and
+  // |AssistantManagerInternal|. This callback will be invoked before
+  // AssistantManager::Start() is called. This is temporary until we've migrated
+  // all initialization code to the mojom service.
+  virtual void SetInitializeCallback(
+      base::OnceCallback<
+          void(assistant_client::AssistantManager*,
+               assistant_client::AssistantManagerInternal*)>) = 0;
 };
 
 }  // namespace assistant

chromeos/services/assistant/proxy/service_controller_proxy.cc

@@ -11,6 +11,7 @@
 #include "chromeos/assistant/internal/cros_display_connection.h"
 #include "chromeos/assistant/internal/internal_util.h"
 #include "chromeos/constants/chromeos_features.h"
+#include "chromeos/services/assistant/proxy/libassistant_service_host.h"
 #include "chromeos/services/assistant/public/cpp/features.h"
 #include "chromeos/services/assistant/public/cpp/migration/assistant_manager_service_delegate.h"
 #include "chromeos/services/assistant/public/cpp/migration/libassistant_v1_api.h"
@@ -36,6 +37,25 @@ constexpr char kServersideDogfoodExperimentId[] = "20347368";
 constexpr char kServersideOpenAppExperimentId[] = "39651593";
 constexpr char kServersideResponseProcessingV2ExperimentId[] = "1793869";
 
+struct StartArguments {
+  StartArguments() = default;
+  StartArguments(StartArguments&&) = default;
+  StartArguments& operator=(StartArguments&&) = default;
+  ~StartArguments() = default;
+
+  assistant_client::ActionModule* action_module;
+  assistant_client::FuchsiaApiDelegate* fuchsia_api_delegate;
+  assistant_client::AssistantManagerDelegate* assistant_manager_delegate;
+  assistant_client::ConversationStateListener* conversation_state_listener;
+  assistant_client::DeviceStateListener* device_state_listener;
+  CrosDisplayConnection* display_connection;
+  std::string libassistant_config;
+  std::string locale;
+  std::string locale_override;
+  bool spoken_feedback_enabled;
+  ServiceControllerProxy::AuthTokens auth_tokens;
+};
+
 void FillServerExperimentIds(std::vector<std::string>* server_experiment_ids) {
   if (base::FeatureList::IsEnabled(kChromeOSAssistantDogfood)) {
     server_experiment_ids->emplace_back(kServersideDogfoodExperimentId);
@@ -58,17 +78,59 @@ void SetServerExperiments(
   }
 }
 
+void SetInternalOptions(
+    assistant_client::AssistantManagerInternal* assistant_manager_internal,
+    const std::string& locale,
+    bool spoken_feedback_enabled) {
+  auto* internal_options =
+      assistant_manager_internal->CreateDefaultInternalOptions();
+  SetAssistantOptions(internal_options, locale, spoken_feedback_enabled);
+
+  internal_options->SetClientControlEnabled(
+      assistant::features::IsRoutinesEnabled());
+
+  if (!features::IsVoiceMatchDisabled())
+    internal_options->EnableRequireVoiceMatchVerification();
+
+  assistant_manager_internal->SetOptions(*internal_options, [](bool success) {
+    DVLOG(2) << "set options: " << success;
+  });
+}
+
+// TODO(b/171748795): This should all be migrated to the mojom service, which
+// should be responsible for the complete creation of the Libassistant
+// objects.
+// Note: this method will be called from the mojom (background) thread.
+void InitializeAssistantManager(
+    StartArguments arguments,
+    assistant_client::AssistantManager* assistant_manager,
+    assistant_client::AssistantManagerInternal* assistant_manager_internal) {
+  SetInternalOptions(assistant_manager_internal, arguments.locale,
+                     arguments.spoken_feedback_enabled);
+  assistant_manager_internal->SetDisplayConnection(
+      arguments.display_connection);
+  assistant_manager_internal->SetLocaleOverride(arguments.locale_override);
+  assistant_manager_internal->RegisterActionModule(arguments.action_module);
+  assistant_manager_internal->SetAssistantManagerDelegate(
+      arguments.assistant_manager_delegate);
+  assistant_manager_internal->GetFuchsiaApiHelperOrDie()->SetFuchsiaApiDelegate(
+      arguments.fuchsia_api_delegate);
+  assistant_manager->AddConversationStateListener(
+      arguments.conversation_state_listener);
+  assistant_manager->AddDeviceStateListener(arguments.device_state_listener);
+  SetServerExperiments(assistant_manager_internal);
+  assistant_manager->SetAuthTokens(arguments.auth_tokens);
+}
+
 }  // namespace
 
 ServiceControllerProxy::ServiceControllerProxy(
-    scoped_refptr<base::SingleThreadTaskRunner> background_task_runner,
+    LibassistantServiceHost* host,
     mojo::PendingRemote<chromeos::libassistant::mojom::ServiceController>
         client)
-    : background_task_runner_(std::move(background_task_runner)),
+    : host_(host),
       service_controller_remote_(std::move(client)),
       state_observer_receiver_(this) {
-  DCHECK(background_task_runner_);
-
   service_controller_remote_->AddAndFireStateObserver(
       state_observer_receiver_.BindNewPipeAndPassRemote());
 }
@@ -92,24 +154,31 @@ void ServiceControllerProxy::Start(
   DCHECK_EQ(state_, State::kStopped);
   state_ = State::kStarting;
 
+  pending_display_connection_ = std::make_unique<CrosDisplayConnection>(
+      event_observer, /*feedback_ui_enabled=*/true,
+      assistant::features::IsMediaSessionIntegrationEnabled());
+
   // The mojom service will create the |AssistantManager|.
   service_controller_remote_->Start(libassistant_config);
 
-  // We need to finalize (and start) the |AssistantManager| once it's created,
-  // so we have to store all the required arguments for that.
+  // We need to initialize the |AssistantManager| once it's created and before
+  // it's started, so we register a callback to do just that.
   StartArguments arguments;
   arguments.action_module = action_module;
   arguments.fuchsia_api_delegate = fuchsia_api_delegate;
   arguments.assistant_manager_delegate = assistant_manager_delegate;
   arguments.conversation_state_listener = conversation_state_listener;
   arguments.device_state_listener = device_state_listener;
-  arguments.event_observer = event_observer;
+  arguments.display_connection = pending_display_connection_.get();
   arguments.locale = locale;
   arguments.locale_override = locale_override;
   arguments.spoken_feedback_enabled = spoken_feedback_enabled;
   arguments.auth_tokens = auth_tokens;
-  arguments.done_callback = std::move(done_callback);
-  pending_start_argument_ = std::move(arguments);
+
+  host_->SetInitializeCallback(
+      base::BindOnce(InitializeAssistantManager, std::move(arguments)));
+
+  on_start_done_callback_ = std::move(done_callback);
 }
 
 void ServiceControllerProxy::Stop() {
@@ -126,21 +195,8 @@ void ServiceControllerProxy::Stop() {
 void ServiceControllerProxy::UpdateInternalOptions(
     const std::string& locale,
     bool spoken_feedback_enabled) {
-  // NOTE: this method is called on multiple threads, it needs to be
-  // thread-safe.
-  auto* internal_options =
-      assistant_manager_internal()->CreateDefaultInternalOptions();
-  SetAssistantOptions(internal_options, locale, spoken_feedback_enabled);
-
-  internal_options->SetClientControlEnabled(
-      assistant::features::IsRoutinesEnabled());
-
-  if (!features::IsVoiceMatchDisabled())
-    internal_options->EnableRequireVoiceMatchVerification();
-
-  assistant_manager_internal()->SetOptions(*internal_options, [](bool success) {
-    DVLOG(2) << "set options: " << success;
-  });
+  SetInternalOptions(assistant_manager_internal(), locale,
+                     spoken_feedback_enabled);
 }
 
 void ServiceControllerProxy::SetAuthTokens(const AuthTokens& tokens) {
@@ -181,45 +237,13 @@ ServiceControllerProxy::assistant_manager_internal() {
 }
 
 void ServiceControllerProxy::FinishCreatingAssistant() {
-  // TODO(b/171748795): This should all be migrated to the mojom service, which
-  // should be responsible for the complete creation of the Libassistant
-  // objects.
-  DCHECK(pending_start_argument_.has_value());
-
-  auto arguments = std::move(pending_start_argument_.value());
-
-  display_connection_ = std::make_unique<CrosDisplayConnection>(
-      arguments.event_observer, /*feedback_ui_enabled=*/true,
-      assistant::features::IsMediaSessionIntegrationEnabled());
-
-  UpdateInternalOptions(arguments.locale, arguments.spoken_feedback_enabled);
-
-  assistant_manager_internal()->SetDisplayConnection(display_connection());
-  assistant_manager_internal()->SetLocaleOverride(arguments.locale_override);
-  assistant_manager_internal()->RegisterActionModule(arguments.action_module);
-  assistant_manager_internal()->SetAssistantManagerDelegate(
-      arguments.assistant_manager_delegate);
-  assistant_manager_internal()
-      ->GetFuchsiaApiHelperOrDie()
-      ->SetFuchsiaApiDelegate(arguments.fuchsia_api_delegate);
-  assistant_manager()->AddConversationStateListener(
-      arguments.conversation_state_listener);
-  assistant_manager()->AddDeviceStateListener(arguments.device_state_listener);
-  SetServerExperiments(assistant_manager_internal());
-  SetAuthTokens(arguments.auth_tokens);
-
-  assistant_manager()->Start();
+  DCHECK(on_start_done_callback_.has_value());
+  DCHECK_NE(pending_display_connection_, nullptr);
 
   state_ = State::kStarted;
-  std::move(arguments.done_callback).Run();
+  display_connection_ = std::move(pending_display_connection_);
+  std::move(on_start_done_callback_.value()).Run();
 }
 
-ServiceControllerProxy::StartArguments::StartArguments() = default;
-ServiceControllerProxy::StartArguments::StartArguments(StartArguments&&) =
-    default;
-ServiceControllerProxy::StartArguments&
-ServiceControllerProxy::StartArguments::operator=(StartArguments&&) = default;
-ServiceControllerProxy::StartArguments::~StartArguments() = default;
-
 }  // namespace assistant
 }  // namespace chromeos

chromeos/services/assistant/proxy/service_controller_proxy.h

@@ -11,9 +11,7 @@
 #include <vector>
 
 #include "base/check.h"
-#include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/single_thread_task_runner.h"
 #include "chromeos/services/libassistant/public/mojom/service_controller.mojom.h"
 #include "mojo/public/cpp/bindings/remote.h"
 
@@ -34,6 +32,7 @@ namespace assistant {
 
 class AssistantEventObserver;
 class CrosDisplayConnection;
+class LibassistantServiceHost;
 
 // Component managing the lifecycle of Libassistant,
 // exposing methods to start/stop and configure Libassistant.
@@ -43,7 +42,7 @@ class ServiceControllerProxy : private libassistant::mojom::StateObserver {
   using AuthTokens = std::vector<std::pair<std::string, std::string>>;
 
   ServiceControllerProxy(
-      scoped_refptr<base::SingleThreadTaskRunner> background_task_runner,
+      LibassistantServiceHost* host,
       mojo::PendingRemote<chromeos::libassistant::mojom::ServiceController>
           client);
 
@@ -52,8 +51,6 @@ class ServiceControllerProxy : private libassistant::mojom::StateObserver {
   ~ServiceControllerProxy() override;
 
   // Can not be invoked before Start() has finished.
-  // Both LibAssistant and Chrome threads may access |display_connection|.
-  // |display_connection| is thread safe.
   CrosDisplayConnection* display_connection() {
     DCHECK(display_connection_);
     return display_connection_.get();
@@ -112,25 +109,6 @@ class ServiceControllerProxy : private libassistant::mojom::StateObserver {
   // Can not be invoked before Start() has finished.
   assistant_client::AssistantManagerInternal* assistant_manager_internal();
 
-  struct StartArguments {
-    StartArguments();
-    StartArguments(StartArguments&&);
-    StartArguments& operator=(StartArguments&&);
-    ~StartArguments();
-    assistant_client::ActionModule* action_module;
-    assistant_client::FuchsiaApiDelegate* fuchsia_api_delegate;
-    assistant_client::AssistantManagerDelegate* assistant_manager_delegate;
-    assistant_client::ConversationStateListener* conversation_state_listener;
-    assistant_client::DeviceStateListener* device_state_listener;
-    AssistantEventObserver* event_observer;
-    std::string libassistant_config;
-    std::string locale;
-    std::string locale_override;
-    bool spoken_feedback_enabled;
-    AuthTokens auth_tokens;
-    base::OnceClosure done_callback;
-  };
-
   void FinishCreatingAssistant();
 
   // libassistant::mojom::StateObserver implementation:
@@ -141,20 +119,22 @@ class ServiceControllerProxy : private libassistant::mojom::StateObserver {
   // Used internally for consistency checks.
   State state_ = State::kStopped;
 
-  scoped_refptr<base::SingleThreadTaskRunner> background_task_runner_;
+  // Owned by |AssistantManagerServiceImpl| which (indirectly) also owns us.
+  LibassistantServiceHost* const host_;
 
   mojo::Remote<chromeos::libassistant::mojom::ServiceController>
       service_controller_remote_;
   mojo::Receiver<chromeos::libassistant::mojom::StateObserver>
       state_observer_receiver_;
 
-  // Arguments passed to the last Start() call.
-  // Used to finish starting Libassistant after the Libassistant mojom service
-  // signals it has created the required objects.
-  // Unset once we've finished starting.
-  base::Optional<StartArguments> pending_start_argument_;
+  // Callback passed to Start(). Will be invoked once the Libassistant service
+  // has started.
+  base::Optional<base::OnceClosure> on_start_done_callback_;
 
   std::unique_ptr<CrosDisplayConnection> display_connection_;
+  // Populated when we're starting but not started yet, so after Start() has
+  // been called but before the mojom service signalled it has started.
+  std::unique_ptr<CrosDisplayConnection> pending_display_connection_;
 
   base::WeakPtrFactory<ServiceControllerProxy> weak_factory_{this};
 };

chromeos/services/assistant/test_support/fake_service_controller.cc

@@ -4,6 +4,7 @@
 
 #include "chromeos/services/assistant/test_support/fake_service_controller.h"
 
+#include "chromeos/services/assistant/public/cpp/migration/libassistant_v1_api.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
@@ -35,6 +36,10 @@ void FakeServiceController::Unbind() {
   state_observers_.Clear();
 }
 
+void FakeServiceController::SetInitializeCallback(InitializeCallback callback) {
+  initialize_callback_ = std::move(callback);
+}
+
 void FakeServiceController::BlockStartCalls() {
   // This lock will be release in |UnblockStartCalls|.
   start_mutex_.lock();
@@ -50,6 +55,12 @@ void FakeServiceController::Start(const std::string& libassistant_config) {
   // Will block if |BlockStartCalls| was invoked.
   std::lock_guard<std::mutex> lock(start_mutex_);
 
+  if (initialize_callback_) {
+    std::move(initialize_callback_)
+        .Run(LibassistantV1Api::Get()->assistant_manager(),
+             LibassistantV1Api::Get()->assistant_manager_internal());
+  }
+
   SetState(State::kStarted);
 }
 

chromeos/services/assistant/test_support/fake_service_controller.h

@@ -12,6 +12,11 @@
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote_set.h"
 
+namespace assistant_client {
+class AssistantManager;
+class AssistantManagerInternal;
+}  // namespace assistant_client
+
 namespace chromeos {
 namespace assistant {
 
@@ -21,6 +26,9 @@ namespace assistant {
 class FakeServiceController : public libassistant::mojom::ServiceController {
  public:
   using State = libassistant::mojom::ServiceState;
+  using InitializeCallback =
+      base::OnceCallback<void(assistant_client::AssistantManager*,
+                              assistant_client::AssistantManagerInternal*)>;
 
   FakeServiceController();
   FakeServiceController(FakeServiceController&) = delete;
@@ -39,6 +47,8 @@ class FakeServiceController : public libassistant::mojom::ServiceController {
                 pending_receiver);
   void Unbind();
 
+  void SetInitializeCallback(InitializeCallback callback);
+
   // Call this to block any call to |Start|. The observers will not be invoked
   // as long as the start call is blocked. Unblock these calls using
   // |UnblockStartCalls|. This is not enabled by default, so unless you call
@@ -61,6 +71,8 @@ class FakeServiceController : public libassistant::mojom::ServiceController {
   // Config passed to LibAssistant when it was started.
   std::string libassistant_config_;
 
+  InitializeCallback initialize_callback_;
+
   State state_ = State::kStopped;
   mojo::Receiver<libassistant::mojom::ServiceController> receiver_;
   mojo::RemoteSet<libassistant::mojom::StateObserver> state_observers_;

chromeos/services/libassistant/libassistant_service.cc

@@ -29,5 +29,9 @@ void LibassistantService::BindServiceController(
   service_controller_->Bind(std::move(receiver));
 }
 
+void LibassistantService::SetInitializeCallback(InitializeCallback callback) {
+  service_controller().SetInitializeCallback(std::move(callback));
+}
+
 }  // namespace libassistant
 }  // namespace chromeos

chromeos/services/libassistant/libassistant_service.h

@@ -12,6 +12,8 @@
 #include "mojo/public/cpp/bindings/receiver.h"
 
 namespace assistant_client {
+class AssistantManager;
+class AssistantManagerInternal;
 class PlatformApi;
 }  // namespace assistant_client
 
@@ -29,6 +31,10 @@ class ServiceController;
 class COMPONENT_EXPORT(LIBASSISTANT_SERVICE) LibassistantService
     : public mojom::LibassistantService {
  public:
+  using InitializeCallback =
+      base::OnceCallback<void(assistant_client::AssistantManager*,
+                              assistant_client::AssistantManagerInternal*)>;
+
   explicit LibassistantService(
       mojo::PendingReceiver<mojom::LibassistantService> receiver,
       assistant_client::PlatformApi* platform_api,
@@ -37,7 +43,11 @@ class COMPONENT_EXPORT(LIBASSISTANT_SERVICE) LibassistantService
   LibassistantService& operator=(LibassistantService&) = delete;
   ~LibassistantService() override;
 
+  void SetInitializeCallback(InitializeCallback callback);
+
  private:
+  ServiceController& service_controller() { return *service_controller_; }
+
   // mojom::LibassistantService implementation:
   void BindServiceController(
       mojo::PendingReceiver<mojom::ServiceController> receiver) override;

chromeos/services/libassistant/service_controller.cc

@@ -32,6 +32,10 @@ void ServiceController::Bind(
   receiver_.Bind(std::move(receiver));
 }
 
+void ServiceController::SetInitializeCallback(InitializeCallback callback) {
+  initialize_callback_ = std::move(callback);
+}
+
 void ServiceController::Start(const std::string& libassistant_config) {
   if (state_ != ServiceState::kStopped)
     return;
@@ -43,6 +47,13 @@ void ServiceController::Start(const std::string& libassistant_config) {
   libassistant_v1_api_ = std::make_unique<assistant::LibassistantV1Api>(
       assistant_manager_.get(), assistant_manager_internal_);
 
+  if (initialize_callback_) {
+    std::move(initialize_callback_)
+        .Run(assistant_manager(), assistant_manager_internal());
+  }
+
+  assistant_manager()->Start();
+
   SetStateAndInformObservers(ServiceState::kStarted);
 
   for (auto& observer : assistant_manager_observers_) {

chromeos/services/libassistant/service_controller.h

@@ -43,6 +43,10 @@ namespace libassistant {
 class COMPONENT_EXPORT(LIBASSISTANT_SERVICE) ServiceController
     : public mojom::ServiceController {
  public:
+  using InitializeCallback =
+      base::OnceCallback<void(assistant_client::AssistantManager*,
+                              assistant_client::AssistantManagerInternal*)>;
+
   ServiceController(assistant::AssistantManagerServiceDelegate* delegate,
                     assistant_client::PlatformApi* platform_api);
   ServiceController(ServiceController&) = delete;
@@ -51,6 +55,12 @@ class COMPONENT_EXPORT(LIBASSISTANT_SERVICE) ServiceController
 
   void Bind(mojo::PendingReceiver<mojom::ServiceController> receiver);
 
+  // Set a callback to initialize |AssistantManager| and
+  // |AssistantManagerInternal|. This callback will be invoked before
+  // AssistantManager::Start() is called. This is temporary until we've migrated
+  // all initialization code to this class.
+  void SetInitializeCallback(InitializeCallback callback);
+
   // mojom::ServiceController implementation:
   void Start(const std::string& libassistant_config) override;
   void Stop() override;
@@ -77,6 +87,9 @@ class COMPONENT_EXPORT(LIBASSISTANT_SERVICE) ServiceController
   // Owned by |AssistantManagerServiceImpl| which indirectly owns us.
   assistant_client::PlatformApi* const platform_api_;
 
+  // Callback called to initialize |AssistantManager| before it's started.
+  InitializeCallback initialize_callback_;
+
   std::unique_ptr<assistant_client::AssistantManager> assistant_manager_;
   assistant_client::AssistantManagerInternal* assistant_manager_internal_ =
       nullptr;