Policy implementation for encryptfs to ext4 migration strategy

components/arc/arc_util.cc

 // Copyright 2017 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 "components/arc/arc_util.h"
 
 #include <string>
 
 #include "ash/shared/app_types.h"
 #include "base/command_line.h"
@@ skipping 13 matching lines @@
 const base::Feature kEnableArcFeature{"EnableARC",
                                       base::FEATURE_DISABLED_BY_DEFAULT};
 
 // Possible values for --arc-availability flag.
 constexpr char kAvailabilityNone[] = "none";
 constexpr char kAvailabilityInstalled[] = "installed";
 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?

 
   if (command_line->HasSwitch(chromeos::switches::kArcAvailability)) {
     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));
   }
 
   // For transition, fallback to old flags.
   // TODO(hidehiko): Remove this and clean up whole this function, when
   // session_manager supports a new flag.
   return command_line->HasSwitch(chromeos::switches::kEnableArc) ||
       (command_line->HasSwitch(chromeos::switches::kArcAvailable) &&
        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);
 }
 
 void SetArcAlwaysStartForTesting() {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       chromeos::switches::kArcAlwaysStart);
 }
 
@@ skipping 33 matching lines @@
 }
 
 bool IsArcAppWindow(aura::Window* window) {
   if (!window)
     return false;
   return window->GetProperty(aura::client::kAppType) ==
          static_cast<int>(ash::AppType::ARC_APP);
 }
 
 }  // namespace arc