Revert "Introduce AssistantServiceManager::State::STARTING state."

This reverts commit de88f517.

Reason for revert: Breaks internal builder

Original change's description:
> Introduce AssistantServiceManager::State::STARTING state.
>
> Until now the |AssistantServiceManager::State| enum did not distinguish
> between the state in which libassistant was started (but not ready yet)
> and in which it is ready.
> That distinction is needed to be able to add tests using libassistant,
> as they have to wait until it is ready.
>
> So now there are 4 states:
>     - STOPPED: It has not started/it has been stopped (this is the same
>     as the old STOPPED state).
>     - STARTING: |AssistantServiceManager::Start| has been called and the
>     libassistant |AssistantManager::Start| call has not finished yet (this
>     is the same as the old STARTED state).
>     - STARTED: the libassistant |AssistantManager::Start| call has
>     finished, but libassistant has not signalled it is ready yet (this
>     is the same as the old RUNNING state).
>     - RUNNING: libassistant signalled it is ready (this state is new).
>
> As this CL also introduces the first unittests for
> |AssistantServiceManagerImpl|, it also contains a bunch of fake/dummy
> classes used for the unittests.
>
> This CL increases test coverage in //chromeos/services/assistant from 28%
> to 41%.
>
> Change-Id: If38a6b70e22e6392537b6658e5f39eebf6dd9099
> BUG: b/141264108
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810019
> Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#700410}

TBR=xiaohuic@chromium.org,wutao@chromium.org,meilinw@chromium.org,jeroendh@google.com

