arc: kArcCompatibleFilesystemChosen pref to local state and integer.

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/login/session/user_session_manager.h"
 
 #include <stddef.h>
 
 #include <set>
 #include <string>
@@ skipping 495 matching lines @@
   CreateUserSession(user_context, has_auth_cookies);
 
   if (!has_active_session)
     StartCrosSession();
 
   if (!user_context.GetDeviceId().empty()) {
     user_manager::known_user::SetDeviceId(user_context.GetAccountId(),
                                           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();
 }
 
 void UserSessionManager::DelegateDeleted(UserSessionManagerDelegate* delegate) {
   if (delegate_ == delegate)
     delegate_ = nullptr;
 }
 
 void UserSessionManager::PerformPostUserLoggedInActions() {
   user_manager::UserManager* user_manager = user_manager::UserManager::Get();
@@ skipping 450 matching lines @@
 }
 
 void UserSessionManager::StartCrosSession() {
   BootTimesRecorder* btl = BootTimesRecorder::Get();
   btl->AddLoginTimeMarker("StartSession-Start", false);
   DBusThreadManager::Get()->GetSessionManagerClient()->StartSession(
       cryptohome::Identification(user_context_.GetAccountId()));
   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::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::PrepareProfileAfterFilesystemCheck(
-    bool arc_compatible_filesystem) {
+void UserSessionManager::PrepareProfile() {
   const bool is_demo_session =
       DemoAppLauncher::IsDemoAppSession(user_context_.GetAccountId());
 
   // TODO(nkostylev): Figure out whether demo session is using the right profile
   // path or not. See https://codereview.chromium.org/171423009
   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);
 
   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.
       // At this point all other Chrome OS services will be notified that it is
       // safe to use this profile.
       UserProfileInitialized(profile, is_incognito_profile,
                              user_context.GetAccountId());
       break;
     case Profile::CREATE_STATUS_LOCAL_FAIL:
     case Profile::CREATE_STATUS_REMOTE_FAIL:
     case Profile::CREATE_STATUS_CANCELED:
     case Profile::MAX_CREATE_STATUS:
       NOTREACHED();
       break;
   }
 }
 
 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 &&
       profile->IsNewProfile()) {
     ChromeUserManager::Get()->SetIsCurrentUserNew(true);
   }
 
   if (user->is_active()) {
     input_method::InputMethodManager* manager =
         input_method::InputMethodManager::Get();
@@ skipping 910 matching lines @@
           ->browser_policy_connector_chromeos()
           ->IsEnterpriseManaged()) {
     return false;
   }
 
   // Do not show end of life notification if this is a guest session
   return !profile->IsGuestSession();
 }
 
 }  // namespace chromeos