Commit fc5df617 authored by Meilin Wang's avatar Meilin Wang Committed by Commit Bot

assistant: fix autotest/tast tests failed due to service disallowed.

Tests failed because assistant was disallowed by account type for the
following reasons:
- Autotest with real gaia login:
  Caused by a race between chromeos login finished and account info fetch
  finished.
- Tast with fake gaia login:
  Account info fetch not be faked under fake gaia env.

This changes fixes these issues by first retry to initiate assistant when
account info updated, and also bypass the account type check in
assistant::IsAssistantAllowedForProfile under fake gaia env.

Misc:
Add IsGaiaServicesDisabled helper function in chromeos_swithes.

Bug: 942781, 939146, 940618
Test: local compile and run autotest/tast tests on DUT.
Change-Id: I25bdcda9587988426745c2dd036da1b8be6f636a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1530119
Commit-Queue: Meilin Wang <meilinw@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#642370}
parent 2deb08f7
......@@ -69,7 +69,11 @@ ash::mojom::AssistantAllowedState IsAssistantAllowedForProfile(
return ash::mojom::AssistantAllowedState::DISALLOWED_BY_LOCALE;
}
if (!ui::DeviceUsesKeyboardLayout2()) {
// Bypass the account type check when using fake gaia login, e.g. in Tast
// tests, or the account is logged in a device with a physical Assistant key
// on keyboard.
if (!chromeos::switches::IsGaiaServicesDisabled() &&
!ui::DeviceUsesKeyboardLayout2()) {
// Only enable non-dasher accounts for devices without physical key.
bool account_supported = false;
auto* identity_manager =
......
......@@ -10,6 +10,7 @@
#include "chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.h"
#include "chrome/browser/chromeos/assistant/assistant_util.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/ui/ash/assistant/assistant_context_util.h"
#include "chrome/browser/ui/ash/assistant/assistant_image_downloader.h"
#include "chrome/browser/ui/ash/assistant/assistant_setup.h"
......@@ -36,9 +37,19 @@ AssistantClient::AssistantClient()
AssistantClient::~AssistantClient() {
DCHECK(g_instance);
g_instance = nullptr;
if (identity_manager_)
identity_manager_->RemoveObserver(this);
}
void AssistantClient::MaybeInit(Profile* profile) {
if (!profile_) {
profile_ = profile;
identity_manager_ = IdentityManagerFactory::GetForProfile(profile_);
identity_manager_->AddObserver(this);
}
DCHECK_EQ(profile_, profile);
if (assistant::IsAssistantAllowedForProfile(profile) !=
ash::mojom::AssistantAllowedState::ALLOWED) {
return;
......@@ -86,3 +97,10 @@ void AssistantClient::RequestAssistantStructure(
RequestAssistantStructureCallback callback) {
RequestAssistantStructureForActiveBrowserWindow(std::move(callback));
}
void AssistantClient::OnExtendedAccountInfoUpdated(const AccountInfo& info) {
if (initialized_)
return;
MaybeInit(profile_);
}
......@@ -11,13 +11,15 @@
#include "chrome/browser/ui/ash/assistant/device_actions.h"
#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "services/identity/public/cpp/identity_manager.h"
class Profile;
class AssistantImageDownloader;
class AssistantSetup;
// Class to handle all assistant in-browser-process functionalities.
class AssistantClient : chromeos::assistant::mojom::Client {
class AssistantClient : chromeos::assistant::mojom::Client,
public identity::IdentityManager::Observer {
public:
static AssistantClient* Get();
......@@ -33,6 +35,14 @@ class AssistantClient : chromeos::assistant::mojom::Client {
RequestAssistantStructureCallback callback) override;
private:
// identity::IdentityManager::Observer:
// Retry to initiate Assistant service when account info has been updated.
// This is necessary if previous calls of MaybeInit() failed due to Assistant
// disallowed by account type. This can happen when the chromeos sign-in
// finished before account info fetching is finished (|hosted_domain| field
// will be empty under this case).
void OnExtendedAccountInfoUpdated(const AccountInfo& info) override;
mojo::Binding<chromeos::assistant::mojom::Client> client_binding_;
mojo::Binding<chromeos::assistant::mojom::DeviceActions>
device_actions_binding_;
......@@ -45,6 +55,10 @@ class AssistantClient : chromeos::assistant::mojom::Client {
bool initialized_ = false;
// Non-owning pointers.
Profile* profile_ = nullptr;
identity::IdentityManager* identity_manager_ = nullptr;
DISALLOW_COPY_AND_ASSIGN(AssistantClient);
};
......
......@@ -647,5 +647,10 @@ bool ShouldSkipOobePostLogin() {
return base::CommandLine::ForCurrentProcess()->HasSwitch(kOobeSkipPostLogin);
}
bool IsGaiaServicesDisabled() {
return base::CommandLine::ForCurrentProcess()->HasSwitch(
kDisableGaiaServices);
}
} // namespace switches
} // namespace chromeos
......@@ -292,6 +292,9 @@ COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool ShouldShowPlayStoreInDemoMode();
// Returns true if we should skip all other OOBE pages after user login.
COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool ShouldSkipOobePostLogin();
// Returns true if GAIA services has been disabled.
COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsGaiaServicesDisabled();
} // namespace switches
} // 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