Remove external dependencies from |AudioInputImpl|

This will allow us to easier move this class to the Libassistant mojm
service.

All dependencies have been moved to the new class |AudioInputHost|.
This class currently talks directly to |AudioInputImpl|, but later
it will instead talk to the mojom APIs.

Some notes:
   - |AudioInputImpl| still depends on
     |AssistantClient::Get()->RequestAudioStreamFactory()|, because the
     Libassistant mojom service will also use an audio stream factory to
     open/close its audio input. In a follow up CL I will introduce a
     delegate to sever this dependency.
   - There is one behavior change:
     |power_manager_client_->NotifyWakeNotification()| was previously
     only called when DSP was enabled, and it is now always called (when
     Libassistant starts a conversation). I can think of no reason why
     this would be an issue.

This is a reland of crrev.com/c/2582537, with the added fix that we no
longer crash when we get a request to set an empty DSP hotword locale.

Bug: b/171748795
Test: chromeos_unittests, deployed on real hardware with and without DSP.
Change-Id: I9ace389ad27e44cafe422d668946bf29ac7f56a9
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2596238
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Auto-Submit: Jeroen Dhollander <jeroendh@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837899}

chromeos/services/assistant/BUILD.gn

@@ -80,6 +80,8 @@ component("lib") {
       "media_session/assistant_media_session.h",
       "platform/audio_device_owner.cc",
       "platform/audio_device_owner.h",
+      "platform/audio_input_host.cc",
+      "platform/audio_input_host.h",
       "platform/audio_input_impl.cc",
       "platform/audio_input_impl.h",
       "platform/audio_input_provider_impl.cc",
@@ -153,6 +155,7 @@ source_set("tests") {
     "//chromeos/audio",
     "//chromeos/constants",
     "//chromeos/dbus:test_support",
+    "//chromeos/dbus/audio",
     "//chromeos/dbus/power",
     "//chromeos/services/assistant/public/cpp",
     "//chromeos/services/assistant/public/mojom",

chromeos/services/assistant/platform/audio_input_host.cc

+// Copyright 2020 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/platform/audio_input_host.h"
+
+#include "base/check.h"
+#include "base/metrics/histogram_functions.h"
+#include "base/optional.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "chromeos/audio/cras_audio_handler.h"
+#include "chromeos/services/assistant/platform/audio_input_impl.h"
+#include "chromeos/services/assistant/public/cpp/features.h"
+
+namespace chromeos {
+namespace assistant {
+
+namespace {
+
+constexpr const char kDefaultLocale[] = "en_us";
+
+AudioInputImpl::LidState ConvertLidState(
+    chromeos::PowerManagerClient::LidState state) {
+  switch (state) {
+    case chromeos::PowerManagerClient::LidState::CLOSED:
+      return AudioInputImpl::LidState::kClosed;
+    case chromeos::PowerManagerClient::LidState::OPEN:
+      return AudioInputImpl::LidState::kOpen;
+    case chromeos::PowerManagerClient::LidState::NOT_PRESENT:
+      // If there is no lid, it can't be closed.
+      return AudioInputImpl::LidState::kOpen;
+  }
+}
+
+// Hotword model is expected to have <language>_<region> format with lower
+// case, while the locale in pref is stored as <language>-<region> with region
+// code in capital letters. So we need to convert the pref locale to the
+// correct format.
+// Examples:
+//     "fr"     ->  "fr_fr"
+//     "nl-BE"  ->  "nl_be"
+base::Optional<std::string> ToHotwordModel(std::string pref_locale) {
+  std::vector<std::string> code_strings = base::SplitString(
+      pref_locale, "-", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+
+  if (code_strings.size() == 0) {
+    // Note: I am not sure this happens during real operations, but it
+    // definitely happens during the ChromeOS performance tests.
+    return base::nullopt;
+  }
+
+  DCHECK_LT(code_strings.size(), 3u);
+
+  // For locales with language code "en", use "en_all" hotword model.
+  if (code_strings[0] == "en")
+    return "en_all";
+
+  // If the language code and country code happen to be the same, e.g.
+  // France (FR) and French (fr), the locale will be stored as "fr" instead
+  // of "fr-FR" in the profile on Chrome OS.
+  if (code_strings.size() == 1)
+    return code_strings[0] + "_" + code_strings[0];
+
+  return code_strings[0] + "_" + base::ToLowerASCII(code_strings[1]);
+}
+
+}  // namespace
+
+chromeos::assistant::AudioInputHost::AudioInputHost(
+    AudioInputImpl* audio_input,
+    CrasAudioHandler* cras_audio_handler,
+    chromeos::PowerManagerClient* power_manager_client)
+    : audio_input_(audio_input),
+      cras_audio_handler_(cras_audio_handler),
+      power_manager_client_(power_manager_client),
+      power_manager_client_observer_(this) {
+  DCHECK(audio_input_);
+  DCHECK(cras_audio_handler_);
+  DCHECK(power_manager_client_);
+
+  power_manager_client_observer_.Observe(power_manager_client);
+  power_manager_client->GetSwitchStates(base::BindOnce(
+      &AudioInputHost::OnInitialLidStateReceived, weak_factory_.GetWeakPtr()));
+}
+
+AudioInputHost::~AudioInputHost() = default;
+
+void AudioInputHost::SetMicState(bool mic_open) {
+  audio_input_->SetMicState(mic_open);
+}
+
+void AudioInputHost::SetDeviceId(const std::string& device_id) {
+  audio_input_->SetDeviceId(device_id);
+}
+
+void AudioInputHost::OnConversationTurnStarted() {
+  audio_input_->OnConversationTurnStarted();
+  // Inform power manager of a wake notification when Libassistant
+  // recognized hotword and started a conversation. We intentionally
+  // avoid using |NotifyUserActivity| because it is not suitable for
+  // this case according to the Platform team.
+  power_manager_client_->NotifyWakeNotification();
+}
+
+void AudioInputHost::OnConversationTurnFinished() {
+  audio_input_->OnConversationTurnFinished();
+}
+
+void AudioInputHost::OnHotwordEnabled(bool enable) {
+  audio_input_->OnHotwordEnabled(enable);
+}
+
+void AudioInputHost::SetHotwordDeviceId(const std::string& device_id) {
+  hotword_device_id_ = device_id;
+  audio_input_->SetHotwordDeviceId(device_id);
+}
+
+void AudioInputHost::SetDspHotwordLocale(std::string pref_locale) {
+  if (!features::IsDspHotwordEnabled())
+    return;
+
+  std::string hotword_model =
+      ToHotwordModel(pref_locale).value_or(kDefaultLocale);
+
+  cras_audio_handler_->SetHotwordModel(
+      GetDspNodeId(), hotword_model,
+      base::BindOnce(&AudioInputHost::SetDspHotwordLocaleCallback,
+                     weak_factory_.GetWeakPtr(), hotword_model));
+}
+
+void AudioInputHost::SetDspHotwordLocaleCallback(std::string pref_locale,
+                                                 bool success) {
+  base::UmaHistogramBoolean("Assistant.SetDspHotwordLocale", success);
+  if (success)
+    return;
+
+  LOG(ERROR) << "Set " << pref_locale
+             << " hotword model failed, fallback to default locale.";
+  // Reset the locale to the default value if we failed to sync it to the locale
+  // stored in user's pref.
+  cras_audio_handler_->SetHotwordModel(
+      GetDspNodeId(), /* hotword_model */ kDefaultLocale,
+      base::BindOnce([](bool success) {
+        if (!success)
+          LOG(ERROR) << "Reset to default hotword model failed.";
+      }));
+}
+
+uint64_t AudioInputHost::GetDspNodeId() const {
+  DCHECK(!hotword_device_id_.empty());
+  uint64_t result;
+  bool success = base::StringToUint64(hotword_device_id_, &result);
+  DCHECK(success) << "Invalid hotword device id '" << hotword_device_id_ << "'";
+  return result;
+}
+
+void AudioInputHost::LidEventReceived(
+    chromeos::PowerManagerClient::LidState state,
+    base::TimeTicks timestamp) {
+  // Lid switch event still gets fired during system suspend, which enables
+  // us to stop DSP recording correctly when user closes lid after the device
+  // goes to sleep.
+  audio_input_->OnLidStateChanged(ConvertLidState(state));
+}
+
+void AudioInputHost::OnInitialLidStateReceived(
+    base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states) {
+  if (switch_states.has_value())
+    audio_input_->OnLidStateChanged(ConvertLidState(switch_states->lid_state));
+}
+
+}  // namespace assistant
+}  // namespace chromeos

chromeos/services/assistant/platform/audio_input_host.h

+// Copyright 2020 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_PLATFORM_AUDIO_INPUT_HOST_H_
+#define CHROMEOS_SERVICES_ASSISTANT_PLATFORM_AUDIO_INPUT_HOST_H_
+
+#include <string>
+
+#include "base/component_export.h"
+#include "base/memory/weak_ptr.h"
+#include "base/scoped_observation.h"
+#include "chromeos/dbus/power/power_manager_client.h"
+
+namespace chromeos {
+class CrasAudioHandler;
+}  // namespace chromeos
+
+namespace chromeos {
+namespace assistant {
+
+class AudioInputImpl;
+
+// Class that provides the bridge between the ChromeOS UI thread and the
+// Libassistant audio input class.
+// The goal is that |AudioInputImpl| no longer depends on any external events.
+// This will allow us to move it to the Libassistant mojom service (at which
+// point this class will talk to the Libassistant mojom service).
+class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputHost
+    : public chromeos::PowerManagerClient::Observer {
+ public:
+  AudioInputHost(AudioInputImpl* audio_input,
+                 CrasAudioHandler* cras_audio_handler,
+                 chromeos::PowerManagerClient* power_manager_client);
+  AudioInputHost(AudioInputHost&) = delete;
+  AudioInputHost& operator=(AudioInputHost&) = delete;
+  ~AudioInputHost() override;
+
+  // Called when the mic state associated with the interaction is changed.
+  void SetMicState(bool mic_open);
+
+  // Setting the input device to use for audio capture.
+  void SetDeviceId(const std::string& device_id);
+
+  // Called when hotword enabled status changed.
+  void OnHotwordEnabled(bool enable);
+
+  // Setting the hotword input device with hardware based hotword detection.
+  void SetHotwordDeviceId(const std::string& device_id);
+
+  // Setting the hotword locale for the input device with DSP support.
+  void SetDspHotwordLocale(std::string pref_locale);
+
+  void OnConversationTurnStarted();
+  void OnConversationTurnFinished();
+
+ private:
+  void SetDspHotwordLocaleCallback(std::string pref_locale, bool success);
+
+  uint64_t GetDspNodeId() const;
+
+  // chromeos::PowerManagerClient::Observer overrides:
+  void LidEventReceived(chromeos::PowerManagerClient::LidState state,
+                        base::TimeTicks timestamp) override;
+
+  void OnInitialLidStateReceived(
+      base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states);
+
+  // Owned by |PlatformApiImpl| which also owns |this|.
+  AudioInputImpl* const audio_input_;
+  CrasAudioHandler* const cras_audio_handler_;
+  chromeos::PowerManagerClient* const power_manager_client_;
+  base::ScopedObservation<chromeos::PowerManagerClient,
+                          chromeos::PowerManagerClient::Observer>
+      power_manager_client_observer_;
+
+  // Hotword input device used for hardware based hotword detection.
+  std::string hotword_device_id_;
+
+  base::WeakPtrFactory<AudioInputHost> weak_factory_{this};
+};
+
+}  // namespace assistant
+}  // namespace chromeos
+
+#endif  // CHROMEOS_SERVICES_ASSISTANT_PLATFORM_AUDIO_INPUT_HOST_H_

chromeos/services/assistant/platform/audio_input_impl.cc

@@ -12,8 +12,6 @@
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/timer/timer.h"
-#include "chromeos/audio/cras_audio_handler.h"
-#include "chromeos/dbus/power/power_manager_client.h"
 #include "chromeos/services/assistant/buildflags.h"
 #include "chromeos/services/assistant/public/cpp/assistant_client.h"
 #include "chromeos/services/assistant/public/cpp/features.h"
@@ -61,13 +59,9 @@ media::ChannelLayout GetChannelLayout(
 class DspHotwordStateManager : public AudioInputImpl::HotwordStateManager {
  public:
   DspHotwordStateManager(AudioInputImpl* input,
-                         scoped_refptr<base::SequencedTaskRunner> task_runner,
-                         chromeos::PowerManagerClient* power_manager_client)
-      : AudioInputImpl::HotwordStateManager(input),
-        task_runner_(task_runner),
-        power_manager_client_(power_manager_client) {
+                         scoped_refptr<base::SequencedTaskRunner> task_runner)
+      : AudioInputImpl::HotwordStateManager(input), task_runner_(task_runner) {
     DCHECK(task_runner_->RunsTasksInCurrentSequence());
-    DCHECK(power_manager_client_);
   }
 
   // HotwordStateManager overrides:
@@ -82,12 +76,6 @@ class DspHotwordStateManager : public AudioInputImpl::HotwordStateManager {
       input_->RecreateAudioInputStream(false /* use_dsp */);
     }
     stream_state_ = StreamState::NORMAL;
-
-    // Inform power manager of a wake notification when Libassistant
-    // recognized hotword and started a conversation. We intentionally
-    // avoid using |NotifyUserActivity| because it is not suitable for
-    // this case according to the Platform team.
-    power_manager_client_->NotifyWakeNotification();
   }
 
   // Runs on main thread.
@@ -155,7 +143,6 @@ class DspHotwordStateManager : public AudioInputImpl::HotwordStateManager {
   }
 
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
-  chromeos::PowerManagerClient* power_manager_client_;
   StreamState stream_state_ = StreamState::HOTWORD;
   base::OneShotTimer second_phase_timer_;
   base::WeakPtrFactory<DspHotwordStateManager> weak_factory_{this};
@@ -196,22 +183,12 @@ void AudioInputImpl::HotwordStateManager::RecreateAudioInputStream() {
   input_->RecreateAudioInputStream(/*use_dsp=*/false);
 }
 
-AudioInputImpl::AudioInputImpl(PowerManagerClient* power_manager_client,
-                               CrasAudioHandler* cras_audio_handler,
-                               const std::string& device_id)
-    : power_manager_client_(power_manager_client),
-      power_manager_client_observer_(this),
-      cras_audio_handler_(cras_audio_handler),
-      task_runner_(base::SequencedTaskRunnerHandle::Get()),
+AudioInputImpl::AudioInputImpl(const std::string& device_id)
+    : task_runner_(base::SequencedTaskRunnerHandle::Get()),
       preferred_device_id_(device_id),
       weak_factory_(this) {
   DETACH_FROM_SEQUENCE(observer_sequence_checker_);
 
-  DCHECK(power_manager_client);
-  power_manager_client_observer_.Add(power_manager_client);
-  power_manager_client->GetSwitchStates(base::BindOnce(
-      &AudioInputImpl::OnSwitchStatesReceived, weak_factory_.GetWeakPtr()));
-
   RecreateStateManager();
   if (features::IsStereoAudioInputEnabled())
     g_current_format = kFormatStereo;
@@ -226,8 +203,8 @@ AudioInputImpl::~AudioInputImpl() {
 
 void AudioInputImpl::RecreateStateManager() {
   if (IsHotwordAvailable()) {
-    state_manager_ = std::make_unique<DspHotwordStateManager>(
-        this, task_runner_, power_manager_client_);
+    state_manager_ =
+        std::make_unique<DspHotwordStateManager>(this, task_runner_);
   } else {
     state_manager_ = std::make_unique<HotwordStateManager>(this);
   }
@@ -331,19 +308,6 @@ void AudioInputImpl::RemoveObserver(
   }
 }
 
-void AudioInputImpl::LidEventReceived(
-    chromeos::PowerManagerClient::LidState state,
-    base::TimeTicks timestamp) {
-  // Lid switch event still gets fired during system suspend, which enables
-  // us to stop DSP recording correctly when user closes lid after the device
-  // goes to sleep.
-  DCHECK(task_runner_->RunsTasksInCurrentSequence());
-  if (state != lid_state_) {
-    lid_state_ = state;
-    UpdateRecordingState();
-  }
-}
-
 void AudioInputImpl::SetMicState(bool mic_open) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
   if (mic_open_ == mic_open)
@@ -394,58 +358,14 @@ void AudioInputImpl::SetHotwordDeviceId(const std::string& device_id) {
     state_manager_->RecreateAudioInputStream();
 }
 
-void AudioInputImpl::SetDspHotwordLocale(std::string pref_locale) {
-  DCHECK(!hotword_device_id_.empty());
-  // SetHotwordModel will fail if hotword streaming is running.
-  DCHECK(!source_);
-
-  if (!features::IsDspHotwordEnabled())
-    return;
-
-  // Hotword model is expected to have <language>_<region> format with lower
-  // case, while the locale in pref is stored as <language>-<region> with region
-  // code in capital letters. So we need to convert the pref locale to the
-  // correct format.
-  if (!base::ReplaceChars(pref_locale, "-", "_", &pref_locale)) {
-    // If the language code and country code happen to be the same, e.g.
-    // France (FR) and French (fr), the locale will be stored as "fr" instead
-    // of "fr-FR" in the profile on Chrome OS.
-    std::string region_code = pref_locale;
-    pref_locale.append("_").append(region_code);
+void AudioInputImpl::OnLidStateChanged(LidState new_state) {
+  // Lid switch event still gets fired during system suspend, which enables
+  // us to stop DSP recording correctly when user closes lid after the device
+  // goes to sleep.
+  if (new_state != lid_state_) {
+    lid_state_ = new_state;
+    UpdateRecordingState();
   }
-
-  // For locales with language code "en", use "en_all" hotword model.
-  std::vector<std::string> code_strings = base::SplitString(
-      pref_locale, "_", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
-  if (code_strings.size() > 0 && code_strings[0] == "en")
-    pref_locale = "en_all";
-
-  uint64_t dsp_node_id;
-  base::StringToUint64(hotword_device_id_, &dsp_node_id);
-  cras_audio_handler_->SetHotwordModel(
-      dsp_node_id, /* hotword_model */ base::ToLowerASCII(pref_locale),
-      base::BindOnce(&AudioInputImpl::SetDspHotwordLocaleCallback,
-                     weak_factory_.GetWeakPtr(), pref_locale));
-}
-
-void AudioInputImpl::SetDspHotwordLocaleCallback(std::string pref_locale,
-                                                 bool success) {
-  base::UmaHistogramBoolean("Assistant.SetDspHotwordLocale", success);
-  if (success)
-    return;
-
-  LOG(ERROR) << "Set " << pref_locale
-             << " hotword model failed, fallback to default locale.";
-  // Reset the locale to the default value "en_us" if we failed to sync it to
-  // the locale stored in user's pref.
-  uint64_t dsp_node_id;
-  base::StringToUint64(hotword_device_id_, &dsp_node_id);
-  cras_audio_handler_->SetHotwordModel(
-      dsp_node_id, /* hotword_model */ "en_us",
-      base::BindOnce([](bool success) {
-        if (!success)
-          LOG(ERROR) << "Reset to default hotword model failed.";
-      }));
 }
 
 void AudioInputImpl::RecreateAudioInputStream(bool use_dsp) {
@@ -520,15 +440,6 @@ void AudioInputImpl::StopRecording() {
   }
 }
 
-void AudioInputImpl::OnSwitchStatesReceived(
-    base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states) {
-  DCHECK(task_runner_->RunsTasksInCurrentSequence());
-  if (switch_states.has_value()) {
-    lid_state_ = switch_states->lid_state;
-    UpdateRecordingState();
-  }
-}
-
 void AudioInputImpl::UpdateRecordingState() {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
@@ -538,8 +449,7 @@ void AudioInputImpl::UpdateRecordingState() {
     has_observers = observers_.size() > 0;
   }
 
-  bool is_lid_closed =
-      lid_state_ == chromeos::PowerManagerClient::LidState::CLOSED;
+  bool is_lid_closed = (lid_state_ == LidState::kClosed);
   bool should_enable_hotword =
       hotword_enabled_ && (!preferred_device_id_.empty());
   bool should_start =

chromeos/services/assistant/platform/audio_input_impl.h

@@ -12,28 +12,26 @@
 #include "base/component_export.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
-#include "base/scoped_observer.h"
 #include "base/sequence_checker.h"
 #include "base/synchronization/lock.h"
 #include "base/time/time.h"
-#include "chromeos/dbus/power/power_manager_client.h"
 #include "chromeos/services/assistant/public/cpp/assistant_service.h"
 #include "libassistant/shared/public/platform_audio_input.h"
 #include "media/base/audio_capturer_source.h"
 
 namespace chromeos {
-class CrasAudioHandler;
-
 namespace assistant {
 
 class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
     : public assistant_client::AudioInput,
-      public media::AudioCapturerSource::CaptureCallback,
-      public chromeos::PowerManagerClient::Observer {
+      public media::AudioCapturerSource::CaptureCallback {
  public:
-  AudioInputImpl(PowerManagerClient* power_manager_client,
-                 CrasAudioHandler* cras_audio_handler,
-                 const std::string& device_id);
+  enum class LidState {
+    kOpen,
+    kClosed,
+  };
+
+  explicit AudioInputImpl(const std::string& device_id);
   ~AudioInputImpl() override;
 
   class HotwordStateManager {
@@ -70,10 +68,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
   void RemoveObserver(
       assistant_client::AudioInput::Observer* observer) override;
 
-  // chromeos::PowerManagerClient::Observer overrides:
-  void LidEventReceived(chromeos::PowerManagerClient::LidState state,
-                        base::TimeTicks timestamp) override;
-
   // Called when the mic state associated with the interaction is changed.
   void SetMicState(bool mic_open);
   void OnConversationTurnStarted();
@@ -84,8 +78,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
 
   void SetDeviceId(const std::string& device_id);
   void SetHotwordDeviceId(const std::string& device_id);
-  void SetDspHotwordLocale(std::string pref_locale);
-  void SetDspHotwordLocaleCallback(std::string pref_locale, bool success);
+
+  // Called when the user opens/closes the lid.
+  void OnLidStateChanged(LidState new_state);
 
   void RecreateAudioInputStream(bool use_dsp);
 
@@ -101,10 +96,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
   void StopRecording();
   void UpdateRecordingState();
 
-  // Updates lid state from received switch states.
-  void OnSwitchStatesReceived(
-      base::Optional<chromeos::PowerManagerClient::SwitchStates> switch_states);
-
   scoped_refptr<media::AudioCapturerSource> source_;
 
   // User explicitly requested to open microphone.
@@ -128,13 +119,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
   // sequence.
   SEQUENCE_CHECKER(observer_sequence_checker_);
 
-  chromeos::PowerManagerClient* power_manager_client_;
-  ScopedObserver<chromeos::PowerManagerClient,
-                 chromeos::PowerManagerClient::Observer>
-      power_manager_client_observer_;
-
-  CrasAudioHandler* const cras_audio_handler_;
-
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
   std::unique_ptr<HotwordStateManager> state_manager_;
@@ -146,8 +130,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AudioInputImpl
   // Device currently being used for recording.
   std::string device_id_;
 
-  chromeos::PowerManagerClient::LidState lid_state_ =
-      chromeos::PowerManagerClient::LidState::NOT_PRESENT;
+  // Start with lidstate |kClosed| so we do not open the microphone before we
+  // know if the lid is open or closed.
+  LidState lid_state_ = LidState::kClosed;
 
   base::WeakPtrFactory<AudioInputImpl> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(AudioInputImpl);

chromeos/services/assistant/platform/audio_input_impl_unittest.cc

@@ -11,10 +11,13 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "chromeos/audio/cras_audio_handler.h"
+#include "chromeos/dbus/audio/fake_cras_audio_client.h"
 #include "chromeos/dbus/power/fake_power_manager_client.h"
+#include "chromeos/services/assistant/platform/audio_input_host.h"
 #include "chromeos/services/assistant/public/cpp/features.h"
 #include "chromeos/services/assistant/test_support/scoped_assistant_client.h"
 #include "services/audio/public/cpp/fake_stream_factory.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
@@ -22,6 +25,9 @@ namespace assistant {
 
 namespace {
 
+using LidState = chromeos::PowerManagerClient::LidState;
+using ::testing::_;
+
 class ScopedFakeAssistantClient : public ScopedAssistantClient {
  public:
   ScopedFakeAssistantClient() = default;
@@ -40,6 +46,24 @@ class ScopedFakeAssistantClient : public ScopedAssistantClient {
   DISALLOW_COPY_AND_ASSIGN(ScopedFakeAssistantClient);
 };
 
+// Mock for |CrosAudioClient|. This inherits from |FakeCrasAudioClient| so we
+// only have to mock the methods we're interested in.
+// It will automatically be installed as the global singleton in its
+// constructor, and removed in the destructor.
+class ScopedCrasAudioClientMock : public FakeCrasAudioClient {
+ public:
+  ScopedCrasAudioClientMock() = default;
+  ScopedCrasAudioClientMock(ScopedCrasAudioClientMock&) = delete;
+  ScopedCrasAudioClientMock& operator=(ScopedCrasAudioClientMock&) = delete;
+  ~ScopedCrasAudioClientMock() override = default;
+
+  MOCK_METHOD(void,
+              SetHotwordModel,
+              (uint64_t node_id,
+               const std::string& hotword_model,
+               VoidDBusMethodCallback callback));
+};
+
 }  // namespace
 
 class AudioInputImplTest : public testing::Test,
@@ -52,16 +76,15 @@ class AudioInputImplTest : public testing::Test,
     PowerManagerClient::InitializeFake();
     CrasAudioHandler::InitializeForTesting();
 
-    audio_input_impl_ = std::make_unique<AudioInputImpl>(
-        FakePowerManagerClient::Get(), CrasAudioHandler::Get(),
-        "fake-device-id");
-
-    audio_input_impl_->AddObserver(this);
+    CreateNewAudioInputImpl();
   }
 
   ~AudioInputImplTest() override {
     audio_input_impl_->RemoveObserver(this);
     audio_input_impl_.reset();
+    // |audio_input_host_| uses the fake power manager client, so must be
+    // destroyed before the power manager client.
+    audio_input_host_.reset();
     CrasAudioHandler::Shutdown();
     chromeos::PowerManagerClient::Shutdown();
   }
@@ -74,8 +97,33 @@ class AudioInputImplTest : public testing::Test,
     return audio_input_impl_->IsUsingHotwordDeviceForTesting();
   }
 
+  void CreateNewAudioInputImpl() {
+    audio_input_impl_ = std::make_unique<AudioInputImpl>("fake-device-id");
+
+    audio_input_host_ = std::make_unique<AudioInputHost>(
+        audio_input_impl_.get(), CrasAudioHandler::Get(),
+        FakePowerManagerClient::Get());
+
+    audio_input_impl_->AddObserver(this);
+
+    // Allow the asynchronous triggered events to run.
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void StopAudioRecording() {
+    SetLidState(LidState::CLOSED);
+    // Allow the asynchronous triggered events to run.
+    base::RunLoop().RunUntilIdle();
+  }
+
   AudioInputImpl* audio_input_impl() { return audio_input_impl_.get(); }
 
+  AudioInputHost& audio_input_host() { return *audio_input_host_; }
+
+  ScopedCrasAudioClientMock& cras_audio_client_mock() {
+    return cras_audio_client_mock_;
+  }
+
   // assistant_client::AudioInput::Observer overrides:
   void OnAudioBufferAvailable(const assistant_client::AudioBuffer& buffer,
                               int64_t timestamp) override {}
@@ -83,37 +131,46 @@ class AudioInputImplTest : public testing::Test,
   void OnAudioStopped() override {}
 
  protected:
-  void ReportLidEvent(chromeos::PowerManagerClient::LidState state) {
+  void ReportLidEvent(LidState state) {
     FakePowerManagerClient::Get()->SetLidState(state,
                                                base::TimeTicks::UnixEpoch());
   }
 
+  void SetLidState(LidState state) { ReportLidEvent(state); }
+
  private:
   base::test::TaskEnvironment task_environment_;
   base::test::ScopedFeatureList scoped_feature_list_;
   ScopedFakeAssistantClient fake_assistant_client_;
+  ::testing::NiceMock<ScopedCrasAudioClientMock> cras_audio_client_mock_;
   std::unique_ptr<AudioInputImpl> audio_input_impl_;
+  std::unique_ptr<AudioInputHost> audio_input_host_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioInputImplTest);
 };
 
 TEST_F(AudioInputImplTest, StopRecordingWhenLidClosed) {
   // Trigger a lid open event.
-  ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
+  ReportLidEvent(LidState::OPEN);
   EXPECT_TRUE(GetRecordingStatus());
 
   // Trigger a lid closed event.
-  ReportLidEvent(chromeos::PowerManagerClient::LidState::CLOSED);
+  ReportLidEvent(LidState::CLOSED);
   EXPECT_FALSE(GetRecordingStatus());
 
   // Trigger a lid open event again.
-  ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
+  ReportLidEvent(LidState::OPEN);
+  EXPECT_TRUE(GetRecordingStatus());
+}
+
+TEST_F(AudioInputImplTest, StartRecordingWhenThereIsNoLid) {
+  ReportLidEvent(LidState::NOT_PRESENT);
   EXPECT_TRUE(GetRecordingStatus());
 }
 
 TEST_F(AudioInputImplTest, StopRecordingWithNoPreferredDevice) {
   // Start as recording.
-  ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
+  ReportLidEvent(LidState::OPEN);
   EXPECT_TRUE(GetRecordingStatus());
 
   // Preferred input device is lost.
@@ -127,7 +184,7 @@ TEST_F(AudioInputImplTest, StopRecordingWithNoPreferredDevice) {
 
 TEST_F(AudioInputImplTest, StopRecordingWhenDisableHotword) {
   // Start as recording.
-  ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
+  ReportLidEvent(LidState::OPEN);
   EXPECT_TRUE(GetRecordingStatus());
 
   // Hotword disabled should stop recording.
@@ -141,7 +198,7 @@ TEST_F(AudioInputImplTest, StopRecordingWhenDisableHotword) {
 
 TEST_F(AudioInputImplTest, StartRecordingWhenDisableHotwordAndForceOpenMic) {
   // Start as recording.
-  ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
+  ReportLidEvent(LidState::OPEN);
   EXPECT_TRUE(GetRecordingStatus());
 
   // Hotword disabled should stop recording.
@@ -157,9 +214,19 @@ TEST_F(AudioInputImplTest, StartRecordingWhenDisableHotwordAndForceOpenMic) {
   EXPECT_FALSE(GetRecordingStatus());
 }
 
+TEST_F(AudioInputImplTest, ShouldReadCurrentLidStateWhenLaunching) {
+  SetLidState(LidState::OPEN);
+  CreateNewAudioInputImpl();
+  EXPECT_TRUE(GetRecordingStatus());
+
+  SetLidState(LidState::CLOSED);
+  CreateNewAudioInputImpl();
+  EXPECT_FALSE(GetRecordingStatus());
+}
+
 TEST_F(AudioInputImplTest, SettingHotwordDeviceDoesNotAffectRecordingState) {
   // Start as recording.
-  ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
+  ReportLidEvent(LidState::OPEN);
   EXPECT_TRUE(GetRecordingStatus());
 
   // Hotword device does not change recording state.
@@ -172,7 +239,7 @@ TEST_F(AudioInputImplTest, SettingHotwordDeviceDoesNotAffectRecordingState) {
 
 TEST_F(AudioInputImplTest, SettingHotwordDeviceUsesHotwordDeviceForRecording) {
   // Start as recording.
-  ReportLidEvent(chromeos::PowerManagerClient::LidState::OPEN);
+  ReportLidEvent(LidState::OPEN);
   EXPECT_TRUE(GetRecordingStatus());
 
   // Hotword device does not change recording state.
@@ -185,5 +252,81 @@ TEST_F(AudioInputImplTest, SettingHotwordDeviceUsesHotwordDeviceForRecording) {
   EXPECT_TRUE(IsUsingHotwordDevice());
 }
 
+TEST_F(AudioInputImplTest, ShouldSendHotwordLocaleToCrasAudioClient) {
+  StopAudioRecording();
+
+  audio_input_host().SetHotwordDeviceId("111");
+
+  EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel);
+  audio_input_host().SetDspHotwordLocale("bla");
+}
+
+TEST_F(AudioInputImplTest,
+       ShouldFormatHotwordLocaleAndSendItToCrasAudioClient) {
+  StopAudioRecording();
+  audio_input_host().SetHotwordDeviceId("111");
+
+  // Normal case
+  EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel(111, "nl_be", _));
+  audio_input_host().SetDspHotwordLocale("nl-BE");
+
+  // Handle the case where country code and language code are the same
+  EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel(111, "fr_fr", _));
+  audio_input_host().SetDspHotwordLocale("fr");
+
+  // use "en_all" for all english locales
+  EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel(111, "en_all", _));
+  audio_input_host().SetDspHotwordLocale("en-US");
+}
+
+TEST_F(AudioInputImplTest, ShouldUseDefaultLocaleIfUserPrefIsRejected) {
+  const std::string default_locale = "en_us";
+  StopAudioRecording();
+  audio_input_host().SetHotwordDeviceId("222");
+
+  EXPECT_CALL(cras_audio_client_mock(),
+              SetHotwordModel(222, "rejected_locale", _))
+      .WillOnce([](uint64_t node_id, const std::string&,
+                   VoidDBusMethodCallback callback) {
+        // Report failure to change the locale
+        std::move(callback).Run(/*success=*/false);
+      });
+
+  EXPECT_CALL(cras_audio_client_mock(),
+              SetHotwordModel(222, default_locale, _));
+
+  audio_input_host().SetDspHotwordLocale("rejected-LOCALE");
+}
+
+TEST_F(AudioInputImplTest, ShouldUseDefaultLocaleIfUserPrefIsEmpty) {
+  const std::string default_locale = "en_us";
+  StopAudioRecording();
+  audio_input_host().SetHotwordDeviceId("222");
+
+  EXPECT_CALL(cras_audio_client_mock(),
+              SetHotwordModel(222, default_locale, _));
+
+  audio_input_host().SetDspHotwordLocale("");
+}
+
+TEST_F(AudioInputImplTest, ShouldDoNothingIfUserPrefIsAccepted) {
+  const std::string default_locale = "en_us";
+  StopAudioRecording();
+  audio_input_host().SetHotwordDeviceId("222");
+
+  EXPECT_CALL(cras_audio_client_mock(),
+              SetHotwordModel(222, "accepted_locale", _))
+      .WillOnce([](uint64_t node_id, const std::string&,
+                   VoidDBusMethodCallback callback) {
+        // Accept the change to the locale.
+        std::move(callback).Run(/*success=*/true);
+      });
+
+  // Do not expect a second call if change of locale is accepted
+  EXPECT_CALL(cras_audio_client_mock(), SetHotwordModel(222, default_locale, _))
+      .Times(0);
+
+  audio_input_host().SetDspHotwordLocale("accepted-LOCALE");
+}
 }  // namespace assistant
 }  // namespace chromeos

chromeos/services/assistant/platform/audio_input_provider_impl.cc

@@ -9,12 +9,8 @@
 namespace chromeos {
 namespace assistant {
 
-AudioInputProviderImpl::AudioInputProviderImpl(
-    PowerManagerClient* power_manager_client,
-    CrasAudioHandler* cras_audio_handler)
-    : audio_input_(power_manager_client,
-                   cras_audio_handler,
-                   /*input_device_id=*/std::string()) {}
+AudioInputProviderImpl::AudioInputProviderImpl()
+    : audio_input_(/*input_device_id=*/std::string()) {}
 
 AudioInputProviderImpl::~AudioInputProviderImpl() = default;
 
@@ -29,25 +25,5 @@ int64_t AudioInputProviderImpl::GetCurrentAudioTime() {
   return 0;
 }
 
-void AudioInputProviderImpl::SetMicState(bool mic_open) {
-  audio_input_.SetMicState(mic_open);
-}
-
-void AudioInputProviderImpl::OnHotwordEnabled(bool enable) {
-  audio_input_.OnHotwordEnabled(enable);
-}
-
-void AudioInputProviderImpl::SetDeviceId(const std::string& device_id) {
-  audio_input_.SetDeviceId(device_id);
-}
-
-void AudioInputProviderImpl::SetHotwordDeviceId(const std::string& device_id) {
-  audio_input_.SetHotwordDeviceId(device_id);
-}
-
-void AudioInputProviderImpl::SetDspHotwordLocale(std::string pref_locale) {
-  audio_input_.SetDspHotwordLocale(pref_locale);
-}
-
 }  // namespace assistant
 }  // namespace chromeos

chromeos/services/assistant/platform/audio_input_provider_impl.h

@@ -14,36 +14,18 @@
 #include "libassistant/shared/public/platform_audio_input.h"
 
 namespace chromeos {
-class CrasAudioHandler;
-class PowerManagerClient;
 
 namespace assistant {
 
 class AudioInputProviderImpl : public assistant_client::AudioInputProvider {
  public:
-  AudioInputProviderImpl(PowerManagerClient* power_manager_client,
-                         CrasAudioHandler* cras_audio_handler);
+  AudioInputProviderImpl();
   ~AudioInputProviderImpl() override;
 
   // assistant_client::AudioInputProvider overrides:
   AudioInputImpl& GetAudioInput() override;
   int64_t GetCurrentAudioTime() override;
 
-  // Called when the mic state associated with the interaction is changed.
-  void SetMicState(bool mic_open);
-
-  // Called when hotword enabled status changed.
-  void OnHotwordEnabled(bool enable);
-
-  // Setting the input device to use for audio capture.
-  void SetDeviceId(const std::string& device_id);
-
-  // Setting the hotword input device with hardware based hotword detection.
-  void SetHotwordDeviceId(const std::string& device_id);
-
-  // Setting the hotword locale for the input device with DSP support.
-  void SetDspHotwordLocale(std::string pref_locale);
-
  private:
   AudioInputImpl audio_input_;
 

chromeos/services/assistant/platform/audio_output_provider_impl.cc

@@ -142,14 +142,10 @@ class AudioOutputImpl : public assistant_client::AudioOutput {
 }  // namespace
 
 AudioOutputProviderImpl::AudioOutputProviderImpl(
-    PowerManagerClient* power_manager_client,
-    CrasAudioHandler* cras_audio_handler,
     AssistantMediaSession* media_session,
     scoped_refptr<base::SequencedTaskRunner> background_task_runner,
     const std::string& device_id)
-    : loop_back_input_(power_manager_client,
-                       cras_audio_handler,
-                       media::AudioDeviceDescription::kLoopbackInputDeviceId),
+    : loop_back_input_(media::AudioDeviceDescription::kLoopbackInputDeviceId),
       volume_control_impl_(media_session),
       main_task_runner_(base::SequencedTaskRunnerHandle::Get()),
       background_task_runner_(background_task_runner),

chromeos/services/assistant/platform/audio_output_provider_impl.h

@@ -24,8 +24,6 @@
 #include "services/audio/public/mojom/stream_factory.mojom.h"
 
 namespace chromeos {
-class CrasAudioHandler;
-class PowerManagerClient;
 
 namespace assistant {
 
@@ -34,8 +32,6 @@ class AssistantMediaSession;
 class AudioOutputProviderImpl : public assistant_client::AudioOutputProvider {
  public:
   AudioOutputProviderImpl(
-      PowerManagerClient* power_manager_client,
-      CrasAudioHandler* cras_audio_handler,
       AssistantMediaSession* media_session,
       scoped_refptr<base::SequencedTaskRunner> background_task_runner,
       const std::string& device_id);

chromeos/services/assistant/platform_api_impl.cc

@@ -84,12 +84,13 @@ PlatformApiImpl::PlatformApiImpl(
     scoped_refptr<base::SequencedTaskRunner> main_thread_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> background_task_runner,
     std::string pref_locale)
-    : audio_input_provider_(power_manager_client, cras_audio_handler),
-      audio_output_provider_(power_manager_client,
-                             cras_audio_handler,
-                             media_session,
+    : audio_input_provider_(),
+      audio_output_provider_(media_session,
                              background_task_runner,
                              media::AudioDeviceDescription::kDefaultDeviceId),
+      audio_input_host_(&audio_input_provider_.GetAudioInput(),
+                        cras_audio_handler,
+                        power_manager_client),
       pref_locale_(pref_locale),
       cras_audio_handler_(cras_audio_handler) {
   // Only enable native power features if they are supported by the UI.
@@ -165,32 +166,32 @@ void PlatformApiImpl::OnAudioNodesChanged() {
     }
   }
 
-  audio_input_provider_.SetDeviceId(
+  audio_input_host_.SetDeviceId(
       input_device ? base::NumberToString(input_device->id) : std::string());
 
   if (hotword_device) {
-    audio_input_provider_.SetHotwordDeviceId(
+    audio_input_host_.SetHotwordDeviceId(
         base::NumberToString(hotword_device->id));
-    audio_input_provider_.SetDspHotwordLocale(pref_locale_);
+    audio_input_host_.SetDspHotwordLocale(pref_locale_);
   } else {
-    audio_input_provider_.SetHotwordDeviceId(std::string());
+    audio_input_host_.SetHotwordDeviceId(std::string());
   }
 }
 
 void PlatformApiImpl::SetMicState(bool mic_open) {
-  audio_input_provider_.SetMicState(mic_open);
+  audio_input_host_.SetMicState(mic_open);
 }
 
 void PlatformApiImpl::OnConversationTurnStarted() {
-  audio_input_provider_.GetAudioInput().OnConversationTurnStarted();
+  audio_input_host_.OnConversationTurnStarted();
 }
 
 void PlatformApiImpl::OnConversationTurnFinished() {
-  audio_input_provider_.GetAudioInput().OnConversationTurnFinished();
+  audio_input_host_.OnConversationTurnFinished();
 }
 
 void PlatformApiImpl::OnHotwordEnabled(bool enable) {
-  audio_input_provider_.OnHotwordEnabled(enable);
+  audio_input_host_.OnHotwordEnabled(enable);
 }
 
 }  // namespace assistant

chromeos/services/assistant/platform_api_impl.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "chromeos/audio/cras_audio_handler.h"
+#include "chromeos/services/assistant/platform/audio_input_host.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"
@@ -101,6 +102,7 @@ class PlatformApiImpl : public CrosPlatformApi,
   FakeAuthProvider auth_provider_;
   FileProviderImpl file_provider_;
   NetworkProviderImpl network_provider_;
+  AudioInputHost audio_input_host_;
   std::unique_ptr<SystemProviderImpl> system_provider_;
   std::string pref_locale_;