Skip to content

Commit

Permalink
assistant: Fix mic button doesn't work with fake gaia account.
Browse files Browse the repository at this point in the history 
|Service| stores the current account info and later registered as a
session activation observer for this account, so that it can enable
listening when the session becomes active. Previously when the login
account was a fake gaia account, e.g. in Tast tests, we did not update
the stored account info with the latest primary account when requesting
access token. In this way, |Service| couldn't receive any session active
signals because it was not observing a valid account.

Bug: b/147442943
Test: run the service in signed-out mode and manually verified.

Change-Id: Iaaef86303686291b419754505ead9b14cca0adbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033642
Commit-Queue: Meilin Wang <meilinw@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740818}
  • Loading branch information
Meilin Wang authored and Commit Bot committed Feb 12, 2020
1 parent 64cf44e commit 75161d9
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 42 deletions.
80 changes: 47 additions & 33 deletions chromeos/services/assistant/service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,14 @@ Service::~Service() {
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);
auto* const ambient_mode_state = ash::AmbientModeState::Get();
if (chromeos::features::IsAmbientModeEnabled() && ambient_mode_state)
ambient_mode_state->RemoveObserver(this);

assistant_state_.RemoveObserver(this);

auto* const session_controller = ash::SessionController::Get();
if (observing_ash_session_ && session_controller) {
if (session_controller && account_id_.is_valid()) {
session_controller->RemoveSessionActivationObserverForAccountId(account_id_,
this);
}
Expand Down Expand Up @@ -435,31 +437,36 @@ void Service::UpdateAssistantManagerState() {
}
}

CoreAccountInfo Service::RetrievePrimaryAccountInfo() {
CoreAccountInfo account_info = identity_manager_->GetPrimaryAccountInfo(
signin::ConsentLevel::kNotRequired);
CHECK(!account_info.account_id.empty());
CHECK(!account_info.gaia.empty());
return account_info;
}

void Service::RequestAccessToken() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

// Bypass access token fetching when service is running in signed-out mode.
if (IsSignedOutMode())
if (IsSignedOutMode()) {
VLOG(1) << "Signed out mode detected, bypass access token fetching.";
return;
}

if (access_token_fetcher_) {
LOG(WARNING) << "Access token already requested.";
return;
}

VLOG(1) << "Start requesting access token.";
CoreAccountInfo account_info = identity_manager_->GetPrimaryAccountInfo(
signin::ConsentLevel::kNotRequired);
CHECK(!account_info.account_id.empty());
CHECK(!account_info.gaia.empty());

CoreAccountInfo account_info = RetrievePrimaryAccountInfo();
if (!identity_manager_->HasAccountWithRefreshToken(account_info.account_id)) {
LOG(ERROR) << "Failed to retrieve primary account info.";
LOG(ERROR) << "Failed to retrieve primary account info. Retrying.";
RetryRefreshToken();
return;
}
account_id_ = user_manager::known_user::GetAccountId(
account_info.email, account_info.gaia, AccountType::GOOGLE);

identity::ScopeSet scopes;
scopes.insert(kScopeAssistant);
scopes.insert(kScopeAuthGcm);
Expand Down Expand Up @@ -547,29 +554,31 @@ void Service::FinalizeAssistantManagerService() {
assistant_manager_service_->GetState() ==
AssistantManagerService::RUNNING);

