Commit 9084aa4c authored by Kristi Park's avatar Kristi Park Committed by Commit Bot

Revert "assistant: Fix mic button doesn't work with fake gaia account."

This reverts commit 75161d96.

Reason for revert: Tentative suspect for
DriveIntegrationServiceWithGaiaDisabledBrowserTest.DriveDisabled
linux-chromeos-chrome browser_test failure
https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/3090

Original change's description:
> assistant: Fix mic button doesn't work with fake gaia account.
> 
> |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}

TBR=xiaohuic@chromium.org,wutao@chromium.org,meilinw@chromium.org

Change-Id: I0c369b2ec7b41207cd1bb4d7e1953f758136f887
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/147442943
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2053164Reviewed-by: default avatarKristi Park <kristipark@chromium.org>
Commit-Queue: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740892}
parent 3336a9cc
...@@ -189,14 +189,12 @@ Service::~Service() { ...@@ -189,14 +189,12 @@ 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 // Add null check for |AmbientModeState| in case that |Service| is released
// after ash has gone. // after ash has gone.
auto* const ambient_mode_state = ash::AmbientModeState::Get(); if (chromeos::features::IsAmbientModeEnabled() &&
if (chromeos::features::IsAmbientModeEnabled() && ambient_mode_state) ash::AmbientModeState::Get())
ambient_mode_state->RemoveObserver(this); 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 (session_controller && account_id_.is_valid()) { if (observing_ash_session_ && session_controller) {
session_controller->RemoveSessionActivationObserverForAccountId(account_id_, session_controller->RemoveSessionActivationObserverForAccountId(account_id_,
this); this);
} }
...@@ -437,22 +435,12 @@ void Service::UpdateAssistantManagerState() { ...@@ -437,22 +435,12 @@ 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() { void Service::RequestAccessToken() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Bypass access token fetching when service is running in signed-out mode. // 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; return;
}
if (access_token_fetcher_) { if (access_token_fetcher_) {
LOG(WARNING) << "Access token already requested."; LOG(WARNING) << "Access token already requested.";
...@@ -460,13 +448,18 @@ void Service::RequestAccessToken() { ...@@ -460,13 +448,18 @@ void Service::RequestAccessToken() {
} }
VLOG(1) << "Start requesting access token."; VLOG(1) << "Start requesting access token.";
CoreAccountInfo account_info = RetrievePrimaryAccountInfo(); CoreAccountInfo account_info = identity_manager_->GetPrimaryAccountInfo(
signin::ConsentLevel::kNotRequired);
CHECK(!account_info.account_id.empty());
CHECK(!account_info.gaia.empty());
if (!identity_manager_->HasAccountWithRefreshToken(account_info.account_id)) { if (!identity_manager_->HasAccountWithRefreshToken(account_info.account_id)) {
LOG(ERROR) << "Failed to retrieve primary account info. Retrying."; LOG(ERROR) << "Failed to retrieve primary account info.";
RetryRefreshToken(); RetryRefreshToken();
return; return;
} }
account_id_ = user_manager::known_user::GetAccountId(
account_info.email, account_info.gaia, AccountType::GOOGLE);
identity::ScopeSet scopes; identity::ScopeSet scopes;
scopes.insert(kScopeAssistant); scopes.insert(kScopeAssistant);
scopes.insert(kScopeAuthGcm); scopes.insert(kScopeAuthGcm);
...@@ -554,31 +547,29 @@ void Service::FinalizeAssistantManagerService() { ...@@ -554,31 +547,29 @@ void Service::FinalizeAssistantManagerService() {
assistant_manager_service_->GetState() == assistant_manager_service_->GetState() ==
AssistantManagerService::RUNNING); AssistantManagerService::RUNNING);
// Ensure one-time mojom initialization. // Using session_observer_binding_ as a flag to control onetime initialization
if (is_assistant_manager_service_finalized_) if (!observing_ash_session_) {
return; // Bind to the AssistantController in ash.
is_assistant_manager_service_finalized_ = true; client_->RequestAssistantController(
assistant_controller_.BindNewPipeAndPassReceiver());
// Bind to the AssistantController in ash. mojo::PendingRemote<mojom::Assistant> remote_for_controller;
client_->RequestAssistantController( BindAssistant(remote_for_controller.InitWithNewPipeAndPassReceiver());
assistant_controller_.BindNewPipeAndPassReceiver()); assistant_controller_->SetAssistant(std::move(remote_for_controller));
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. // Bind to the AssistantAlarmTimerController in ash.
client_->RequestAssistantAlarmTimerController( client_->RequestAssistantAlarmTimerController(
assistant_alarm_timer_controller_.BindNewPipeAndPassReceiver()); assistant_alarm_timer_controller_.BindNewPipeAndPassReceiver());
// Bind to the AssistantNotificationController in ash. // Bind to the AssistantNotificationController in ash.
client_->RequestAssistantNotificationController( client_->RequestAssistantNotificationController(
assistant_notification_controller_.BindNewPipeAndPassReceiver()); assistant_notification_controller_.BindNewPipeAndPassReceiver());
// Bind to the AssistantScreenContextController in ash. // Bind to the AssistantScreenContextController in ash.
client_->RequestAssistantScreenContextController( client_->RequestAssistantScreenContextController(
assistant_screen_context_controller_.BindNewPipeAndPassReceiver()); assistant_screen_context_controller_.BindNewPipeAndPassReceiver());
AddAshSessionObserver(); AddAshSessionObserver();
}
} }
void Service::StopAssistantManagerService() { void Service::StopAssistantManagerService() {
...@@ -592,14 +583,9 @@ void Service::StopAssistantManagerService() { ...@@ -592,14 +583,9 @@ void Service::StopAssistantManagerService() {
void Service::AddAshSessionObserver() { void Service::AddAshSessionObserver() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
observing_ash_session_ = true;
// No session controller in unittest. // No session controller in unittest.
if (ash::SessionController::Get()) { 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( ash::SessionController::Get()->AddSessionActivationObserverForAccountId(
account_id_, this); account_id_, this);
} }
......
...@@ -26,7 +26,6 @@ ...@@ -26,7 +26,6 @@
#include "chromeos/services/assistant/public/mojom/assistant.mojom.h" #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
#include "chromeos/services/assistant/public/mojom/settings.mojom.h" #include "chromeos/services/assistant/public/mojom/settings.mojom.h"
#include "components/account_id/account_id.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_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/receiver.h"
...@@ -141,8 +140,14 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service ...@@ -141,8 +140,14 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
void UpdateAssistantManagerState(); void UpdateAssistantManagerState();
CoreAccountInfo RetrievePrimaryAccountInfo();
void RequestAccessToken(); 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, void GetAccessTokenCallback(GoogleServiceAuthError error,
signin::AccessTokenInfo access_token_info); signin::AccessTokenInfo access_token_info);
void RetryRefreshToken(); void RetryRefreshToken();
...@@ -169,10 +174,12 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service ...@@ -169,10 +174,12 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
mojo::Receiver<mojom::AssistantService> receiver_; mojo::Receiver<mojom::AssistantService> receiver_;
mojo::ReceiverSet<mojom::Assistant> assistant_receivers_; mojo::ReceiverSet<mojom::Assistant> assistant_receivers_;
bool observing_ash_session_ = false;
mojo::Remote<mojom::Client> client_; mojo::Remote<mojom::Client> client_;
mojo::Remote<mojom::DeviceActions> device_actions_; mojo::Remote<mojom::DeviceActions> device_actions_;
signin::IdentityManager* const identity_manager_; signin::IdentityManager* const identity_manager_;
AccountId account_id_; AccountId account_id_;
std::unique_ptr<AssistantManagerService> assistant_manager_service_; std::unique_ptr<AssistantManagerService> assistant_manager_service_;
std::unique_ptr<base::OneShotTimer> token_refresh_timer_; std::unique_ptr<base::OneShotTimer> token_refresh_timer_;
...@@ -182,8 +189,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service ...@@ -182,8 +189,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
chromeos::PowerManagerClient::Observer> chromeos::PowerManagerClient::Observer>
power_manager_observer_{this}; 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. // Whether the current user session is active.
bool session_active_ = false; bool session_active_ = false;
// Whether the lock screen is on. // Whether the lock screen is on.
......
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