ChromeOS: Disable ARC when incompatible filesystem is detected.

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 3c40a7c2f950b82644f66845c0aada8f88eee9d2..c8fbdfab49bf97f931c8f9b9a9c7c814db110c05 100644
--- a/chrome/browser/chromeos/arc/arc_util.cc
+++ b/chrome/browser/chromeos/arc/arc_util.cc
@@ -4,7 +4,15 @@
 
 #include "chrome/browser/chromeos/arc/arc_util.h"
 
+#include <linux/magic.h>
+#include <sys/statfs.h>
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"

[xiyuan, 2017/04/04 15:32:21]

nit: not used?

[kinaba, 2017/04/05 02:35:27]

Done.

 #include "base/logging.h"
+#include "base/strings/string_split.h"

[xiyuan, 2017/04/04 15:32:21]

nit: not used?

[kinaba, 2017/04/05 02:35:27]

Done.

+#include "base/sys_info.h"
+#include "base/threading/thread_restrictions.h"
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
 #include "chrome/browser/chromeos/login/user_flow.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
@@ -92,6 +100,19 @@ bool IsArcAllowedForProfile(const Profile* profile) {
     return false;
   }
 
+  // Do not allow newer version of ARC on old filesystem
+  if (base::SysInfo::IsRunningOnChromeOS()) {

[hidehiko, 2017/04/04 18:35:12]

Could you add a bit more comment about IsRunningOnChromeOS?
Because this is put under chromeos/ directory, so we expect this code compiles
for ChromeOS.
IIUC, the guard is needed for {unit,browser}tests, right?

[kinaba, 2017/04/05 02:35:28]

Done.

+    std::string arc_sdk_version;
+    if (!profile->GetPrefs()->GetBoolean(

[hidehiko, 2017/04/04 18:35:12]

Could you comment the strategy here? We're making sure;
- Underlying file system is compatible with ARC. Or
- SDK version is 23 (meaning M).

Optional: Also, it may be good to briefly mention about the function who and/or
when kArcCompatibileFilesystemChosen value is set?

[kinaba, 2017/04/05 02:35:26]

Done.

+            prefs::kArcCompatibleFilesystemChosen) &&
+        (!base::SysInfo::GetLsbReleaseValue("CHROMEOS_ARC_ANDROID_SDK_VERSION",
+                                            &arc_sdk_version) ||

[Junichi Uekawa, 2017/04/04 23:48:12]

If CHROMEOS_ARC_ANDROID_SDK_VERSION is not there, we assume Android isn't on
this board?
So this path is slightly different.

[kinaba, 2017/04/05 02:35:27]

If Android isn't on the board at all, a check above (IsArcAvailable()) should
have exited this function already.
We don't need to care what to do here in that case. If Android is there but we
failed to read lsb-release,
something bad is happening and thus I'd like to take safer (disable ARC) branch.

+         arc_sdk_version != "23")) {

[Junichi Uekawa, 2017/04/04 23:48:13]

can you name this constant, like: 
constexpr std::string kAndroidMSdkVersion("23");

[kinaba, 2017/04/05 02:35:27]

Done.

+      VLOG(1) << "Users postponed to migrate to dircrypto are not supported.";

[hidehiko, 2017/04/04 18:35:12]

nit/optional: how about output arc_sdk_version here?

[kinaba, 2017/04/05 02:35:28]

Done.

+      return false;
+    }
+  }
+
   return true;
 }
 
@@ -141,4 +162,14 @@ bool AreArcAllOptInPreferencesManagedForProfile(const Profile* profile) {
              prefs::kArcLocationServiceEnabled);
 }
 
+bool IsArcCompatibleFilesystem(const base::FilePath& path) {
+  base::ThreadRestrictions::AssertIOAllowed();
+
+  // If it can be verified it is not on ecryptfs, then it is ok.
+  struct statfs statfs_buf;
+  if (statfs(path.value().c_str(), &statfs_buf) < 0)
+    return false;
+  return statfs_buf.f_type != ECRYPTFS_SUPER_MAGIC;
+}
+
 }  // namespace arc