Policy implementation for encryptfs to ext4 migration strategy

components/arc/arc_util.cc

Index: components/arc/arc_util.cc
diff --git a/components/arc/arc_util.cc b/components/arc/arc_util.cc
index 6a82a7046b5eb813c2bb2f7b40a01205326194f0..4e0e74b73b8283e0f4a6134cd91f4027e62a42cb 100644
--- a/components/arc/arc_util.cc
+++ b/components/arc/arc_util.cc
@@ -31,6 +31,8 @@ constexpr char kAvailabilityOfficiallySupported[] = "officially-supported";
 
 }  // namespace
 
+ARC_AVAILABILITY_POLICY_STATUS arc_availability_policy_status = UNKNOWN;

[hidehiko, 2017/05/25 12:41:55]

Please move this into anonymous namespace.
sytle: g_ prefix please.

[igorcov, 2017/05/31 17:25:35]

Done.

+
 bool IsArcAvailable() {
   const auto* command_line = base::CommandLine::ForCurrentProcess();

[hidehiko, 2017/05/25 12:41:56]

Please DCHECK arc_availability_policy_status is not UNKNOWN.

Also, could you at least manually check the code by switching the check to CHECK
temporarily for both enterprise policy applied case and regular case?

[igorcov, 2017/05/31 17:25:35]

Can't DCHECK that, because this function is called a few times before the policy
is loaded. In that case, the status remains as UNKNOWN and arc is disabled for
the time being.

I've done manual tests for both, enrolled device and consumer owned device,
simulating different policy values with YAPS.

[hidehiko, 2017/06/01 16:07:15]

I think this is must-have thing. Otherwise IsArcAvailable() could change the
value in a process. If impossible, maybe it is not yet the time to code review,
as designdoc is not yet fixed AFAIK?

 
@@ -38,8 +40,21 @@ bool IsArcAvailable() {
     std::string value = command_line->GetSwitchValueASCII(
         chromeos::switches::kArcAvailability);
     DCHECK(value == kAvailabilityNone || value == kAvailabilityInstalled ||
-           value == kAvailabilityOfficiallySupported)
+           value == kAvailabilityOfficiallySupported ||
+           value == kAvailabilityOfficiallySupportedWithMigration)
         << "Unknown flag value: " << value;
+
+    // In the case of supported with migration, the ARC availability is defined

[hidehiko, 2017/05/25 12:41:55]

Please add detailed unit tests for new behavior.

[igorcov, 2017/05/31 17:25:35]

Done.

+    // by the policy in case the device is managed, or is available if the
+    // device is consumer owned.
+    // TODO(igorcov): Remove this after migration. crbug.com/725493
+    if (value == kAvailabilityOfficiallySupportedWithMigration) {
+      if (arc_availability_policy_status == AVAILABLE)

[hidehiko, 2017/05/25 12:41:55]

nit: return arc_availability_policy_status == AVAILABLE;

[igorcov, 2017/05/31 17:25:35]

Done.

+        return true;
+      else
+        return false;
+    }
+
     return value == kAvailabilityOfficiallySupported ||
            (value == kAvailabilityInstalled &&
             base::FeatureList::IsEnabled(kEnableArcFeature));
@@ -53,6 +68,14 @@ bool IsArcAvailable() {
        base::FeatureList::IsEnabled(kEnableArcFeature));
 }
 
+void SetArcAvailabilityPolicyStatus(ARC_AVAILABILITY_POLICY_STATUS status) {
+  arc_availability_policy_status = status;

[hidehiko, 2017/05/25 12:41:55]

To ensure this is called exact once,
DCHECK_EQ(UNKNOWN, arc_availability_policy_status);
please.

+}
+
+ARC_AVAILABILITY_POLICY_STATUS GetArcAvailabilityPolicyStatus() {
+  return arc_availability_policy_status;
+}
+
 bool ShouldArcAlwaysStart() {
   return base::CommandLine::ForCurrentProcess()->HasSwitch(
       chromeos::switches::kArcAlwaysStart);