assistant: small refactors

* Create |new_display_connection_| instance when starting new
  libassistant instance to make it consistent and safer
* Add lock region to guard the background thread
* Fix chrome_http_connection.cc file ordering

Bug: None
Test: locally build and run
Change-Id: I6ae24148dc3c3fe1f5a40863beabd10707b1edb1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830014
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700910}

chromeos/services/assistant/assistant_manager_service_impl.cc

@@ -1002,12 +1002,13 @@ void AssistantManagerServiceImpl::OnCommunicationError(int error_code) {
 void AssistantManagerServiceImpl::StartAssistantInternal(
     const base::Optional<std::string>& access_token) {
   DCHECK(background_thread_.task_runner()->BelongsToCurrentThread());
-
-  display_connection_ = std::make_unique<CrosDisplayConnection>(
+  base::AutoLock lock(new_assistant_manager_lock_);
+  // There can only be one |AssistantManager| instance at any given time.
+  DCHECK(!assistant_manager_);
+  new_display_connection_ = std::make_unique<CrosDisplayConnection>(
       this, assistant::features::IsFeedbackUiEnabled(),
       assistant::features::IsMediaSessionIntegrationEnabled());
 
-  base::AutoLock lock(new_assistant_manager_lock_);
   new_assistant_manager_.reset(assistant_client::AssistantManager::Create(
       platform_api_.get(), CreateLibAssistantConfig()));
   auto* assistant_manager_internal =
@@ -1015,7 +1016,8 @@ void AssistantManagerServiceImpl::StartAssistantInternal(
 
   UpdateInternalOptions(assistant_manager_internal);
 
-  assistant_manager_internal->SetDisplayConnection(display_connection_.get());
+  assistant_manager_internal->SetDisplayConnection(
+      new_display_connection_.get());
   assistant_manager_internal->RegisterActionModule(action_module_.get());
   assistant_manager_internal->SetAssistantManagerDelegate(this);
   assistant_manager_internal->GetFuchsiaApiHelperOrDie()->SetFuchsiaApiDelegate(
@@ -1054,6 +1056,7 @@ void AssistantManagerServiceImpl::PostInitAssistant(
       return;
     }
 
+    display_connection_ = std::move(new_display_connection_);
     assistant_manager_ = std::move(new_assistant_manager_);
   }
 

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -318,6 +318,10 @@ class AssistantManagerServiceImpl
   // NOTE: |display_connection_| is used by |assistant_manager_| and must be
   // declared before so it will be destructed after.
   std::unique_ptr<CrosDisplayConnection> display_connection_;
+  // Similar to |new_asssistant_manager_|, created on |background_thread_| then
+  // posted to main thread to finish initialization then move to
+  // |display_connection_|.
+  std::unique_ptr<CrosDisplayConnection> new_display_connection_;
   std::unique_ptr<assistant_client::AssistantManager> assistant_manager_;
   std::unique_ptr<AssistantSettingsManagerImpl> assistant_settings_manager_;
   // |new_asssistant_manager_| is created on |background_thread_| then posted to

chromeos/services/assistant/chromium_http_connection.cc

@@ -41,18 +41,6 @@ constexpr int kResponseCodeInvalid = -1;
 
 }  // namespace
 
-ChromiumHttpConnectionFactory::ChromiumHttpConnectionFactory(
-    std::unique_ptr<SharedURLLoaderFactoryInfo> url_loader_factory_info)
-    : url_loader_factory_(
-          SharedURLLoaderFactory::Create(std::move(url_loader_factory_info))) {}
-
-ChromiumHttpConnectionFactory::~ChromiumHttpConnectionFactory() = default;
-
-HttpConnection* ChromiumHttpConnectionFactory::Create(
-    HttpConnection::Delegate* delegate) {
-  return new ChromiumHttpConnection(url_loader_factory_->Clone(), delegate);
-}
-
 ChromiumHttpConnection::ChromiumHttpConnection(
     std::unique_ptr<SharedURLLoaderFactoryInfo> url_loader_factory_info,
     Delegate* delegate)
@@ -404,5 +392,17 @@ void ChromiumHttpConnection::OnResponseStarted(
   }
 }
 
+ChromiumHttpConnectionFactory::ChromiumHttpConnectionFactory(
+    std::unique_ptr<SharedURLLoaderFactoryInfo> url_loader_factory_info)
+    : url_loader_factory_(
+          SharedURLLoaderFactory::Create(std::move(url_loader_factory_info))) {}
+
+ChromiumHttpConnectionFactory::~ChromiumHttpConnectionFactory() = default;
+
+HttpConnection* ChromiumHttpConnectionFactory::Create(
+    HttpConnection::Delegate* delegate) {
+  return new ChromiumHttpConnection(url_loader_factory_->Clone(), delegate);
+}
+
 }  // namespace assistant
 }  // namespace chromeos