arc: kArcCompatibleFilesystemChosen pref to local state and integer.

chrome/browser/chromeos/arc/arc_util.cc

Index: chrome/browser/chromeos/arc/arc_util.cc
diff --git a/chrome/browser/chromeos/arc/arc_util.cc b/chrome/browser/chromeos/arc/arc_util.cc
index 7723792c34c6f22bc5eecadd693adb496c8f206d..3fbdd7aa9e3a0093fd5b6ce6928b1eb8ac29c2a5 100644
--- a/chrome/browser/chromeos/arc/arc_util.cc
+++ b/chrome/browser/chromeos/arc/arc_util.cc
@@ -19,6 +19,7 @@
 #include "chrome/common/pref_names.h"
 #include "components/arc/arc_util.h"
 #include "components/prefs/pref_service.h"
+#include "components/user_manager/known_user.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
 
@@ -121,10 +122,20 @@ bool IsArcAllowedInAppListForProfile(const Profile* profile) {
 }
 
 bool IsArcCompatibleFileSystemUsedForProfile(const Profile* profile) {
-  // chromeos::UserSessionManager::PrepareProfile does the actual file system
-  // check and stores the result to prefs, so that it survives crash-restart.
+  const user_manager::User* user =
+      chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
+  if (!user)
+    return false;

[hidehiko, 2017/04/13 12:38:27]

IIUC, this case is not expected even for testing?
Then, could you add NOTREACHED()? If it is expected, then could you add VLOG()
with comments?

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

Think this should only happen in tests. GetUserByProfile is based on profile
path and should work after a user is logged in and we know the user hash.

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

For instance, the sign-in profile does not have an user associated.

Currently this function is not called for such profiles, but its just because
IsArcAllowedForProfile() checks chromeos::ProfileHelper::IsPrimaryProfile
earlier than checking with this function--and the order is kind of arbitrary.
I'd like to keep the check with a comment added. WDYT?

+
+  // chromeos::UserSessionManager does the actual file system check and stores
+  // the result to prefs, so that it survives crash-restart.
+  int filesystem_compatibility = kFileSystemIncompatible;
+  user_manager::known_user::GetIntegerPref(
+      user->GetAccountId(), prefs::kArcCompatibleFilesystemChosen,
+      &filesystem_compatibility);
+
   const bool is_filesystem_compatible =
-      profile->GetPrefs()->GetBoolean(prefs::kArcCompatibleFilesystemChosen);
+      filesystem_compatibility >= kFileSystemCompatible;

[hidehiko, 2017/04/13 12:38:27]

Could you use "==" instead, just in case that a new value is added?
Or, could you comment a caution in the enum definition?

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

changed to "!= Incompatible" (to align with the usage below,
and added a comment to the enum definition.

Only a safe choice here is to use switch-case and rely on the static
check on exhaustiveness, but to me it sounds like overkill.

   std::string arc_sdk_version;
   const bool is_M = base::SysInfo::GetLsbReleaseValue(kLsbReleaseArcVersionKey,
                                                       &arc_sdk_version) &&