arc: Implement safe access to ArcAuthService.

chrome/browser/chromeos/arc/arc_auth_service.cc

Index: chrome/browser/chromeos/arc/arc_auth_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_service.cc b/chrome/browser/chromeos/arc/arc_auth_service.cc
index b9e6986c54d8648191e565c52f478c4d283041e0..ed8da4a40f936f2653ee87815e2db78e8f01b176 100644
--- a/chrome/browser/chromeos/arc/arc_auth_service.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_service.cc
@@ -57,6 +57,9 @@ ArcAuthService* g_arc_auth_service = nullptr;
 // Skip creating UI in unit tests
 bool g_disable_ui_for_testing = false;
 
+// Skip checking for cryptohome data ephemeral users.
+bool g_allow_cryptohome_data_ephemeral_user_for_testing = false;

[khmel, 2016/07/18 20:23:42]

In most browser tests user is logged as Ephemeral user, which is disabled by
Arc. As I investigated there is no easy ways to make user non-Ephemeral. There
are Login browser tests that may do this however they have different hierarchy,
incompatible with AppList browser tests.

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/login_mana...

There is yet another sample ArcAuthBrowser Tests. However it is support is very
massive and I am not sure that this makes sense to add to AppList browser tests.

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/arc_auth_ser...


Easiest way to solve this is to allow Ephemeral users for Arc tests.

WDYT?

[xiyuan, 2016/07/18 20:59:12]

Sounds like UserManagerBase::IsUserCryptohomeDataEphemeral is not doing a
correct job here. We take a shortcut there by checking whether user is persisted
in the list to test ephemeral. This is not 100% correct. We probably should also
check AreEphemeralUsersEnabled() in [1].

[1]
https://cs.chromium.org/chromium/src/components/user_manager/user_manager_bas...

Can you try if that could work without adding
g_allow_cryptohome_data_ephemeral_user_for_testing ?

[khmel, 2016/07/18 22:15:11]

Yes, this helps

+
 // Use specified ash::ShelfDelegate for unit tests.
 ash::ShelfDelegate* g_shelf_delegate_for_testing = nullptr;
 
@@ -133,6 +136,11 @@ void ArcAuthService::DisableUIForTesting() {
 }
 
 // static
+void ArcAuthService::AllowCryptohomeDataEphemeralUserForTesting() {
+  g_allow_cryptohome_data_ephemeral_user_for_testing = true;
+}
+
+// static
 void ArcAuthService::SetShelfDelegateForTesting(
     ash::ShelfDelegate* shelf_delegate) {
   g_shelf_delegate_for_testing = shelf_delegate;
@@ -178,7 +186,8 @@ bool ArcAuthService::IsAllowedForProfile(const Profile* profile) {
     return false;
   }
 
-  if (user_manager::UserManager::Get()
+  if (!g_allow_cryptohome_data_ephemeral_user_for_testing &&
+      user_manager::UserManager::Get()
           ->IsCurrentUserCryptohomeDataEphemeral()) {
     VLOG(2) << "Users with ephemeral data are not supported in Arc.";
     return false;
@@ -342,9 +351,6 @@ void ArcAuthService::OnPrimaryUserProfilePrepared(Profile* profile) {
 
   Shutdown();
 
-  profile_ = profile;
-  SetState(State::STOPPED);
-
   if (!IsAllowedForProfile(profile))
     return;
 
@@ -354,6 +360,9 @@ void ArcAuthService::OnPrimaryUserProfilePrepared(Profile* profile) {
     return;
   }
 
+  profile_ = profile;
+  SetState(State::STOPPED);
+
   PrefServiceSyncableFromProfile(profile_)->AddSyncedPrefObserver(
       prefs::kArcEnabled, this);
 
@@ -637,6 +646,9 @@ bool ArcAuthService::IsArcManaged() const {
 
 bool ArcAuthService::IsArcEnabled() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!IsAllowed())
+    return false;
+
   DCHECK(profile_);
   return profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled);
 }