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: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#642370}(cherry picked from commit fc5df617da765bb5faafa924628d5a8f7afc4211)
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1535267
Reviewed-by: Yue Li <updowndota@chromium.org>
Cr-Commit-Position: refs/branch-heads/3729@{#375}
Cr-Branched-From: d4a8972e30b604f090aeda5dfff68386ae656267-refs/heads/master@{#638880}diff --git a/chrome/browser/chromeos/assistant/assistant_util.cc b/chrome/browser/chromeos/assistant/assistant_util.cc
index f428d7f..a99b29f5 100644
--- a/chrome/browser/chromeos/assistant/assistant_util.cc
+++ b/chrome/browser/chromeos/assistant/assistant_util.cc
@@ -72,7 +72,11 @@
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 =
diff --git a/chrome/browser/ui/ash/assistant/assistant_client.cc b/chrome/browser/ui/ash/assistant/assistant_client.cc
index a6508e2..76dd233 100644
--- a/chrome/browser/ui/ash/assistant/assistant_client.cc
+++ b/chrome/browser/ui/ash/assistant/assistant_client.cc
@@ -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() {
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 @@
RequestAssistantStructureCallback callback) {
RequestAssistantStructureForActiveBrowserWindow(std::move(callback));
}
+
+void AssistantClient::OnExtendedAccountInfoUpdated(const AccountInfo& info) {
+ if (initialized_)
+ return;
+
+ MaybeInit(profile_);
+}
diff --git a/chrome/browser/ui/ash/assistant/assistant_client.h b/chrome/browser/ui/ash/assistant/assistant_client.h
index eef5571..97c17c2 100644
--- a/chrome/browser/ui/ash/assistant/assistant_client.h
+++ b/chrome/browser/ui/ash/assistant/assistant_client.h
@@ -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 @@
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 @@
bool initialized_ = false;
+ // Non-owning pointers.
+ Profile* profile_ = nullptr;
+ identity::IdentityManager* identity_manager_ = nullptr;
+
DISALLOW_COPY_AND_ASSIGN(AssistantClient);
};
diff --git a/chromeos/constants/chromeos_switches.cc b/chromeos/constants/chromeos_switches.cc
index 98e2b8cb..b33df97 100644
--- a/chromeos/constants/chromeos_switches.cc
+++ b/chromeos/constants/chromeos_switches.cc
@@ -647,5 +647,10 @@
return base::CommandLine::ForCurrentProcess()->HasSwitch(kOobeSkipPostLogin);
}
+bool IsGaiaServicesDisabled() {
+ return base::CommandLine::ForCurrentProcess()->HasSwitch(
+ kDisableGaiaServices);
+}
+
} // namespace switches
} // namespace chromeos
diff --git a/chromeos/constants/chromeos_switches.h b/chromeos/constants/chromeos_switches.h
index c78173a47..9bf1260 100644
--- a/chromeos/constants/chromeos_switches.h
+++ b/chromeos/constants/chromeos_switches.h
@@ -292,6 +292,9 @@
// 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