Change-Id: Ia0a876aa4ccd1043d54f10dc42a0930ade00fb6f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1828221Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/master@{#700462}

chromeos/services/assistant/BUILD.gn

@@ -68,9 +68,6 @@ component("lib") {
 
   if (enable_cros_libassistant) {
     sources += [
-      "assistant_manager_service_delegate.h",
-      "assistant_manager_service_delegate_impl.cc",
-      "assistant_manager_service_delegate_impl.h",
       "assistant_manager_service_impl.cc",
       "assistant_manager_service_impl.h",
       "assistant_settings_manager_impl.cc",
@@ -79,7 +76,6 @@ component("lib") {
       "chromium_api_delegate.h",
       "chromium_http_connection.cc",
       "chromium_http_connection.h",
-      "cros_platform_api.h",
       "media_session/assistant_media_session.cc",
       "media_session/assistant_media_session.h",
       "platform/audio_device_owner.cc",
@@ -165,32 +161,21 @@ source_set("tests") {
     "fake_client.cc",
     "fake_client.h",
     "service_unittest.cc",
-    "test_support/fully_initialized_assistant_state.cc",
-    "test_support/fully_initialized_assistant_state.h",
   ]
 
   if (enable_cros_libassistant) {
     sources += [
-      "assistant_manager_service_impl_unittest.cc",
       "platform/audio_input_impl_unittest.cc",
       "platform/audio_output_provider_impl_unittest.cc",
       "platform/network_provider_impl_unittest.cc",
       "platform/power_manager_provider_impl_unittest.cc",
       "platform/system_provider_impl_unittest.cc",
-      "test_support/fake_assistant_manager_service_delegate.cc",
-      "test_support/fake_assistant_manager_service_delegate.h",
-      "test_support/fake_platform_api.cc",
-      "test_support/fake_platform_api.h",
-      "test_support/mock_media_manager.cc",
-      "test_support/mock_media_manager.h",
     ]
 
     deps += [
-      "//chromeos/assistant/internal",
-      "//chromeos/assistant/internal:test_support",
-      "//chromeos/assistant/internal/proto/google3",
       "//chromeos/dbus",
       "//chromeos/services/network_config/public/mojom",
+      "//libassistant/shared/public",
       "//services/audio/public/cpp:test_support",
       "//services/device/public/cpp:test_support",
     ]

chromeos/services/assistant/assistant_manager_service.h

@@ -26,14 +26,10 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerService
   enum State {
     // Initial state, the service is created but not started yet.
     STOPPED = 0,
-    // Start has been called but libassistant creation is still in progress.
-    // Calling |assistant_manager()| will still return a nullptr.
-    STARTING = 1,
-    // The service is started, libassistant has been created, but libassistant
-    // is not ready yet to take requests.
-    STARTED = 2,
+    // The service is started, it takes a little time to be fully running.
+    STARTED = 1,
     // The service is fully running and ready to take requests.
-    RUNNING = 3
+    RUNNING = 2
   };
 
   ~AssistantManagerService() override = default;

chromeos/services/assistant/assistant_manager_service_delegate.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_MANAGER_SERVICE_DELEGATE_H_
-#define CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_MANAGER_SERVICE_DELEGATE_H_
-
-#include <memory>
-
-#include "base/macros.h"
-#include "base/single_thread_task_runner.h"
-
-namespace assistant_client {
-class AssistantManager;
-class AssistantManagerInternal;
-class PlatformApi;
-}  // namespace assistant_client
-
-namespace chromeos {
-namespace assistant {
-
-class AssistantMediaSession;
-class CrosPlatformApi;
-
-// Interface class that provides factory methods for assistant internal
-// functionality.
-class AssistantManagerServiceDelegate {
- public:
-  AssistantManagerServiceDelegate() = default;
-  virtual ~AssistantManagerServiceDelegate() = default;
-
-  virtual std::unique_ptr<CrosPlatformApi> CreatePlatformApi(
-      AssistantMediaSession* media_session,
-      scoped_refptr<base::SingleThreadTaskRunner>
-          background_thread_task_runner) = 0;
-
-  virtual std::unique_ptr<assistant_client::AssistantManager>
-  CreateAssistantManager(assistant_client::PlatformApi* platform_api,
-                         const std::string& lib_assistant_config) = 0;
-
-  virtual assistant_client::AssistantManagerInternal*
-  UnwrapAssistantManagerInternal(
-      assistant_client::AssistantManager* assistant_manager) = 0;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(AssistantManagerServiceDelegate);
-};
-
-}  // namespace assistant
-}  // namespace chromeos
-
-#endif  // CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_MANAGER_SERVICE_DELEGATE_H_

chromeos/services/assistant/assistant_manager_service_delegate_impl.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chromeos/services/assistant/assistant_manager_service_delegate_impl.h"
-
-#include "ash/public/cpp/assistant/assistant_state_base.h"
-#include "chromeos/services/assistant/platform_api_impl.h"
-#include "chromeos/services/assistant/service_context.h"
-#include "libassistant/shared/internal_api/assistant_manager_internal.h"
-#include "libassistant/shared/public/assistant_manager.h"
-
-namespace chromeos {
-namespace assistant {
-
-AssistantManagerServiceDelegateImpl::AssistantManagerServiceDelegateImpl(
-    mojo::PendingRemote<device::mojom::BatteryMonitor> battery_monitor,
-    mojom::Client* client,
-    ServiceContext* context)
-    : battery_monitor_(std::move(battery_monitor)),
-      client_(client),
-      context_(context) {}
-
-AssistantManagerServiceDelegateImpl::~AssistantManagerServiceDelegateImpl() =
-    default;
-
-std::unique_ptr<CrosPlatformApi>
-AssistantManagerServiceDelegateImpl::CreatePlatformApi(
-    AssistantMediaSession* media_session,
-    scoped_refptr<base::SingleThreadTaskRunner> background_thread_task_runner) {
-  return std::make_unique<PlatformApiImpl>(
-      client_, media_session, context_->power_manager_client(),
-      context_->cras_audio_handler(), std::move(battery_monitor_),
-      context_->main_task_runner(), background_thread_task_runner,
-      context_->assistant_state()->locale().value());
-}
-
-std::unique_ptr<assistant_client::AssistantManager>
-AssistantManagerServiceDelegateImpl::CreateAssistantManager(
-    assistant_client::PlatformApi* platform_api,
-    const std::string& lib_assistant_config) {
-  // This circumnvent way of creating the unique_ptr is required because
-  // |AssistantManager::Create| returns a raw pointer, and passing that in the
-  // constructor of unique_ptr is blocked by our presubmit checks that try to
-  // force us to use make_unique, which we can't use here.
-  std::unique_ptr<assistant_client::AssistantManager> result;
-  result.reset(assistant_client::AssistantManager::Create(
-      platform_api, lib_assistant_config));
-  return result;
-}
-
-assistant_client::AssistantManagerInternal*
-AssistantManagerServiceDelegateImpl::UnwrapAssistantManagerInternal(
-    assistant_client::AssistantManager* assistant_manager) {
-  return assistant_client::UnwrapAssistantManagerInternal(assistant_manager);
-}
-
-}  // namespace assistant
-}  // namespace chromeos

chromeos/services/assistant/assistant_manager_service_delegate_impl.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_MANAGER_SERVICE_DELEGATE_IMPL_H_
-#define CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_MANAGER_SERVICE_DELEGATE_IMPL_H_
-
-#include "chromeos/services/assistant/assistant_manager_service_delegate.h"
-#include "mojo/public/cpp/bindings/pending_remote.h"
-#include "services/device/public/mojom/battery_monitor.mojom.h"
-
-namespace chromeos {
-namespace assistant {
-namespace mojom {
-class Client;
-}  // namespace mojom
-}  // namespace assistant
-}  // namespace chromeos
-
-namespace chromeos {
-namespace assistant {
-
-class ServiceContext;
-
-class AssistantManagerServiceDelegateImpl
-    : public AssistantManagerServiceDelegate {
- public:
-  AssistantManagerServiceDelegateImpl(
-      mojo::PendingRemote<device::mojom::BatteryMonitor> battery_monitor,
-      mojom::Client* client,
-      ServiceContext* context);
-  ~AssistantManagerServiceDelegateImpl() override;
-
-  std::unique_ptr<CrosPlatformApi> CreatePlatformApi(
-      AssistantMediaSession* media_session,
-      scoped_refptr<base::SingleThreadTaskRunner> background_thread_task_runner)
-      override;
-
-  std::unique_ptr<assistant_client::AssistantManager> CreateAssistantManager(
-      assistant_client::PlatformApi* platform_api,
-      const std::string& lib_assistant_config) override;
-
-  assistant_client::AssistantManagerInternal* UnwrapAssistantManagerInternal(
-      assistant_client::AssistantManager* assistant_manager) override;
-
- private:
-  mojo::PendingRemote<device::mojom::BatteryMonitor> battery_monitor_;
-  // Owned by the parent |Service| which will destroy |this| before |context_|.
-  mojom::Client* client_;
-  ServiceContext* context_;
-
-  DISALLOW_COPY_AND_ASSIGN(AssistantManagerServiceDelegateImpl);
-};
-
-}  // namespace assistant
-}  // namespace chromeos
-
-#endif  // CHROMEOS_SERVICES_ASSISTANT_ASSISTANT_MANAGER_SERVICE_DELEGATE_IMPL_H_

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -20,6 +20,7 @@
 #include "chromeos/services/assistant/assistant_manager_service.h"
 #include "chromeos/services/assistant/assistant_settings_manager_impl.h"
 #include "chromeos/services/assistant/chromium_api_delegate.h"
+#include "chromeos/services/assistant/platform_api_impl.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "libassistant/shared/internal_api/assistant_manager_delegate.h"
 #include "libassistant/shared/public/conversation_state_listener.h"
@@ -52,9 +53,7 @@ namespace chromeos {
 namespace assistant {
 
 class AssistantMediaSession;
-class CrosPlatformApi;
 class ServiceContext;
-class AssistantManagerServiceDelegate;
 
 // Enumeration of Assistant query response type, also recorded in histograms.
 // These values are persisted to logs. Entries should not be renumbered and
@@ -84,7 +83,7 @@ enum class AssistantQueryResponseType {
 // Since LibAssistant is a standalone library, all callbacks come from it
 // running on threads not owned by Chrome. Thus we need to post the callbacks
 // onto the main thread.
-class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
+class AssistantManagerServiceImpl
     : public AssistantManagerService,
       public ::chromeos::assistant::action::AssistantActionObserver,
       public AssistantEventObserver,
@@ -98,8 +97,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   // |service| owns this class and must outlive this class.
   AssistantManagerServiceImpl(
       mojom::Client* client,
+      mojo::PendingRemote<device::mojom::BatteryMonitor> battery_monitor,
       ServiceContext* context,
-      std::unique_ptr<AssistantManagerServiceDelegate> delegate,
       std::unique_ptr<network::SharedURLLoaderFactoryInfo>
           url_loader_factory_info,
       bool is_signed_out_mode);
@@ -223,10 +222,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   void UpdateInternalMediaPlayerStatus(
       media_session::mojom::MediaSessionAction action);
 
-  // The start runs in the background. This will wait until the background
-  // thread is finished.
-  void WaitUntilStartIsFinishedForTesting();
-
  private:
   void StartAssistantInternal(const base::Optional<std::string>& access_token);
   void PostInitAssistant(base::OnceClosure post_init_callback);
@@ -314,13 +309,10 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   mojom::DeviceActions* device_actions();
   scoped_refptr<base::SequencedTaskRunner> main_task_runner();
 
-  bool HasStartFinished() const;
-  void SetStartFinished();
-
   mojom::Client* const client_;
   State state_ = State::STOPPED;
   std::unique_ptr<AssistantMediaSession> media_session_;
-  std::unique_ptr<CrosPlatformApi> platform_api_;
+  std::unique_ptr<PlatformApiImpl> platform_api_;
   std::unique_ptr<action::CrosActionModule> action_module_;
   ChromiumApiDelegate chromium_api_delegate_;
   // NOTE: |display_connection_| is used by |assistant_manager_| and must be
@@ -328,14 +320,11 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   std::unique_ptr<CrosDisplayConnection> display_connection_;
   std::unique_ptr<assistant_client::AssistantManager> assistant_manager_;
   std::unique_ptr<AssistantSettingsManagerImpl> assistant_settings_manager_;
-  // |new_assistant_manager_| is created on |background_thread_| then posted to
+  // |new_asssistant_manager_| is created on |background_thread_| then posted to
   // main thread to finish initialization then move to |assistant_manager_|.
   std::unique_ptr<assistant_client::AssistantManager> new_assistant_manager_;
-  // Same ownership as |new_assistant_manager_|.
-  assistant_client::AssistantManagerInternal* new_assistant_manager_internal_ =
-      nullptr;
   base::Lock new_assistant_manager_lock_;
-  // same ownership as |assistant_manager_|.
+  // same ownership as assistant_manager_.
   assistant_client::AssistantManagerInternal* assistant_manager_internal_ =
       nullptr;
   mojo::RemoteSet<mojom::AssistantInteractionSubscriber>
@@ -345,8 +334,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   // Owned by the parent |Service| which will destroy |this| before |context_|.
   ServiceContext* const context_;
 
-  std::unique_ptr<AssistantManagerServiceDelegate> delegate_;
-
   bool spoken_feedback_enabled_ = false;
 
   ax::mojom::AssistantExtraPtr assistant_extra_;
@@ -381,6 +368,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   base::UnguessableToken media_session_audio_focus_id_ =
       base::UnguessableToken::Null();
 
+  bool start_finished_ = false;
+
   mojo::Binding<mojom::AppListEventSubscriber> app_list_subscriber_binding_;
 
   base::ObserverList<AssistantCommunicationErrorObserver> error_observers_;

chromeos/services/assistant/assistant_settings_manager_impl.cc

@@ -24,17 +24,6 @@ using SpeakerIdEnrollmentState =
 namespace chromeos {
 namespace assistant {
 
-namespace {
-
-bool HasStarted(const AssistantManagerService* assistant_manager_service) {
-  return (assistant_manager_service->GetState() ==
-              AssistantManagerService::STARTED ||
-          assistant_manager_service->GetState() ==
-              AssistantManagerService::RUNNING);
-}
-
-}  // namespace
-
 AssistantSettingsManagerImpl::AssistantSettingsManagerImpl(
     ServiceContext* context,
     AssistantManagerServiceImpl* assistant_manager_service)
@@ -51,7 +40,8 @@ void AssistantSettingsManagerImpl::BindRequest(
 
 void AssistantSettingsManagerImpl::GetSettings(const std::string& selector,
                                                GetSettingsCallback callback) {
-  DCHECK(HasStarted(assistant_manager_service_));
+  DCHECK(assistant_manager_service_->GetState() ==
+         AssistantManagerService::State::RUNNING);
   DCHECK(main_task_runner()->RunsTasksInCurrentSequence());
 
   // TODO(xiaohuic): libassistant could be restarting for various reasons. In
@@ -89,7 +79,8 @@ void AssistantSettingsManagerImpl::GetSettings(const std::string& selector,
 void AssistantSettingsManagerImpl::UpdateSettings(
     const std::string& update,
     GetSettingsCallback callback) {
-  DCHECK(HasStarted(assistant_manager_service_));
+  DCHECK(assistant_manager_service_->GetState() ==
+         AssistantManagerService::State::RUNNING);
   DCHECK(main_task_runner()->RunsTasksInCurrentSequence());
 
   if (!assistant_manager_service_->assistant_manager_internal()) {
@@ -123,7 +114,8 @@ void AssistantSettingsManagerImpl::UpdateSettings(
 void AssistantSettingsManagerImpl::StartSpeakerIdEnrollment(
     bool skip_cloud_enrollment,
     mojom::SpeakerIdEnrollmentClientPtr client) {
-  DCHECK(HasStarted(assistant_manager_service_));
+  DCHECK(assistant_manager_service_->GetState() ==
+         AssistantManagerService::State::RUNNING);
   DCHECK(main_task_runner()->RunsTasksInCurrentSequence());
 
   if (!assistant_manager_service_->assistant_manager_internal())
@@ -150,7 +142,8 @@ void AssistantSettingsManagerImpl::StartSpeakerIdEnrollment(
 
 void AssistantSettingsManagerImpl::StopSpeakerIdEnrollment(
     StopSpeakerIdEnrollmentCallback callback) {
-  DCHECK(HasStarted(assistant_manager_service_));
+  DCHECK(assistant_manager_service_->GetState() ==
+         AssistantManagerService::State::RUNNING);
   DCHECK(main_task_runner()->RunsTasksInCurrentSequence());
 
   if (!assistant_manager_service_->assistant_manager_internal()) {

chromeos/services/assistant/constants.h

@@ -5,12 +5,9 @@
 #ifndef CHROMEOS_SERVICES_ASSISTANT_CONSTANTS_H_
 #define CHROMEOS_SERVICES_ASSISTANT_CONSTANTS_H_
 
-#include "base/component_export.h"
-
 namespace chromeos {
 namespace assistant {
 
-COMPONENT_EXPORT(ASSISTANT_SERVICE)
 extern const char kUserID[];
 
 }  // namespace assistant

chromeos/services/assistant/cros_platform_api.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROMEOS_SERVICES_ASSISTANT_CROS_PLATFORM_API_H_
-#define CHROMEOS_SERVICES_ASSISTANT_CROS_PLATFORM_API_H_
-
-#include "base/macros.h"
-#include "libassistant/shared/public/platform_api.h"
-
-namespace chromeos {
-namespace assistant {
-
-// Platform API required by the voice assistant, extended with some methods used
-// when ChromeOS needs to make changes to the platform state.
-class CrosPlatformApi : public assistant_client::PlatformApi {
- public:
-  CrosPlatformApi() = default;
-  ~CrosPlatformApi() override = default;
-
-  // Called when the mic state associated with the interaction is changed.
-  virtual void SetMicState(bool mic_open) = 0;
-
-  virtual void OnConversationTurnStarted() = 0;
-  virtual void OnConversationTurnFinished() = 0;
-
-  // Called when hotword enabled status changed.
-  virtual void OnHotwordEnabled(bool enable) = 0;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(CrosPlatformApi);
-};
-
-}  // namespace assistant
-}  // namespace chromeos
-
-#endif  // CHROMEOS_SERVICES_ASSISTANT_CROS_PLATFORM_API_H_

chromeos/services/assistant/fake_assistant_manager_service_impl.cc

@@ -23,7 +23,7 @@ void FakeAssistantManagerServiceImpl::Start(
     const base::Optional<std::string>& access_token,
     bool enable_hotword,
     base::OnceClosure callback) {
-  state_ = State::STARTING;
+  state_ = State::STARTED;
   start_callback_ = std::move(callback);
 }
 

chromeos/services/assistant/platform_api_impl.cc

@@ -184,14 +184,6 @@ void PlatformApiImpl::SetMicState(bool mic_open) {
   audio_input_provider_.SetMicState(mic_open);
 }
 
-void PlatformApiImpl::OnConversationTurnStarted() {
-  audio_input_provider_.GetAudioInput().OnConversationTurnStarted();
-}
-
-void PlatformApiImpl::OnConversationTurnFinished() {
-  audio_input_provider_.GetAudioInput().OnConversationTurnFinished();
-}
-
 void PlatformApiImpl::OnHotwordEnabled(bool enable) {
   audio_input_provider_.OnHotwordEnabled(enable);
 }

chromeos/services/assistant/platform_api_impl.h

@@ -11,13 +11,13 @@
 #include <vector>
 
 #include "chromeos/audio/cras_audio_handler.h"
-#include "chromeos/services/assistant/cros_platform_api.h"
 #include "chromeos/services/assistant/platform/audio_input_provider_impl.h"
 #include "chromeos/services/assistant/platform/audio_output_provider_impl.h"
 #include "chromeos/services/assistant/platform/file_provider_impl.h"
 #include "chromeos/services/assistant/platform/network_provider_impl.h"
 #include "chromeos/services/assistant/platform/system_provider_impl.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
+#include "libassistant/shared/public/platform_api.h"
 #include "libassistant/shared/public/platform_auth.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "services/device/public/mojom/battery_monitor.mojom.h"
@@ -30,7 +30,7 @@ namespace assistant {
 class AssistantMediaSession;
 
 // Platform API required by the voice assistant.
-class PlatformApiImpl : public CrosPlatformApi,
+class PlatformApiImpl : public assistant_client::PlatformApi,
                         CrasAudioHandler::AudioObserver {
  public:
   PlatformApiImpl(
@@ -56,13 +56,10 @@ class PlatformApiImpl : public CrosPlatformApi,
   void OnAudioNodesChanged() override;
 
   // Called when the mic state associated with the interaction is changed.
-  void SetMicState(bool mic_open) override;
-
-  void OnConversationTurnStarted() override;
-  void OnConversationTurnFinished() override;
+  void SetMicState(bool mic_open);
 
   // Called when hotword enabled status changed.
-  void OnHotwordEnabled(bool enable) override;
+  void OnHotwordEnabled(bool enable);
 
  private:
   // ChromeOS does not use auth manager, so we don't yet need to implement a

chromeos/services/assistant/service.cc

@@ -24,7 +24,6 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager/power_supply_properties.pb.h"
 #include "chromeos/services/assistant/assistant_manager_service.h"
-#include "chromeos/services/assistant/assistant_manager_service_delegate_impl.h"
 #include "chromeos/services/assistant/assistant_settings_manager.h"
 #include "chromeos/services/assistant/fake_assistant_manager_service_impl.h"
 #include "chromeos/services/assistant/fake_assistant_settings_manager_impl.h"
@@ -315,7 +314,6 @@ void Service::UpdateAssistantManagerState() {
         DVLOG(1) << "Request Assistant start";
       }
       break;
-    case AssistantManagerService::State::STARTING:
     case AssistantManagerService::State::STARTED:
       // Wait if |assistant_manager_service_| is not at a stable state.
       update_assistant_manager_callback_.Cancel();
@@ -437,13 +435,10 @@ void Service::CreateAssistantManagerService() {
   client_->RequestBatteryMonitor(
       battery_monitor.InitWithNewPipeAndPassReceiver());
 
-  auto delegate = std::make_unique<AssistantManagerServiceDelegateImpl>(
-      std::move(battery_monitor), client_.get(), context());
-
   // |assistant_manager_service_| is only created once.
   DCHECK(url_loader_factory_info_);
   assistant_manager_service_ = std::make_unique<AssistantManagerServiceImpl>(
-      client_.get(), context(), std::move(delegate),
+      client_.get(), std::move(battery_monitor), context(),
       std::move(url_loader_factory_info_), is_signed_out_mode_);
 #else
   assistant_manager_service_ =
@@ -455,10 +450,9 @@ void Service::CreateAssistantManagerService() {
 
 void Service::FinalizeAssistantManagerService() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   DCHECK(assistant_manager_service_->GetState() ==
-             AssistantManagerService::STARTED ||
-         assistant_manager_service_->GetState() ==
-             AssistantManagerService::RUNNING);
+         AssistantManagerService::State::RUNNING);
 
   // Using session_observer_binding_ as a flag to control onetime initialization
   if (!observing_ash_session_) {

chromeos/services/assistant/service_unittest.cc

@@ -8,6 +8,7 @@
 #include <utility>
 #include <vector>
 
+#include "ash/public/cpp/assistant/assistant_state.h"
 #include "ash/public/mojom/assistant_state_controller.mojom.h"
 #include "base/bind.h"
 #include "base/logging.h"
@@ -26,7 +27,6 @@
 #include "chromeos/services/assistant/fake_client.h"
 #include "chromeos/services/assistant/pref_connection_delegate.h"
 #include "chromeos/services/assistant/public/cpp/assistant_prefs.h"
-#include "chromeos/services/assistant/test_support/fully_initialized_assistant_state.h"
 #include "components/prefs/testing_pref_service.h"
 #include "mojo/public/cpp/bindings/interface_ptr_set.h"
 #include "services/identity/public/mojom/identity_accessor.mojom.h"
@@ -131,6 +131,12 @@ class FakeAssistantClient : public FakeClient {
   explicit FakeAssistantClient(ash::AssistantState* assistant_state)
       : assistant_state_(assistant_state) {}
 
+  mojom::ClientPtr CreatePendingRemoteAndBind() {
+    mojom::ClientPtr ptr;
+    binding_.Bind(mojo::MakeRequest(&ptr));
+    return ptr;
+  }
+
  private:
   // FakeClient:
   void RequestAssistantStateController(
@@ -140,6 +146,7 @@ class FakeAssistantClient : public FakeClient {
   }
 
   ash::AssistantState* const assistant_state_;
+  mojo::Binding<mojom::Client> binding_{this};
 
   DISALLOW_COPY_AND_ASSIGN(FakeAssistantClient);
 };
@@ -194,6 +201,11 @@ class AssistantServiceTest : public testing::Test {
         base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
             &url_loader_factory_);
 
+    assistant_state()->NotifyArcPlayStoreEnabledChanged(true);
+    assistant_state()->NotifyFeatureAllowed(
+        ash::mojom::AssistantAllowedState::ALLOWED);
+    assistant_state()->NotifyLocaleChanged("en_US");
+
     auto fake_pref_connection = std::make_unique<FakePrefConnectionDelegate>();
     pref_service_ = fake_pref_connection->pref_service();
 
@@ -253,7 +265,7 @@ class AssistantServiceTest : public testing::Test {
   std::unique_ptr<Service> service_;
   mojo::Remote<mojom::AssistantService> remote_service_;
 
-  FullyInitializedAssistantState assistant_state_;
+  ash::AssistantState assistant_state_;
 
   FakeIdentityAccessor fake_identity_accessor_;
   FakeAssistantClient fake_assistant_client_{&assistant_state_};
@@ -330,19 +342,19 @@ TEST_F(AssistantServiceTest, StopImmediatelyIfAssistantIsRunning) {
 }
 
 TEST_F(AssistantServiceTest, StopDelayedIfAssistantNotFinishedStarting) {
-  // Test is set up as |State::STARTING|, turning settings off will trigger
+  // Test is set up as |State::STARTED|, turning settings off will trigger
   // logic to try to stop it.
   pref_service()->SetBoolean(prefs::kAssistantEnabled, false);
 
   EXPECT_EQ(assistant_manager()->GetState(),
-            AssistantManagerService::State::STARTING);
+            AssistantManagerService::State::STARTED);
 
   mock_task_runner()->FastForwardBy(kUpdateAssistantManagerDelay);
   base::RunLoop().RunUntilIdle();
 
   // No change of state because it is still starting.
   EXPECT_EQ(assistant_manager()->GetState(),
-            AssistantManagerService::State::STARTING);
+            AssistantManagerService::State::STARTED);
 
   assistant_manager()->FinishStart();
 

chromeos/services/assistant/test_support/fake_assistant_manager_service_delegate.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chromeos/services/assistant/test_support/fake_assistant_manager_service_delegate.h"
-
-#include "chromeos/assistant/internal/test_support/fake_assistant_manager.h"
-#include "chromeos/assistant/internal/test_support/fake_assistant_manager_internal.h"
-#include "chromeos/services/assistant/test_support/fake_platform_api.h"
-
-namespace chromeos {
-namespace assistant {
-
-FakeAssistantManagerServiceDelegate::FakeAssistantManagerServiceDelegate()
-    : assistant_manager_(std::make_unique<FakeAssistantManager>()),
-      assistant_manager_internal_(
-          std::make_unique<FakeAssistantManagerInternal>()),
-      assistant_manager_ptr_(assistant_manager_.get()) {}
-
-FakeAssistantManagerServiceDelegate::~FakeAssistantManagerServiceDelegate() =
-    default;
-
-std::unique_ptr<CrosPlatformApi>
-FakeAssistantManagerServiceDelegate::CreatePlatformApi(
-    AssistantMediaSession* media_session,
-    scoped_refptr<base::SingleThreadTaskRunner> background_thread_task_runner) {
-  return std::make_unique<FakePlatformApi>();
-}
-
-std::unique_ptr<assistant_client::AssistantManager>
-FakeAssistantManagerServiceDelegate::CreateAssistantManager(
-    assistant_client::PlatformApi* platform_api,
-    const std::string& lib_assistant_config) {
-  DCHECK(assistant_manager_);
-  return std::move(assistant_manager_);
-}
-
-assistant_client::AssistantManagerInternal*
-FakeAssistantManagerServiceDelegate::UnwrapAssistantManagerInternal(
-    assistant_client::AssistantManager* assistant_manager) {
-  return assistant_manager_internal_.get();
-}
-
-}  // namespace assistant
-}  // namespace chromeos

chromeos/services/assistant/test_support/fake_assistant_manager_service_delegate.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FAKE_ASSISTANT_MANAGER_SERVICE_DELEGATE_H_
-#define CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FAKE_ASSISTANT_MANAGER_SERVICE_DELEGATE_H_
-
-#include "chromeos/services/assistant/assistant_manager_service_delegate.h"
-
-namespace chromeos {
-namespace assistant {
-
-class FakeAssistantManager;
-class FakeAssistantManagerInternal;
-
-// Implementation of |AssistantManagerServiceDelegate| that returns fake
-// instances for all of the member methods. Used during unittests.
-class FakeAssistantManagerServiceDelegate
-    : public AssistantManagerServiceDelegate {
- public:
-  FakeAssistantManagerServiceDelegate();
-  ~FakeAssistantManagerServiceDelegate() override;
-
-  FakeAssistantManager* assistant_manager() { return assistant_manager_ptr_; }
-
-  FakeAssistantManagerInternal* assistant_manager_internal() {
-    return assistant_manager_internal_.get();
-  }
-
-  // AssistantManagerServiceDelegate:
-  std::unique_ptr<CrosPlatformApi> CreatePlatformApi(
-      AssistantMediaSession* media_session,
-      scoped_refptr<base::SingleThreadTaskRunner> background_thread_task_runner)
-      override;
-  std::unique_ptr<assistant_client::AssistantManager> CreateAssistantManager(
-      assistant_client::PlatformApi* platform_api,
-      const std::string& lib_assistant_config) override;
-  assistant_client::AssistantManagerInternal* UnwrapAssistantManagerInternal(
-      assistant_client::AssistantManager* assistant_manager) override;
-
- private:
-  // Will be initialized in the constructor and moved out when
-  // |CreateAssistantManager| is called.
-  std::unique_ptr<FakeAssistantManager> assistant_manager_;
-  std::unique_ptr<FakeAssistantManagerInternal> assistant_manager_internal_;
-  FakeAssistantManager* assistant_manager_ptr_;
-
-  DISALLOW_COPY_AND_ASSIGN(FakeAssistantManagerServiceDelegate);
-};
-
-}  // namespace assistant
-}  // namespace chromeos
-
-#endif  // CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FAKE_ASSISTANT_MANAGER_SERVICE_DELEGATE_H_

chromeos/services/assistant/test_support/fake_platform_api.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chromeos/services/assistant/test_support/fake_platform_api.h"
-#include "base/logging.h"
-
-namespace chromeos {
-namespace assistant {
-
-assistant_client::AudioInputProvider& FakePlatformApi::GetAudioInputProvider() {
-  NOTIMPLEMENTED();
-  abort();
-}
-
-assistant_client::AudioOutputProvider&
-FakePlatformApi::GetAudioOutputProvider() {
-  NOTIMPLEMENTED();
-  abort();
-}
-
-assistant_client::AuthProvider& FakePlatformApi::GetAuthProvider() {
-  NOTIMPLEMENTED();
-  abort();
-}
-
-assistant_client::FileProvider& FakePlatformApi::GetFileProvider() {
-  NOTIMPLEMENTED();
-  abort();
-}
-
-assistant_client::NetworkProvider& FakePlatformApi::GetNetworkProvider() {
-  NOTIMPLEMENTED();
-  abort();
-}
-
-assistant_client::SystemProvider& FakePlatformApi::GetSystemProvider() {
-  NOTIMPLEMENTED();
-  abort();
-}
-
-}  // namespace assistant
-}  // namespace chromeos

chromeos/services/assistant/test_support/fake_platform_api.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FAKE_PLATFORM_API_H_
-#define CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FAKE_PLATFORM_API_H_
-
-#include "base/macros.h"
-#include "chromeos/services/assistant/cros_platform_api.h"
-
-namespace chromeos {
-namespace assistant {
-
-// Fake implementation of the |CrosPlatformApi| used during the unittests.
-// As of now the |assistant_client::PlatformApi| methods are not implemented
-// and will assert when called.
-class FakePlatformApi : public CrosPlatformApi {
- public:
-  FakePlatformApi() = default;
-  ~FakePlatformApi() override = default;
-
-  // CrosPlatformApi overrides
-  assistant_client::AudioInputProvider& GetAudioInputProvider() override;
-  assistant_client::AudioOutputProvider& GetAudioOutputProvider() override;
-  assistant_client::AuthProvider& GetAuthProvider() override;
-  assistant_client::FileProvider& GetFileProvider() override;
-  assistant_client::NetworkProvider& GetNetworkProvider() override;
-  assistant_client::SystemProvider& GetSystemProvider() override;
-  void SetMicState(bool mic_open) override {}
-  void OnHotwordEnabled(bool enable) override {}
-  void OnConversationTurnStarted() override {}
-  void OnConversationTurnFinished() override {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(FakePlatformApi);
-};
-
-}  // namespace assistant
-}  // namespace chromeos
-
-#endif  // CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FAKE_PLATFORM_API_H_

chromeos/services/assistant/test_support/fully_initialized_assistant_state.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chromeos/services/assistant/test_support/fully_initialized_assistant_state.h"
-
-namespace chromeos {
-namespace assistant {
-
-FullyInitializedAssistantState::FullyInitializedAssistantState() {
-  InitializeAllValues();
-}
-
-void FullyInitializedAssistantState::InitializeAllValues() {
-  settings_enabled_ = true;
-  consent_status_ =
-      chromeos::assistant::prefs::ConsentStatus::kActivityControlAccepted;
-  context_enabled_ = true;
-  hotword_enabled_ = true;
-  hotword_always_on_ = true;
-  launch_with_mic_open_ = true;
-  notification_enabled_ = true;
-  allowed_state_ = ash::mojom::AssistantAllowedState::ALLOWED;
-  locale_ = "en_US";
-  arc_play_store_enabled_ = true;
-  locked_full_screen_enabled_ = true;
-}
-
-}  // namespace assistant
-}  // namespace chromeos

chromeos/services/assistant/test_support/fully_initialized_assistant_state.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FULLY_INITIALIZED_ASSISTANT_STATE_H_
-#define CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FULLY_INITIALIZED_ASSISTANT_STATE_H_
-
-#include "ash/public/cpp/assistant/assistant_state.h"
-
-namespace chromeos {
-namespace assistant {
-
-// Instance of |AssistantState| where every base::Optional value has a non-null
-// value. All values will be set to their equivalent of enabled.
-class FullyInitializedAssistantState : public ash::AssistantState {
- public:
-  FullyInitializedAssistantState();
-  ~FullyInitializedAssistantState() override = default;
-
- private:
-  void InitializeAllValues();
-
-  DISALLOW_COPY_AND_ASSIGN(FullyInitializedAssistantState);
-};
-
-}  // namespace assistant
-}  // namespace chromeos
-
-#endif  // CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_FULLY_INITIALIZED_ASSISTANT_STATE_H_

chromeos/services/assistant/test_support/mock_media_manager.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chromeos/services/assistant/test_support/mock_media_manager.h"
-
-namespace chromeos {
-namespace assistant {
-
-MockMediaManager::MockMediaManager() = default;
-MockMediaManager::~MockMediaManager() = default;
-
-}  // namespace assistant
-}  // namespace chromeos

chromeos/services/assistant/test_support/mock_media_manager.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_MOCK_MEDIA_MANAGER_H_
-#define CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_MOCK_MEDIA_MANAGER_H_
-
-#include "libassistant/shared/public/media_manager.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-namespace chromeos {
-namespace assistant {
-
-class MockMediaManager : public assistant_client::MediaManager {
- public:
-  MockMediaManager();
-  ~MockMediaManager() override;
-
-  // MediaManager:
-  MOCK_METHOD1(AddListener, void(Listener*));
-  MOCK_METHOD0(Next, void());
-  MOCK_METHOD0(Previous, void());
-  MOCK_METHOD0(Pause, void());
-  MOCK_METHOD0(Resume, void());
-  MOCK_METHOD0(StopAndClearPlaylist, void());
-  MOCK_METHOD0(PlayPause, void());
-  MOCK_METHOD1(SetExternalPlaybackState,
-               void(const assistant_client::MediaStatus&));
-};
-
-}  // namespace assistant
-}  // namespace chromeos
-
-#endif  // CHROMEOS_SERVICES_ASSISTANT_TEST_SUPPORT_MOCK_MEDIA_MANAGER_H_