// Using session_observer_binding_ as a flag to control onetime initialization
if (!observing_ash_session_) {
// Bind to the AssistantController in ash.
client_->RequestAssistantController(
assistant_controller_.BindNewPipeAndPassReceiver());
mojo::PendingRemote<mojom::Assistant> remote_for_controller;
BindAssistant(remote_for_controller.InitWithNewPipeAndPassReceiver());
assistant_controller_->SetAssistant(std::move(remote_for_controller));
// Ensure one-time mojom initialization.
if (is_assistant_manager_service_finalized_)
return;
is_assistant_manager_service_finalized_ = true;

// Bind to the AssistantController in ash.
client_->RequestAssistantController(
assistant_controller_.BindNewPipeAndPassReceiver());
mojo::PendingRemote<mojom::Assistant> remote_for_controller;
BindAssistant(remote_for_controller.InitWithNewPipeAndPassReceiver());
assistant_controller_->SetAssistant(std::move(remote_for_controller));

// Bind to the AssistantAlarmTimerController in ash.
client_->RequestAssistantAlarmTimerController(
assistant_alarm_timer_controller_.BindNewPipeAndPassReceiver());
// Bind to the AssistantAlarmTimerController in ash.
client_->RequestAssistantAlarmTimerController(
assistant_alarm_timer_controller_.BindNewPipeAndPassReceiver());

// Bind to the AssistantNotificationController in ash.
client_->RequestAssistantNotificationController(
assistant_notification_controller_.BindNewPipeAndPassReceiver());
// Bind to the AssistantNotificationController in ash.
client_->RequestAssistantNotificationController(
assistant_notification_controller_.BindNewPipeAndPassReceiver());

// Bind to the AssistantScreenContextController in ash.
client_->RequestAssistantScreenContextController(
assistant_screen_context_controller_.BindNewPipeAndPassReceiver());
// Bind to the AssistantScreenContextController in ash.
client_->RequestAssistantScreenContextController(
assistant_screen_context_controller_.BindNewPipeAndPassReceiver());

AddAshSessionObserver();
}
AddAshSessionObserver();
}

void Service::StopAssistantManagerService() {
Expand All @@ -583,9 +592,14 @@ void Service::StopAssistantManagerService() {
void Service::AddAshSessionObserver() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

observing_ash_session_ = true;
// No session controller in unittest.
if (ash::SessionController::Get()) {
// Note that this account can either be a regular account using real gaia,
// or a fake gaia account.
CoreAccountInfo account_info = RetrievePrimaryAccountInfo();
account_id_ = user_manager::known_user::GetAccountId(
account_info.email, account_info.gaia, AccountType::GOOGLE);
DCHECK(account_id_.is_valid());
ash::SessionController::Get()->AddSessionActivationObserverForAccountId(
account_id_, this);
}
Expand Down
13 changes: 4 additions & 9 deletions chromeos/services/assistant/service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
#include "chromeos/services/assistant/public/mojom/settings.mojom.h"
#include "components/account_id/account_id.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
Expand Down Expand Up @@ -140,14 +141,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service

void UpdateAssistantManagerState();

CoreAccountInfo RetrievePrimaryAccountInfo();
void RequestAccessToken();

void GetUnconsentedPrimaryAccountInfoCallback(
const base::Optional<CoreAccountId>& account_id,
const base::Optional<std::string>& gaia,
const base::Optional<std::string>& email,
const identity::AccountState& account_state);

void GetAccessTokenCallback(GoogleServiceAuthError error,
signin::AccessTokenInfo access_token_info);
void RetryRefreshToken();
Expand All @@ -174,12 +169,10 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
mojo::Receiver<mojom::AssistantService> receiver_;
mojo::ReceiverSet<mojom::Assistant> assistant_receivers_;

bool observing_ash_session_ = false;
mojo::Remote<mojom::Client> client_;
mojo::Remote<mojom::DeviceActions> device_actions_;

signin::IdentityManager* const identity_manager_;

AccountId account_id_;
std::unique_ptr<AssistantManagerService> assistant_manager_service_;
std::unique_ptr<base::OneShotTimer> token_refresh_timer_;
Expand All @@ -189,6 +182,8 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
chromeos::PowerManagerClient::Observer>
power_manager_observer_{this};

// Flag to guard the one-time mojom initialization.
bool is_assistant_manager_service_finalized_ = false;
// Whether the current user session is active.
bool session_active_ = false;
// Whether the lock screen is on.
Expand Down

0 comments on commit 75161d9

Please sign in to comment.