arc: kArcCompatibleFilesystemChosen pref to local state and integer.

chrome/browser/chromeos/login/session/user_session_manager.cc

Index: chrome/browser/chromeos/login/session/user_session_manager.cc
diff --git a/chrome/browser/chromeos/login/session/user_session_manager.cc b/chrome/browser/chromeos/login/session/user_session_manager.cc
index 1d6b4e5ae10ae12a9c719d4a7d3c651f57a38561..e313762582912b84370aac25aa1d785db56547fa 100644
--- a/chrome/browser/chromeos/login/session/user_session_manager.cc
+++ b/chrome/browser/chromeos/login/session/user_session_manager.cc
@@ -513,6 +513,27 @@ void UserSessionManager::StartSession(
                                           user_context.GetDeviceId());
   }
 
+  // If the filesystem compatibiliy with ARC is not established yet, test it.

[Luis Héctor Chávez, 2017/04/13 21:11:47]

nit: s/compatibiliy/compatibility/

[kinaba, 2017/04/14 04:33:55]

Acknowledged.

+  int arc_filesystem_compatibility = arc::kFileSystemIncompatible;

[Luis Héctor Chávez, 2017/04/13 21:11:47]

nit: s/int/arc::FileSystemCompatibilityState/

[kinaba, 2017/04/14 04:33:55]

GetIntegerPref needs to take int* so it cannot be an enum type unfortunately.
I factored out the pref reader code to
arc_util.cc::GetFileSystemCompatibilityPref to minimize the use of raw int type.

+  user_manager::known_user::GetIntegerPref(
+      user_context_.GetAccountId(), prefs::kArcCompatibleFilesystemChosen,
+      &arc_filesystem_compatibility);
+  if (arc_filesystem_compatibility == arc::kFileSystemIncompatible) {

[xiyuan, 2017/04/13 16:35:59]

What about old devices that are never going to be compatible? This would make
login slow for them no matter what.

I wonder whether it makes sense to check IsArcAvailable (or something similar)
to limit fs check to only targeted devices.

[Luis Héctor Chávez, 2017/04/13 21:11:47]

When you move the ownership of the pref to arc_util.cc, add the IsArcAvailable
check there.

[kinaba, 2017/04/14 04:33:55]

Thanks for the suggestions. Both done (IsArcAvailable + move to arc_util)

+    base::PostTaskWithTraitsAndReplyWithResult(
+        FROM_HERE,
+        base::TaskTraits()
+            .WithShutdownBehavior(
+                base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
+            .WithPriority(base::TaskPriority::USER_BLOCKING)
+            .MayBlock(),
+        base::Bind(&arc::IsArcCompatibleFilesystem,
+                   ProfileHelper::GetProfilePathByUserIdHash(
+                       user_context_.GetUserIDHash())),
+        base::Bind(&UserSessionManager::SetFileSystemPrefAndPrepareProfile,
+                   AsWeakPtr()));
+    return;
+  }
+
   PrepareProfile();
 }
 
@@ -983,24 +1004,17 @@ void UserSessionManager::StartCrosSession() {
   btl->AddLoginTimeMarker("StartSession-End", false);
 }
 
-void UserSessionManager::PrepareProfile() {
-  const base::FilePath profile_path =
-      ProfileHelper::GetProfilePathByUserIdHash(user_context_.GetUserIDHash());
-
-  base::PostTaskWithTraitsAndReplyWithResult(
-      FROM_HERE,
-      base::TaskTraits()
-          .WithShutdownBehavior(
-              base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
-          .WithPriority(base::TaskPriority::USER_BLOCKING)
-          .MayBlock(),
-      base::Bind(&arc::IsArcCompatibleFilesystem, profile_path),
-      base::Bind(&UserSessionManager::PrepareProfileAfterFilesystemCheck,
-                 AsWeakPtr()));
-}
-
-void UserSessionManager::PrepareProfileAfterFilesystemCheck(
+void UserSessionManager::SetFileSystemPrefAndPrepareProfile(
     bool arc_compatible_filesystem) {
+  if (arc_compatible_filesystem) {
+    user_manager::known_user::SetIntegerPref(

[Luis Héctor Chávez, 2017/04/13 21:11:47]

Is it possible to centralize the ownership of pref registering/setting/querying
to a single file? arc_util.h seems like a good candidate.

[kinaba, 2017/04/14 04:33:55]

Done.

+        user_context_.GetAccountId(), prefs::kArcCompatibleFilesystemChosen,
+        arc::kFileSystemCompatible);
+  }
+  PrepareProfile();
+}
+
+void UserSessionManager::PrepareProfile() {
   const bool is_demo_session =
       DemoAppLauncher::IsDemoAppSession(user_context_.GetAccountId());
 
@@ -1009,13 +1023,12 @@ void UserSessionManager::PrepareProfileAfterFilesystemCheck(
   g_browser_process->profile_manager()->CreateProfileAsync(
       ProfileHelper::GetProfilePathByUserIdHash(user_context_.GetUserIDHash()),
       base::Bind(&UserSessionManager::OnProfileCreated, AsWeakPtr(),
-                 user_context_, is_demo_session, arc_compatible_filesystem),
+                 user_context_, is_demo_session),
       base::string16(), std::string(), std::string());
 }
 
 void UserSessionManager::OnProfileCreated(const UserContext& user_context,
                                           bool is_incognito_profile,
-                                          bool arc_compatible_filesystem,
                                           Profile* profile,
                                           Profile::CreateStatus status) {
   CHECK(profile);
@@ -1023,7 +1036,7 @@ void UserSessionManager::OnProfileCreated(const UserContext& user_context,
   switch (status) {
     case Profile::CREATE_STATUS_CREATED:
       // Profile created but before initializing extensions and promo resources.
-      InitProfilePreferences(profile, user_context, arc_compatible_filesystem);
+      InitProfilePreferences(profile, user_context);
       break;
     case Profile::CREATE_STATUS_INITIALIZED:
       // Profile is created, extensions and promo resources are initialized.
@@ -1043,11 +1056,7 @@ void UserSessionManager::OnProfileCreated(const UserContext& user_context,
 
 void UserSessionManager::InitProfilePreferences(
     Profile* profile,
-    const UserContext& user_context,
-    bool arc_compatible_filesystem) {
-  profile->GetPrefs()->SetBoolean(prefs::kArcCompatibleFilesystemChosen,
-                                  arc_compatible_filesystem);
-
+    const UserContext& user_context) {
   const user_manager::User* user =
       ProfileHelper::Get()->GetUserByProfile(profile);
   if (user->GetType() == user_manager::USER_TYPE_KIOSK_APP &&