Commit 1d42dc3d authored by Meilin Wang's avatar Meilin Wang Committed by Commit Bot

Enable signed-out libassistant for ambient mode.

This change enables the mode switch between signed-in and signed-out
for Liassbistant when user enters/exits the ambient mode, so that we
can further explore the behavior of signed-out mode.

Bug: none.
Test: run unittest added in this change
Change-Id: Idcb4903401173b8f86a3428a0f452b5ce9b7a5ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1995804
Commit-Queue: Meilin Wang <meilinw@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737011}
parent 3711eb12
...@@ -139,6 +139,7 @@ source_set("tests") { ...@@ -139,6 +139,7 @@ source_set("tests") {
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//chromeos/audio", "//chromeos/audio",
"//chromeos/constants",
"//chromeos/dbus:test_support", "//chromeos/dbus:test_support",
"//chromeos/dbus/power", "//chromeos/dbus/power",
"//chromeos/services/assistant/public/cpp:prefs", "//chromeos/services/assistant/public/cpp:prefs",
......
...@@ -44,7 +44,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerService ...@@ -44,7 +44,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerService
~AssistantManagerService() override = default; ~AssistantManagerService() override = default;
// Start the assistant in the background with |access_token|, where the // Start the Assistant in the background with |access_token|, where the
// token can be nullopt when the service is being started under the signed // token can be nullopt when the service is being started under the signed
// out mode. // out mode.
// If you want to know when the service is started, use // If you want to know when the service is started, use
...@@ -52,14 +52,20 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerService ...@@ -52,14 +52,20 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerService
virtual void Start(const base::Optional<std::string>& access_token, virtual void Start(const base::Optional<std::string>& access_token,
bool enable_hotword) = 0; bool enable_hotword) = 0;
// Stop the assistant. // Stop the Assistant.
virtual void Stop() = 0; virtual void Stop() = 0;
// Return the current state. // Return the current state.
virtual State GetState() const = 0; virtual State GetState() const = 0;
// Set access token for assistant. // Set access token for Assistant. Passing a nullopt will reconfigure
virtual void SetAccessToken(const std::string& access_token) = 0; // Libassistant to run in signed-out mode, and passing a valid non-empty value
// will switch the mode back to normal.
virtual void SetAccessToken(
const base::Optional<std::string>& access_token) = 0;
// Enable/disable ambient mode for Assistant.
virtual void EnableAmbientMode(bool enabled) = 0;
// Turn on / off all listening, including hotword and voice query. // Turn on / off all listening, including hotword and voice query.
virtual void EnableListening(bool enable) = 0; virtual void EnableListening(bool enable) = 0;
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include "ash/public/cpp/ambient/ambient_mode_state.h"
#include "ash/public/cpp/assistant/assistant_state_base.h" #include "ash/public/cpp/assistant/assistant_state_base.h"
#include "base/barrier_closure.h" #include "base/barrier_closure.h"
#include "base/bind.h" #include "base/bind.h"
...@@ -26,6 +27,7 @@ ...@@ -26,6 +27,7 @@
#include "chromeos/assistant/internal/proto/google3/assistant/api/client_input/warmer_welcome_input.pb.h" #include "chromeos/assistant/internal/proto/google3/assistant/api/client_input/warmer_welcome_input.pb.h"
#include "chromeos/assistant/internal/proto/google3/assistant/api/client_op/device_args.pb.h" #include "chromeos/assistant/internal/proto/google3/assistant/api/client_op/device_args.pb.h"
#include "chromeos/constants/chromeos_features.h" #include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h"
#include "chromeos/dbus/util/version_loader.h" #include "chromeos/dbus/util/version_loader.h"
#include "chromeos/services/assistant/assistant_manager_service_delegate.h" #include "chromeos/services/assistant/assistant_manager_service_delegate.h"
#include "chromeos/services/assistant/constants.h" #include "chromeos/services/assistant/constants.h"
...@@ -143,6 +145,15 @@ CommunicationErrorType CommunicationErrorTypeFromLibassistantErrorCode( ...@@ -143,6 +145,15 @@ CommunicationErrorType CommunicationErrorTypeFromLibassistantErrorCode(
return CommunicationErrorType::Other; return CommunicationErrorType::Other;
} }
std::vector<std::pair<std::string, std::string>> ReturnAuthTokensOrEmpty(
const base::Optional<std::string>& access_token) {
if (!access_token.has_value())
return {};
DCHECK(!access_token.value().empty());
return {std::pair<std::string, std::string>(kUserID, access_token.value())};
}
} // namespace } // namespace
AssistantManagerServiceImpl::AssistantManagerServiceImpl( AssistantManagerServiceImpl::AssistantManagerServiceImpl(
...@@ -151,8 +162,7 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl( ...@@ -151,8 +162,7 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl(
std::unique_ptr<AssistantManagerServiceDelegate> delegate, std::unique_ptr<AssistantManagerServiceDelegate> delegate,
std::unique_ptr<network::PendingSharedURLLoaderFactory> std::unique_ptr<network::PendingSharedURLLoaderFactory>
pending_url_loader_factory, pending_url_loader_factory,
base::Optional<std::string> s3_server_uri_override, base::Optional<std::string> s3_server_uri_override)
bool is_signed_out_mode)
: client_(client), : client_(client),
media_session_(std::make_unique<AssistantMediaSession>(client_, this)), media_session_(std::make_unique<AssistantMediaSession>(client_, this)),
action_module_(std::make_unique<action::CrosActionModule>( action_module_(std::make_unique<action::CrosActionModule>(
...@@ -165,7 +175,6 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl( ...@@ -165,7 +175,6 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl(
context_(context), context_(context),
delegate_(std::move(delegate)), delegate_(std::move(delegate)),
background_thread_("background thread"), background_thread_("background thread"),
is_signed_out_mode_(is_signed_out_mode),
libassistant_config_(CreateLibAssistantConfig(s3_server_uri_override)), libassistant_config_(CreateLibAssistantConfig(s3_server_uri_override)),
weak_factory_(this) { weak_factory_(this) {
background_thread_.Start(); background_thread_.Start();
...@@ -182,10 +191,6 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl( ...@@ -182,10 +191,6 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl(
} }
AssistantManagerServiceImpl::~AssistantManagerServiceImpl() { AssistantManagerServiceImpl::~AssistantManagerServiceImpl() {
auto* ambient_state = ash::AmbientModeState::Get();
if (ambient_state)
ambient_state->RemoveObserver(this);
background_thread_.Stop(); background_thread_.Stop();
} }
...@@ -202,15 +207,12 @@ void AssistantManagerServiceImpl::Start( ...@@ -202,15 +207,12 @@ void AssistantManagerServiceImpl::Start(
EnableHotword(enable_hotword); EnableHotword(enable_hotword);
// Check the AmbientModeState to keep us synced on |ambient_state|.
if (chromeos::features::IsAmbientModeEnabled()) { if (chromeos::features::IsAmbientModeEnabled()) {
auto* ambient_state = ash::AmbientModeState::Get(); auto* ambient_state = ash::AmbientModeState::Get();
DCHECK(ambient_state); DCHECK(ambient_state);
// Update the support action list in action module when system enters/exits EnableAmbientMode(ambient_state->enabled());
// the Ambient Mode. Some actions such as open URL in the browser will be
// disabled in this mode.
action_module_->SetAmbientModeEnabled(ambient_state->enabled());
ambient_state->AddObserver(this);
} }
// LibAssistant creation will make file IO and sync wait. Post the creation to // LibAssistant creation will make file IO and sync wait. Post the creation to
...@@ -229,12 +231,6 @@ void AssistantManagerServiceImpl::Stop() { ...@@ -229,12 +231,6 @@ void AssistantManagerServiceImpl::Stop() {
SetStateAndInformObservers(State::STOPPED); SetStateAndInformObservers(State::STOPPED);
if (chromeos::features::IsAmbientModeEnabled()) {
auto* ambient_state = ash::AmbientModeState::Get();
DCHECK(ambient_state);
ambient_state->RemoveObserver(this);
}
// When user disables the feature, we also deletes all data. // When user disables the feature, we also deletes all data.
if (!assistant_state()->settings_enabled().value() && assistant_manager_) if (!assistant_state()->settings_enabled().value() && assistant_manager_)
assistant_manager_->ResetAllDataAndShutdown(); assistant_manager_->ResetAllDataAndShutdown();
...@@ -251,20 +247,28 @@ AssistantManagerService::State AssistantManagerServiceImpl::GetState() const { ...@@ -251,20 +247,28 @@ AssistantManagerService::State AssistantManagerServiceImpl::GetState() const {
} }
void AssistantManagerServiceImpl::SetAccessToken( void AssistantManagerServiceImpl::SetAccessToken(
const std::string& access_token) { const base::Optional<std::string>& access_token) {
if (!assistant_manager_) if (!assistant_manager_)
return; return;
DCHECK(!access_token.empty());
VLOG(1) << "Set access token."; VLOG(1) << "Set access token.";
// Push the |access_token| we got as an argument into AssistantManager before // Push the |access_token| we got as an argument into AssistantManager before
// starting to ensure that all server requests will be authenticated once // starting to ensure that all server requests will be authenticated once
// it is started. |user_id| is used to pair a user to their |access_token|, // it is started. |user_id| is used to pair a user to their |access_token|,
// since we do not support multi-user in this example we can set it to a // since we do not support multi-user in this example we can set it to a
// dummy value like "0". // dummy value like "0". When |access_token| does not contain a value, we
assistant_manager_->SetAuthTokens( // need to switch Libassistant to signed-out mode so that it can work with
{std::pair<std::string, std::string>(kUserID, access_token)}); // 0 auth token.
assistant_manager_->SetAuthTokens(ReturnAuthTokensOrEmpty(access_token));
}
void AssistantManagerServiceImpl::EnableAmbientMode(bool enabled) {
if (!assistant_manager_)
return;
// Update |action_module_| accordingly, as some actions, e.g. open URL
// in the browser, are not supported in ambient mode.
action_module_->SetAmbientModeEnabled(enabled);
} }
void AssistantManagerServiceImpl::RegisterFallbackMediaHandler() { void AssistantManagerServiceImpl::RegisterFallbackMediaHandler() {
...@@ -1178,10 +1182,9 @@ void AssistantManagerServiceImpl::StartAssistantInternal( ...@@ -1178,10 +1182,9 @@ void AssistantManagerServiceImpl::StartAssistantInternal(
server_experiment_ids); server_experiment_ids);
} }
if (!is_signed_out_mode_) { // When |access_token| does not contain a value, we will start Libassistant
new_assistant_manager_->SetAuthTokens( // in signed-out mode by calling SetAuthTokens() with an empty vector.
{std::pair<std::string, std::string>(kUserID, access_token.value())}); new_assistant_manager_->SetAuthTokens(ReturnAuthTokensOrEmpty(access_token));
}
new_assistant_manager_->Start(); new_assistant_manager_->Start();
} }
...@@ -1316,10 +1319,6 @@ void AssistantManagerServiceImpl::OnAndroidAppListRefreshed( ...@@ -1316,10 +1319,6 @@ void AssistantManagerServiceImpl::OnAndroidAppListRefreshed(
display_connection_->OnAndroidAppListRefreshed(android_apps_info); display_connection_->OnAndroidAppListRefreshed(android_apps_info);
} }
void AssistantManagerServiceImpl::OnAmbientModeEnabled(bool enabled) {
action_module_->SetAmbientModeEnabled(enabled);
}
void AssistantManagerServiceImpl::UpdateInternalOptions( void AssistantManagerServiceImpl::UpdateInternalOptions(
assistant_client::AssistantManagerInternal* assistant_manager_internal) { assistant_client::AssistantManagerInternal* assistant_manager_internal) {
// Build internal options // Build internal options
...@@ -1331,7 +1330,11 @@ void AssistantManagerServiceImpl::UpdateInternalOptions( ...@@ -1331,7 +1330,11 @@ void AssistantManagerServiceImpl::UpdateInternalOptions(
internal_options->SetClientControlEnabled( internal_options->SetClientControlEnabled(
assistant::features::IsRoutinesEnabled()); assistant::features::IsRoutinesEnabled());
if (is_signed_out_mode_) { // TODO(meilinw): remove this logic and instead use the new config flag
// once we uprev.
if (chromeos::features::IsAmbientModeEnabled() ||
base::CommandLine::ForCurrentProcess()->HasSwitch(
chromeos::switches::kDisableGaiaServices)) {
internal_options->SetUserCredentialMode( internal_options->SetUserCredentialMode(
assistant_client::InternalOptions::UserCredentialMode::SIGNED_OUT); assistant_client::InternalOptions::UserCredentialMode::SIGNED_OUT);
} }
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "ash/public/cpp/ambient/ambient_mode_state.h"
#include "ash/public/mojom/assistant_controller.mojom.h" #include "ash/public/mojom/assistant_controller.mojom.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/optional.h" #include "base/optional.h"
...@@ -96,8 +95,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl ...@@ -96,8 +95,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
public assistant_client::DeviceStateListener, public assistant_client::DeviceStateListener,
public assistant_client::MediaManager::Listener, public assistant_client::MediaManager::Listener,
public media_session::mojom::MediaControllerObserver, public media_session::mojom::MediaControllerObserver,
public mojom::AppListEventSubscriber, public mojom::AppListEventSubscriber {
public ash::AmbientModeStateObserver {
public: public:
// |service| owns this class and must outlive this class. // |service| owns this class and must outlive this class.
AssistantManagerServiceImpl( AssistantManagerServiceImpl(
...@@ -106,8 +104,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl ...@@ -106,8 +104,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
std::unique_ptr<AssistantManagerServiceDelegate> delegate, std::unique_ptr<AssistantManagerServiceDelegate> delegate,
std::unique_ptr<network::PendingSharedURLLoaderFactory> std::unique_ptr<network::PendingSharedURLLoaderFactory>
pending_url_loader_factory, pending_url_loader_factory,
base::Optional<std::string> s3_server_uri_override, base::Optional<std::string> s3_server_uri_override);
bool is_signed_out_mode);
~AssistantManagerServiceImpl() override; ~AssistantManagerServiceImpl() override;
...@@ -116,7 +113,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl ...@@ -116,7 +113,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
bool enable_hotword) override; bool enable_hotword) override;
void Stop() override; void Stop() override;
State GetState() const override; State GetState() const override;
void SetAccessToken(const std::string& access_token) override; void SetAccessToken(const base::Optional<std::string>& access_token) override;
void EnableAmbientMode(bool enabled) override;
void EnableListening(bool enable) override; void EnableListening(bool enable) override;
void EnableHotword(bool enable) override; void EnableHotword(bool enable) override;
void SetArcPlayStoreEnabled(bool enable) override; void SetArcPlayStoreEnabled(bool enable) override;
...@@ -204,9 +202,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl ...@@ -204,9 +202,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
void OnAndroidAppListRefreshed( void OnAndroidAppListRefreshed(
std::vector<mojom::AndroidAppInfoPtr> apps_info) override; std::vector<mojom::AndroidAppInfoPtr> apps_info) override;
// ash::AmbientModeStateObserver overrides:
void OnAmbientModeEnabled(bool enabled) override;
void UpdateInternalOptions( void UpdateInternalOptions(
assistant_client::AssistantManagerInternal* assistant_manager_internal); assistant_client::AssistantManagerInternal* assistant_manager_internal);
...@@ -367,7 +362,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl ...@@ -367,7 +362,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
std::string receive_url_response_; std::string receive_url_response_;
bool is_first_client_discourse_context_query_ = true; bool is_first_client_discourse_context_query_ = true;
bool is_signed_out_mode_;
mojo::Receiver<media_session::mojom::MediaControllerObserver> mojo::Receiver<media_session::mojom::MediaControllerObserver>
media_controller_observer_receiver_{this}; media_controller_observer_receiver_{this};
......
...@@ -197,8 +197,7 @@ class AssistantManagerServiceImplTest : public testing::Test { ...@@ -197,8 +197,7 @@ class AssistantManagerServiceImplTest : public testing::Test {
assistant_manager_service_ = std::make_unique<AssistantManagerServiceImpl>( assistant_manager_service_ = std::make_unique<AssistantManagerServiceImpl>(
&assistant_client_, service_context_.get(), std::move(delegate), &assistant_client_, service_context_.get(), std::move(delegate),
shared_url_loader_factory_->Clone(), s3_server_uri_override, shared_url_loader_factory_->Clone(), s3_server_uri_override);
/*is_signed_out_mode=*/false);
} }
void TearDown() override { void TearDown() override {
......
...@@ -21,6 +21,7 @@ void FakeAssistantManagerServiceImpl::Start( ...@@ -21,6 +21,7 @@ void FakeAssistantManagerServiceImpl::Start(
const base::Optional<std::string>& access_token, const base::Optional<std::string>& access_token,
bool enable_hotword) { bool enable_hotword) {
SetStateAndInformObservers(State::STARTING); SetStateAndInformObservers(State::STARTING);
access_token_ = access_token;
} }
void FakeAssistantManagerServiceImpl::Stop() { void FakeAssistantManagerServiceImpl::Stop() {
...@@ -28,12 +29,16 @@ void FakeAssistantManagerServiceImpl::Stop() { ...@@ -28,12 +29,16 @@ void FakeAssistantManagerServiceImpl::Stop() {
} }
void FakeAssistantManagerServiceImpl::SetAccessToken( void FakeAssistantManagerServiceImpl::SetAccessToken(
const std::string& access_token) {} const base::Optional<std::string>& access_token) {
access_token_ = access_token;
}
void FakeAssistantManagerServiceImpl::EnableListening(bool enable) {} void FakeAssistantManagerServiceImpl::EnableListening(bool enable) {}
void FakeAssistantManagerServiceImpl::EnableHotword(bool enable) {} void FakeAssistantManagerServiceImpl::EnableHotword(bool enable) {}
void FakeAssistantManagerServiceImpl::EnableAmbientMode(bool enabled) {}
void FakeAssistantManagerServiceImpl::SetArcPlayStoreEnabled(bool enabled) {} void FakeAssistantManagerServiceImpl::SetArcPlayStoreEnabled(bool enabled) {}
AssistantManagerService::State FakeAssistantManagerServiceImpl::GetState() AssistantManagerService::State FakeAssistantManagerServiceImpl::GetState()
......
...@@ -31,9 +31,10 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) FakeAssistantManagerServiceImpl ...@@ -31,9 +31,10 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) FakeAssistantManagerServiceImpl
void Start(const base::Optional<std::string>& access_token, void Start(const base::Optional<std::string>& access_token,
bool enable_hotword) override; bool enable_hotword) override;
void Stop() override; void Stop() override;
void SetAccessToken(const std::string& access_token) override; void SetAccessToken(const base::Optional<std::string>& access_token) override;
void EnableListening(bool enable) override; void EnableListening(bool enable) override;
void EnableHotword(bool enable) override; void EnableHotword(bool enable) override;
void EnableAmbientMode(bool enabled) override;
void SetArcPlayStoreEnabled(bool enabled) override; void SetArcPlayStoreEnabled(bool enabled) override;
State GetState() const override; State GetState() const override;
AssistantSettingsManager* GetAssistantSettingsManager() override; AssistantSettingsManager* GetAssistantSettingsManager() override;
...@@ -74,12 +75,16 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) FakeAssistantManagerServiceImpl ...@@ -74,12 +75,16 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) FakeAssistantManagerServiceImpl
// |AssistantStateObserver| of the change. // |AssistantStateObserver| of the change.
void SetStateAndInformObservers(State new_state); void SetStateAndInformObservers(State new_state);
// Return the |access_token| that was passed to |SetAccessToken|.
base::Optional<std::string> access_token() { return access_token_; }
private: private:
// Send out a |AssistantStateObserver::OnStateChange(state)| event if we are // Send out a |AssistantStateObserver::OnStateChange(state)| event if we are
// transitioning from a prior state to a later state. // transitioning from a prior state to a later state.
void MaybeSendStateChange(State state, State old_state, State target_state); void MaybeSendStateChange(State state, State old_state, State target_state);
State state_ = State::STOPPED; State state_ = State::STOPPED;
base::Optional<std::string> access_token_;
FakeAssistantSettingsManagerImpl assistant_settings_manager_; FakeAssistantSettingsManagerImpl assistant_settings_manager_;
base::ObserverList<StateObserver> state_observers_; base::ObserverList<StateObserver> state_observers_;
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "build/buildflag.h" #include "build/buildflag.h"
#include "chromeos/assistant/buildflags.h" #include "chromeos/assistant/buildflags.h"
#include "chromeos/audio/cras_audio_handler.h" #include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h" #include "chromeos/constants/chromeos_switches.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/power_manager/power_supply_properties.pb.h" #include "chromeos/dbus/power_manager/power_supply_properties.pb.h"
...@@ -90,6 +91,22 @@ base::Optional<std::string> GetS3ServerUriOverride() { ...@@ -90,6 +91,22 @@ base::Optional<std::string> GetS3ServerUriOverride() {
} }
#endif #endif
// In the signed-out mode, we are going to run Assistant service without
// using user's signed in account information.
bool IsSignedOutMode() {
// We will switch the Libassitsant mode to signed-out/signed-in when user
// enters/exits the ambient mode.
const bool entered_ambient_mode =
chromeos::features::IsAmbientModeEnabled() &&
ash::AmbientModeState::Get()->enabled();
// Note that we shouldn't toggle the flag to true when exiting ambient
// mode if we have been using fake gaia login, e.g. in the Tast test.
return entered_ambient_mode ||
base::CommandLine::ForCurrentProcess()->HasSwitch(
chromeos::switches::kDisableGaiaServices);
}
} // namespace } // namespace
class Service::Context : public ServiceContext { class Service::Context : public ServiceContext {
...@@ -164,6 +181,11 @@ Service::Service(mojo::PendingReceiver<mojom::AssistantService> receiver, ...@@ -164,6 +181,11 @@ Service::Service(mojo::PendingReceiver<mojom::AssistantService> receiver,
Service::~Service() { Service::~Service() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Add null check for |AmbientModeState| in case that |Service| is released
// after ash has gone.
if (chromeos::features::IsAmbientModeEnabled() &&
ash::AmbientModeState::Get())
ash::AmbientModeState::Get()->RemoveObserver(this);
assistant_state_.RemoveObserver(this); assistant_state_.RemoveObserver(this);
auto* const session_controller = ash::SessionController::Get(); auto* const session_controller = ash::SessionController::Get();
if (observing_ash_session_ && session_controller) { if (observing_ash_session_ && session_controller) {
...@@ -213,12 +235,8 @@ void Service::Init(mojo::PendingRemote<mojom::Client> client, ...@@ -213,12 +235,8 @@ void Service::Init(mojo::PendingRemote<mojom::Client> client,
DCHECK(!assistant_manager_service_); DCHECK(!assistant_manager_service_);
// Don't fetch token for test. if (chromeos::features::IsAmbientModeEnabled())
if (base::CommandLine::ForCurrentProcess()->HasSwitch( ash::AmbientModeState::Get()->AddObserver(this);
chromeos::switches::kDisableGaiaServices)) {
is_signed_out_mode_ = true;
return;
}
RequestAccessToken(); RequestAccessToken();
} }
...@@ -338,18 +356,34 @@ void Service::OnStateChanged(AssistantManagerService::State new_state) { ...@@ -338,18 +356,34 @@ void Service::OnStateChanged(AssistantManagerService::State new_state) {
UpdateListeningState(); UpdateListeningState();
} }
void Service::OnAmbientModeEnabled(bool enabled) {
if (IsSignedOutMode()) {
UpdateAssistantManagerState();
} else {
// Refresh the access_token before we switch back to signed-in mode in case
// that we don't have any auth_token cached before.
RequestAccessToken();
}
}
void Service::UpdateAssistantManagerState() { void Service::UpdateAssistantManagerState() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!assistant_state_.hotword_enabled().has_value() || if (!assistant_state_.hotword_enabled().has_value() ||
!assistant_state_.settings_enabled().has_value() || !assistant_state_.settings_enabled().has_value() ||
!assistant_state_.locale().has_value() || !assistant_state_.locale().has_value() ||
(!access_token_.has_value() && !is_signed_out_mode_) || (!access_token_.has_value() && !IsSignedOutMode()) ||
!assistant_state_.arc_play_store_enabled().has_value()) { !assistant_state_.arc_play_store_enabled().has_value()) {
// Assistant state has not finished initialization, let's wait. // Assistant state has not finished initialization, let's wait.
return; return;
} }
if (IsSignedOutMode()) {
// Clear |access_token_| in signed-out mode to keep it synced with what we
// will pass to the |assistant_manager_service_|.
access_token_ = base::nullopt;
}
if (!assistant_manager_service_) if (!assistant_manager_service_)
CreateAssistantManagerService(); CreateAssistantManagerService();
...@@ -357,9 +391,7 @@ void Service::UpdateAssistantManagerState() { ...@@ -357,9 +391,7 @@ void Service::UpdateAssistantManagerState() {
switch (state) { switch (state) {
case AssistantManagerService::State::STOPPED: case AssistantManagerService::State::STOPPED:
if (assistant_state_.settings_enabled().value()) { if (assistant_state_.settings_enabled().value()) {
assistant_manager_service_->Start( assistant_manager_service_->Start(access_token_, ShouldEnableHotword());
is_signed_out_mode_ ? base::nullopt : access_token_,
ShouldEnableHotword());
DVLOG(1) << "Request Assistant start"; DVLOG(1) << "Request Assistant start";
} }
break; break;
...@@ -384,8 +416,11 @@ void Service::UpdateAssistantManagerState() { ...@@ -384,8 +416,11 @@ void Service::UpdateAssistantManagerState() {
break; break;
case AssistantManagerService::State::RUNNING: case AssistantManagerService::State::RUNNING:
if (assistant_state_.settings_enabled().value()) { if (assistant_state_.settings_enabled().value()) {
if (!is_signed_out_mode_) assistant_manager_service_->SetAccessToken(access_token_);
assistant_manager_service_->SetAccessToken(access_token_.value()); if (chromeos::features::IsAmbientModeEnabled()) {
assistant_manager_service_->EnableAmbientMode(
ash::AmbientModeState::Get()->enabled());
}
assistant_manager_service_->EnableHotword(ShouldEnableHotword()); assistant_manager_service_->EnableHotword(ShouldEnableHotword());
assistant_manager_service_->SetArcPlayStoreEnabled( assistant_manager_service_->SetArcPlayStoreEnabled(
assistant_state_.arc_play_store_enabled().value()); assistant_state_.arc_play_store_enabled().value());
...@@ -407,8 +442,8 @@ identity::mojom::IdentityAccessor* Service::GetIdentityAccessor() { ...@@ -407,8 +442,8 @@ identity::mojom::IdentityAccessor* Service::GetIdentityAccessor() {
void Service::RequestAccessToken() { void Service::RequestAccessToken() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Bypass access token fetching under signed out mode. // Bypass access token fetching when service is running in signed-out mode.
if (is_signed_out_mode_) if (IsSignedOutMode())
return; return;
VLOG(1) << "Start requesting access token."; VLOG(1) << "Start requesting access token.";
...@@ -504,8 +539,7 @@ Service::CreateAndReturnAssistantManagerService() { ...@@ -504,8 +539,7 @@ Service::CreateAndReturnAssistantManagerService() {
DCHECK(pending_url_loader_factory_); DCHECK(pending_url_loader_factory_);
return std::make_unique<AssistantManagerServiceImpl>( return std::make_unique<AssistantManagerServiceImpl>(
client_.get(), context(), std::move(delegate), client_.get(), context(), std::move(delegate),
std::move(pending_url_loader_factory_), GetS3ServerUriOverride(), std::move(pending_url_loader_factory_), GetS3ServerUriOverride());
is_signed_out_mode_);
#else #else
return std::make_unique<FakeAssistantManagerServiceImpl>(); return std::make_unique<FakeAssistantManagerServiceImpl>();
#endif #endif
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <memory> #include <memory>
#include <string> #include <string>
#include "ash/public/cpp/ambient/ambient_mode_state.h"
#include "ash/public/cpp/session/session_activation_observer.h" #include "ash/public/cpp/session/session_activation_observer.h"
#include "ash/public/mojom/assistant_controller.mojom.h" #include "ash/public/mojom/assistant_controller.mojom.h"
#include "base/callback.h" #include "base/callback.h"
...@@ -64,7 +65,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service ...@@ -64,7 +65,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
public ash::SessionActivationObserver, public ash::SessionActivationObserver,
public ash::AssistantStateObserver, public ash::AssistantStateObserver,
public AssistantManagerService::CommunicationErrorObserver, public AssistantManagerService::CommunicationErrorObserver,
public AssistantManagerService::StateObserver { public AssistantManagerService::StateObserver,
public ash::AmbientModeStateObserver {
public: public:
Service(mojo::PendingReceiver<mojom::AssistantService> receiver, Service(mojo::PendingReceiver<mojom::AssistantService> receiver,
std::unique_ptr<network::PendingSharedURLLoaderFactory> std::unique_ptr<network::PendingSharedURLLoaderFactory>
...@@ -131,6 +133,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service ...@@ -131,6 +133,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
// AssistantManagerService::StateObserver overrides: // AssistantManagerService::StateObserver overrides:
void OnStateChanged(AssistantManagerService::State new_state) override; void OnStateChanged(AssistantManagerService::State new_state) override;
// ash::AmbientModeStateObserver overrides:
void OnAmbientModeEnabled(bool enabled) override;
void UpdateAssistantManagerState(); void UpdateAssistantManagerState();
identity::mojom::IdentityAccessor* GetIdentityAccessor(); identity::mojom::IdentityAccessor* GetIdentityAccessor();
...@@ -191,9 +196,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service ...@@ -191,9 +196,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
bool locked_ = false; bool locked_ = false;
// Whether the power source is connected. // Whether the power source is connected.
bool power_source_connected_ = false; bool power_source_connected_ = false;
// In the signed-out mode, we are going to run Assistant service without
// using user's signed in account information.
bool is_signed_out_mode_ = false;
// The value passed into |SetAssistantManagerServiceForTesting|. // The value passed into |SetAssistantManagerServiceForTesting|.
// Will be moved into |assistant_manager_service_| when the service is // Will be moved into |assistant_manager_service_| when the service is
......
...@@ -13,12 +13,14 @@ ...@@ -13,12 +13,14 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "base/test/test_mock_time_task_runner.h" #include "base/test/test_mock_time_task_runner.h"
#include "base/time/tick_clock.h" #include "base/time/tick_clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "chromeos/audio/cras_audio_handler.h" #include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/dbus/power/fake_power_manager_client.h" #include "chromeos/dbus/power/fake_power_manager_client.h"
#include "chromeos/services/assistant/assistant_state_proxy.h" #include "chromeos/services/assistant/assistant_state_proxy.h"
#include "chromeos/services/assistant/fake_assistant_manager_service_impl.h" #include "chromeos/services/assistant/fake_assistant_manager_service_impl.h"
...@@ -167,6 +169,9 @@ class AssistantServiceTest : public testing::Test { ...@@ -167,6 +169,9 @@ class AssistantServiceTest : public testing::Test {
~AssistantServiceTest() override = default; ~AssistantServiceTest() override = default;
void SetUp() override { void SetUp() override {
scoped_feature_list_.InitAndEnableFeature(
chromeos::features::kAmbientModeFeature);
chromeos::CrasAudioHandler::InitializeForTesting(); chromeos::CrasAudioHandler::InitializeForTesting();
PowerManagerClient::InitializeFake(); PowerManagerClient::InitializeFake();
...@@ -229,8 +234,11 @@ class AssistantServiceTest : public testing::Test { ...@@ -229,8 +234,11 @@ class AssistantServiceTest : public testing::Test {
return mock_task_runner_.get(); return mock_task_runner_.get();
} }
ash::AmbientModeState* ambient_mode_state() { return &ambient_mode_state_; }
private: private:
base::test::TaskEnvironment task_environment_; base::test::TaskEnvironment task_environment_;
base::test::ScopedFeatureList scoped_feature_list_;
std::unique_ptr<Service> service_; std::unique_ptr<Service> service_;
mojo::Remote<mojom::AssistantService> remote_service_; mojo::Remote<mojom::AssistantService> remote_service_;
...@@ -248,6 +256,8 @@ class AssistantServiceTest : public testing::Test { ...@@ -248,6 +256,8 @@ class AssistantServiceTest : public testing::Test {
scoped_refptr<base::TestMockTimeTaskRunner> mock_task_runner_; scoped_refptr<base::TestMockTimeTaskRunner> mock_task_runner_;
std::unique_ptr<base::OneShotTimer> mock_timer_; std::unique_ptr<base::OneShotTimer> mock_timer_;
ash::AmbientModeState ambient_mode_state_;
DISALLOW_COPY_AND_ASSIGN(AssistantServiceTest); DISALLOW_COPY_AND_ASSIGN(AssistantServiceTest);
}; };
...@@ -368,5 +378,23 @@ TEST_F(AssistantServiceTest, ShouldSetClientStatusToNotReadyWhenStopped) { ...@@ -368,5 +378,23 @@ TEST_F(AssistantServiceTest, ShouldSetClientStatusToNotReadyWhenStopped) {
EXPECT_EQ(client()->status(), ash::mojom::AssistantState::NOT_READY); EXPECT_EQ(client()->status(), ash::mojom::AssistantState::NOT_READY);
} }
TEST_F(AssistantServiceTest,
ShouldResetAccessTokenWhenAmbientModeStateChanged) {
assistant_manager()->FinishStart();
EXPECT_EQ(assistant_manager()->GetState(),
AssistantManagerService::State::RUNNING);
ASSERT_TRUE(assistant_manager()->access_token().has_value());
ASSERT_EQ(assistant_manager()->access_token().value(), "fake access token");
ambient_mode_state()->SetAmbientModeEnabled(true);
base::RunLoop().RunUntilIdle();
ASSERT_FALSE(assistant_manager()->access_token().has_value());
ambient_mode_state()->SetAmbientModeEnabled(false);
base::RunLoop().RunUntilIdle();
ASSERT_TRUE(assistant_manager()->access_token().has_value());
ASSERT_EQ(assistant_manager()->access_token().value(), "fake access token");
}
} // namespace assistant } // namespace assistant
} // namespace chromeos } // namespace chromeos
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment