Move GetSystemPropertyInt, UpstartOperation and ConfigureUpstartJobs to arc util.

Bug: b:172063497
Test: components_unittests --gtest_filter="ArcVmClientAdapterTest.*"
Change-Id: I53a6174cdfa64c9607bcbf64da90d8855e735fc3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2581018Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Sherry Hu <shuanghu@google.com>
Cr-Commit-Position: refs/heads/master@{#835064}

components/arc/BUILD.gn

@@ -203,6 +203,7 @@ static_library("arc_base_utils") {
     "//chromeos/constants",
     "//chromeos/dbus:dbus",
     "//chromeos/dbus/session_manager",
+    "//chromeos/dbus/upstart:upstart",
     "//components/exo",
     "//ui/aura",
   ]

components/arc/arc_util.cc

@@ -15,12 +15,14 @@
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/optional.h"
+#include "base/process/launch.h"
 #include "base/strings/string_number_conversions.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/dbus/concierge_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/debug_daemon/debug_daemon_client.h"
 #include "chromeos/dbus/session_manager/session_manager_client.h"
+#include "chromeos/dbus/upstart/upstart_client.h"
 #include "components/arc/arc_features.h"
 #include "components/exo/shell_surface_util.h"
 #include "components/user_manager/user_manager.h"
@@ -45,6 +47,8 @@ constexpr char kAvailabilityOfficiallySupported[] = "officially-supported";
 constexpr char kAlwaysStartWithNoPlayStore[] =
     "always-start-with-no-play-store";
 
+constexpr const char kCrosSystemPath[] = "/usr/bin/crossystem";
+
 void SetArcCpuRestrictionCallback(
     login_manager::ContainerCpuRestrictionState state,
     bool success) {
@@ -110,6 +114,36 @@ void SetArcContainerCpuRestriction(CpuRestrictionState cpu_restriction_state) {
       state, base::BindOnce(SetArcCpuRestrictionCallback, state));
 }
 
+// Decodes a job name that may have "_2d" e.g. |kArcCreateDataJobName|
+// and returns a decoded string.
+std::string DecodeJobName(const std::string& raw_job_name) {
+  constexpr const char* kFind = "_2d";
+  std::string decoded(raw_job_name);
+  base::ReplaceSubstringsAfterOffset(&decoded, 0, kFind, "-");
+  return decoded;
+}
+
+// Called when the Upstart operation started in ConfigureUpstartJobs is
+// done. Handles the fatal error (if any) and then starts the next job.
+void OnConfigureUpstartJobs(std::deque<JobDesc> jobs,
+                            chromeos::VoidDBusMethodCallback callback,
+                            bool result) {
+  const std::string job_name = DecodeJobName(jobs.front().job_name);
+  const bool is_start = (jobs.front().operation == UpstartOperation::JOB_START);
+
+  if (!result && is_start) {
+    LOG(ERROR) << "Failed to start " << job_name;
+    // TODO(yusukes): Record UMA for this case.
+    std::move(callback).Run(false);
+    return;
+  }
+
+  VLOG(1) << job_name
+          << (is_start ? " started" : (result ? " stopped " : " not running?"));
+  jobs.pop_front();
+  ConfigureUpstartJobs(std::move(jobs), std::move(callback));
+}
+
 }  // namespace
 
 bool IsArcAvailable() {
@@ -347,4 +381,60 @@ bool GenerateFirstStageFstab(const base::FilePath& combined_property_file_name,
                          combined_property_file_name.value().c_str()));
 }
 
+int GetSystemPropertyInt(const std::string& property) {
+  std::string output;
+  if (!base::GetAppOutput({kCrosSystemPath, property}, &output))
+    return -1;
+  int output_int;
+  return base::StringToInt(output, &output_int) ? output_int : -1;
+}
+
+JobDesc::JobDesc(const std::string& job_name,
+                 UpstartOperation operation,
+                 const std::vector<std::string>& environment)
+    : job_name(job_name), operation(operation), environment(environment) {}
+
+JobDesc::~JobDesc() = default;
+
+JobDesc::JobDesc(const JobDesc& other) = default;
+
+void ConfigureUpstartJobs(std::deque<JobDesc> jobs,
+                          chromeos::VoidDBusMethodCallback callback) {
+  if (jobs.empty()) {
+    std::move(callback).Run(true);
+    return;
+  }
+
+  if (jobs.front().operation == UpstartOperation::JOB_STOP_AND_START) {
+    // Expand the restart operation into two, stop and start.
+    jobs.front().operation = UpstartOperation::JOB_START;
+    jobs.push_front({jobs.front().job_name, UpstartOperation::JOB_STOP,
+                     jobs.front().environment});
+  }
+
+  const auto& job_name = jobs.front().job_name;
+  const auto& operation = jobs.front().operation;
+  const auto& environment = jobs.front().environment;
+
+  VLOG(1) << (operation == UpstartOperation::JOB_START ? "Starting "
+                                                       : "Stopping ")
+          << DecodeJobName(job_name);
+
+  auto wrapped_callback = base::BindOnce(&OnConfigureUpstartJobs,
+                                         std::move(jobs), std::move(callback));
+  switch (operation) {
+    case UpstartOperation::JOB_START:
+      chromeos::UpstartClient::Get()->StartJob(job_name, environment,
+                                               std::move(wrapped_callback));
+      break;
+    case UpstartOperation::JOB_STOP:
+      chromeos::UpstartClient::Get()->StopJob(job_name, environment,
+                                              std::move(wrapped_callback));
+      break;
+    case UpstartOperation::JOB_STOP_AND_START:
+      NOTREACHED();
+      break;
+  }
+}
+
 }  // namespace arc

components/arc/arc_util.h

@@ -10,8 +10,11 @@
 // users' preferences, and FeatureList.
 
 #include <stdint.h>
+#include <deque>
 #include <string>
 
+#include "chromeos/dbus/dbus_method_call_status.h"
+
 namespace aura {
 class Window;
 }  // namespace aura
@@ -36,6 +39,30 @@ enum class CpuRestrictionState {
   CPU_RESTRICTION_BACKGROUND = 1,
 };
 
+enum class UpstartOperation {
+  JOB_START = 0,
+  JOB_STOP,
+  // This sends STOP D-Bus message, then sends START. Unlike 'initctl restart',
+  // this starts the job even when the job hasn't been started yet (and
+  // therefore the stop operation fails.)
+  JOB_STOP_AND_START,
+};
+
+// Upstart Job Description
+struct JobDesc {
+  // Explicit ctor/dtor declaration is necessary for complex struct. See
+  // https://cs.chromium.org/chromium/src/tools/clang/plugins/FindBadConstructsConsumer.cpp
+  JobDesc(const std::string& job_name,
+          UpstartOperation operation,
+          const std::vector<std::string>& environment);
+  ~JobDesc();
+  JobDesc(const JobDesc& other);
+
+  std::string job_name;
+  UpstartOperation operation;
+  std::vector<std::string> environment;
+};
+
 // Name of the crosvm instance when ARCVM is enabled.
 constexpr char kArcVmName[] = "arcvm";
 
@@ -160,6 +187,17 @@ int32_t GetLcdDensityForDeviceScaleFactor(float device_scale_factor);
 bool GenerateFirstStageFstab(const base::FilePath& combined_property_file_name,
                              const base::FilePath& fstab_path);
 
+// Gets a system property managed by crossystem. This function can be called
+// only with base::MayBlock().
+int GetSystemPropertyInt(const std::string& property);
+
+// Starts or stops a job in |jobs| one by one. If starting a job fails, the
+// whole operation is aborted and the |callback| is immediately called with
+// false. Errors on stopping a job is just ignored with some logs. Once all jobs
+// are successfully processed, |callback| is called with true.
+void ConfigureUpstartJobs(std::deque<JobDesc> jobs,
+                          chromeos::VoidDBusMethodCallback callback);
+
 }  // namespace arc
 
 #endif  // COMPONENTS_ARC_ARC_UTIL_H_

components/arc/session/arc_vm_client_adapter.cc

@@ -48,7 +48,6 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/debug_daemon/debug_daemon_client.h"
 #include "chromeos/dbus/session_manager/session_manager_client.h"
-#include "chromeos/dbus/upstart/upstart_client.h"
 #include "chromeos/system/statistics_provider.h"
 #include "components/arc/arc_features.h"
 #include "components/arc/arc_util.h"
@@ -71,7 +70,6 @@ constexpr char kArcVmPostLoginServicesJobName[] =
 constexpr char kArcVmPostVmStartServicesJobName[] =
     "arcvm_2dpost_2dvm_2dstart_2dservices";
 
-constexpr const char kCrosSystemPath[] = "/usr/bin/crossystem";
 constexpr const char kArcVmBootNotificationServerSocketPath[] =
     "/run/arcvm_boot_notification_server/host.socket";
 
@@ -290,16 +288,6 @@ vm_tools::concierge::StartArcVmRequest CreateStartArcVmRequest(
   return request;
 }
 
-// Gets a system property managed by crossystem. This function can be called
-// only with base::MayBlock().
-int GetSystemPropertyInt(const std::string& property) {
-  std::string output;
-  if (!base::GetAppOutput({kCrosSystemPath, property}, &output))
-    return -1;
-  int output_int;
-  return base::StringToInt(output, &output_int) ? output_int : -1;
-}
-
 const sockaddr_un* GetArcVmBootNotificationServerAddress() {
   static struct sockaddr_un address {
     .sun_family = AF_UNIX,
@@ -379,98 +367,6 @@ bool SendUpgradePropsToArcVmBootNotificationServer(
   return true;
 }
 
-// Decodes a job name that may have "_2d" e.g. |kArcCreateDataJobName|
-// and returns a decoded string.
-std::string DecodeJobName(const std::string& raw_job_name) {
-  constexpr const char* kFind = "_2d";
-  std::string decoded(raw_job_name);
-  base::ReplaceSubstringsAfterOffset(&decoded, 0, kFind, "-");
-  return decoded;
-}
-
-enum class UpstartOperation {
-  JOB_START = 0,
-  JOB_STOP,
-  // This sends STOP D-Bus message, then sends START. Unlike 'initctl restart',
-  // this starts the job even when the job hasn't been started yet (and
-  // therefore the stop operation fails.)
-  JOB_STOP_AND_START,
-};
-
-struct JobDesc {
-  std::string job_name;
-  UpstartOperation operation;
-  std::vector<std::string> environment;
-};
-
-void OnConfigureUpstartJobs(std::deque<JobDesc> jobs,
-                            chromeos::VoidDBusMethodCallback callback,
-                            bool result);
-
-// Starts or stops a job in |jobs| one by one. If starting a job fails, the
-// whole operation is aborted and the |callback| is immediately called with
-// false. Errors on stopping a job is just ignored with some logs. Once all jobs
-// are successfully processed, |callback| is called with true.
-void ConfigureUpstartJobs(std::deque<JobDesc> jobs,
-                          chromeos::VoidDBusMethodCallback callback) {
-  if (jobs.empty()) {
-    std::move(callback).Run(true);
-    return;
-  }
-
-  if (jobs.front().operation == UpstartOperation::JOB_STOP_AND_START) {
-    // Expand the restart operation into two, stop and start.
-    jobs.front().operation = UpstartOperation::JOB_START;
-    jobs.push_front({jobs.front().job_name, UpstartOperation::JOB_STOP,
-                     jobs.front().environment});
-  }
-
-  const auto& job_name = jobs.front().job_name;
-  const auto& operation = jobs.front().operation;
-  const auto& environment = jobs.front().environment;
-
-  VLOG(1) << (operation == UpstartOperation::JOB_START ? "Starting "
-                                                       : "Stopping ")
-          << DecodeJobName(job_name);
-
-  auto wrapped_callback = base::BindOnce(&OnConfigureUpstartJobs,
-                                         std::move(jobs), std::move(callback));
-  switch (operation) {
-    case UpstartOperation::JOB_START:
-      chromeos::UpstartClient::Get()->StartJob(job_name, environment,
-                                               std::move(wrapped_callback));
-      break;
-    case UpstartOperation::JOB_STOP:
-      chromeos::UpstartClient::Get()->StopJob(job_name, environment,
-                                              std::move(wrapped_callback));
-      break;
-    case UpstartOperation::JOB_STOP_AND_START:
-      NOTREACHED();
-      break;
-  }
-}
-
-// Called when the Upstart operation started in ConfigureUpstartJobs is
-// done. Handles the fatal error (if any) and then starts the next job.
-void OnConfigureUpstartJobs(std::deque<JobDesc> jobs,
-                            chromeos::VoidDBusMethodCallback callback,
-                            bool result) {
-  const std::string job_name = DecodeJobName(jobs.front().job_name);
-  const bool is_start = (jobs.front().operation == UpstartOperation::JOB_START);
-
-  if (!result && is_start) {
-    LOG(ERROR) << "Failed to start " << job_name;
-    // TODO(yusukes): Record UMA for this case.
-    std::move(callback).Run(false);
-    return;
-  }
-
-  VLOG(1) << job_name
-          << (is_start ? " started" : (result ? " stopped " : " not running?"));
-  jobs.pop_front();
-  ConfigureUpstartJobs(std::move(jobs), std::move(callback));
-}
-
 // Returns true if the daemon for adb-over-usb should be started on the device.
 bool ShouldStartAdbd(bool is_dev_mode,
                      bool is_host_on_vm,