Revert "arcvm: Start arcvm-adbd on starting ARCVM"

This reverts commit 1af1bc07.

Reason for revert: Blocking the Chrome on Chromeos uprev.

https://bugs.chromium.org/p/chromium/issues/detail?id=1123901#c6

Original change's description:
> arcvm: Start arcvm-adbd on starting ARCVM
> 
> This CL depends on the following CLs:
> https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2380431
> https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/2378806
> https://chrome-internal-review.googlesource.com/c/chromeos/overlays/project-cheets-private/+/3238221
> 
> BUG=b:166314117
> TEST=arc.Boot.vm still passes, arcvm-adbd runs
> 
> Change-Id: I385a807d84a98b4a97dcc66daf6bef5881eef2bd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2380733
> Commit-Queue: Yusuke Sato <yusukes@chromium.org>
> Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#803035}

TBR=yusukes@chromium.org,hidehiko@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: b:166314117
Change-Id: I30644f24fbe139330a54e88aefc1b73c0340340e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392625Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804337}

components/arc/session/arc_vm_client_adapter.cc

@@ -65,7 +65,6 @@ constexpr const char kArcVmMountMyFilesJobName[] = "arcvm_2dmount_2dmyfiles";
 constexpr const char kArcVmMountRemovableMediaJobName[] =
     "arcvm_2dmount_2dremovable_2dmedia";
 constexpr const char kArcVmServerProxyJobName[] = "arcvm_2dserver_2dproxy";
-constexpr const char kArcVmAdbdJobName[] = "arcvm_2dadbd";
 constexpr const char kArcVmPerBoardFeaturesJobName[] =
     "arcvm_2dper_2dboard_2dfeatures";
 constexpr const char kArcVmBootNotificationServerJobName[] =
@@ -84,7 +83,6 @@ constexpr base::TimeDelta kConnectSleepDurationInitial =
 
 base::Optional<base::TimeDelta> g_connect_timeout_limit_for_testing;
 base::Optional<base::TimeDelta> g_connect_sleep_duration_initial_for_testing;
-bool g_enable_adb_over_usb_for_testing = false;
 
 chromeos::ConciergeClient* GetConciergeClient() {
   return chromeos::DBusThreadManager::Get()->GetConciergeClient();
@@ -449,16 +447,6 @@ void OnConfigureUpstartJobs(std::deque<JobDesc> jobs,
   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,
-                     bool has_adbd_json,
-                     bool is_adb_over_usb_disabled) {
-  // Do the same check as ArcSetup::MaybeStartAdbdProxy().
-  return is_dev_mode && !is_host_on_vm && has_adbd_json &&
-         !is_adb_over_usb_disabled;
-}
-
 }  // namespace
 
 class ArcVmClientAdapter : public ArcClientAdapter,
@@ -602,20 +590,6 @@ class ArcVmClientAdapter : public ArcClientAdapter,
   void OnIsDevMode(chromeos::VoidDBusMethodCallback callback,
                    bool is_dev_mode) {
     is_dev_mode_ = is_dev_mode;
-    base::ThreadPool::PostTaskAndReplyWithResult(
-        FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
-        base::BindOnce([]() {
-          std::string output;
-          return base::GetAppOutput({"crossystem", "dev_enable_udc?0"},
-                                    &output);
-        }),
-        base::BindOnce(&ArcVmClientAdapter::OnIsAdbOverUsbDisabled,
-                       weak_factory_.GetWeakPtr(), std::move(callback)));
-  }
-
-  void OnIsAdbOverUsbDisabled(chromeos::VoidDBusMethodCallback callback,
-                              bool is_adb_over_usb_disabled) {
-    is_adb_over_usb_disabled_ = is_adb_over_usb_disabled;
     std::deque<JobDesc> jobs{
         // Note: the first Upstart job is a task, and the callback for the start
         // request won't be called until the task finishes. When the callback is
@@ -678,6 +652,34 @@ class ArcVmClientAdapter : public ArcClientAdapter,
       std::move(callback).Run(false);
       return;
     }
+    std::vector<std::string> environment = {
+        "CHROMEOS_USER=" +
+        cryptohome::CreateAccountIdentifierFromIdentification(cryptohome_id_)
+            .account_id()};
+    std::deque<JobDesc> jobs{
+        JobDesc{kArcVmServerProxyJobName, UpstartOperation::JOB_START, {}},
+        JobDesc{kArcCreateDataJobName, UpstartOperation::JOB_START,
+                std::move(environment)},
+        JobDesc{kArcVmMountMyFilesJobName, UpstartOperation::JOB_START, {}},
+        JobDesc{
+            kArcVmMountRemovableMediaJobName, UpstartOperation::JOB_START, {}},
+    };
+    ConfigureUpstartJobs(
+        std::move(jobs),
+        base::BindOnce(&ArcVmClientAdapter::OnConfigureUpstartJobsOnUpgradeArc,
+                       weak_factory_.GetWeakPtr(), std::move(params),
+                       std::move(callback)));
+  }
+
+  void OnConfigureUpstartJobsOnUpgradeArc(
+      UpgradeParams params,
+      chromeos::VoidDBusMethodCallback callback,
+      bool result) {
+    if (!result) {
+      LOG(ERROR) << "ConfigureUpstartJobs (on upgrading ARCVM) failed";
+      std::move(callback).Run(false);
+      return;
+    }
     VLOG(2) << "Checking file system status";
     base::ThreadPool::PostTaskAndReplyWithResult(
         FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
@@ -705,47 +707,6 @@ class ArcVmClientAdapter : public ArcClientAdapter,
       return;
     }
 
-    std::vector<std::string> environment_for_create_data = {
-        "CHROMEOS_USER=" +
-        cryptohome::CreateAccountIdentifierFromIdentification(cryptohome_id_)
-            .account_id()};
-    std::deque<JobDesc> jobs{
-        JobDesc{kArcVmServerProxyJobName, UpstartOperation::JOB_START, {}},
-        JobDesc{kArcCreateDataJobName, UpstartOperation::JOB_START,
-                std::move(environment_for_create_data)},
-        JobDesc{kArcVmMountMyFilesJobName, UpstartOperation::JOB_START, {}},
-        JobDesc{
-            kArcVmMountRemovableMediaJobName, UpstartOperation::JOB_START, {}},
-    };
-    if (ShouldStartAdbd(*is_dev_mode_, is_host_on_vm_,
-                        file_system_status.has_adbd_json(),
-                        *is_adb_over_usb_disabled_) ||
-        g_enable_adb_over_usb_for_testing) {
-      // Start the daemon for supporting adb-over-usb.
-      std::vector<std::string> environment_for_adbd = {"SERIALNUMBER=" +
-                                                       serial_number_};
-      jobs.emplace_back(JobDesc{kArcVmAdbdJobName,
-                                UpstartOperation::JOB_STOP_AND_START,
-                                std::move(environment_for_adbd)});
-    }
-    ConfigureUpstartJobs(
-        std::move(jobs),
-        base::BindOnce(&ArcVmClientAdapter::OnConfigureUpstartJobsOnUpgradeArc,
-                       weak_factory_.GetWeakPtr(), std::move(params),
-                       std::move(file_system_status), std::move(callback)));
-  }
-
-  void OnConfigureUpstartJobsOnUpgradeArc(
-      UpgradeParams params,
-      FileSystemStatus file_system_status,
-      chromeos::VoidDBusMethodCallback callback,
-      bool result) {
-    if (!result) {
-      LOG(ERROR) << "ConfigureUpstartJobs (on upgrading ARCVM) failed";
-      std::move(callback).Run(false);
-      return;
-    }
-
     const int32_t cpus =
         base::SysInfo::NumberOfProcessors() - start_params_.num_cores_disabled;
     DCHECK_LT(0, cpus);
@@ -826,8 +787,6 @@ class ArcVmClientAdapter : public ArcClientAdapter,
   base::Optional<bool> is_dev_mode_;
   // True when the *host* is running on a VM.
   const bool is_host_on_vm_;
-  // True when adb-over-usb is disabled.
-  base::Optional<bool> is_adb_over_usb_disabled_;
 
   // A cryptohome ID of the primary profile.
   cryptohome::Identification cryptohome_id_;
@@ -875,10 +834,6 @@ void SetArcVmBootNotificationServerAddressForTesting(
   g_connect_sleep_duration_initial_for_testing = connect_sleep_duration_initial;
 }
 
-void EnableAdbOverUsbForTesting() {
-  g_enable_adb_over_usb_for_testing = true;
-}
-
 std::vector<std::string> GenerateUpgradeProps(
     const UpgradeParams& upgrade_params,
     const std::string& serial_number,

components/arc/session/arc_vm_client_adapter.h

@@ -38,9 +38,6 @@ void SetArcVmBootNotificationServerAddressForTesting(
     base::TimeDelta connect_timeout_limit,
     base::TimeDelta connect_sleep_duration_initial);
 
-// Enable adb-over-usb and let the adapter start the support daemon for testing.
-void EnableAdbOverUsbForTesting();
-
 // Generates a list of props from |upgrade_params|, each of which takes the form
 // "prefix.prop_name=value"
 std::vector<std::string> GenerateUpgradeProps(

components/arc/session/arc_vm_client_adapter_unittest.cc

@@ -49,7 +49,6 @@ constexpr const char kArcVmMountMyFilesJobName[] = "arcvm_2dmount_2dmyfiles";
 constexpr const char kArcVmMountRemovableMediaJobName[] =
     "arcvm_2dmount_2dremovable_2dmedia";
 constexpr const char kArcVmServerProxyJobName[] = "arcvm_2dserver_2dproxy";
-constexpr const char kArcVmAdbdJobName[] = "arcvm_2dadbd";
 constexpr const char kArcVmPerBoardFeaturesJobName[] =
     "arcvm_2dper_2dboard_2dfeatures";
 constexpr const char kArcVmBootNotificationServerJobName[] =
@@ -282,10 +281,10 @@ class ArcVmClientAdapterTest : public testing::Test,
     SetArcVmBootNotificationServerAddressForTesting(
         std::string(kArcVmBootNotificationServerAddress,
                     sizeof(kArcVmBootNotificationServerAddress)),
-        // connect_timeout_limit
-        base::TimeDelta::FromMilliseconds(100),
-        // connect_sleep_duration_initial
-        base::TimeDelta::FromMilliseconds(20));
+        base::TimeDelta::FromMilliseconds(100) /* connect_timeout_limit */,
+        base::TimeDelta::FromMilliseconds(
+            20) /* connect_sleep_duration_initial */
+    );
   }
 
   void TearDown() override {
@@ -475,6 +474,7 @@ class ArcVmClientAdapterTest : public testing::Test,
   }
 
  private:
+
   void RewriteStatus(FileSystemStatus* status) {
     status->set_host_rootfs_writable_for_testing(host_rootfs_writable_);
     status->set_system_image_ext_format_for_testing(system_image_ext_format_);
@@ -747,47 +747,6 @@ TEST_F(ArcVmClientAdapterTest, UpgradeArc_StartArcVmProxyFailure) {
   EXPECT_TRUE(arc_instance_stopped_called());
 }
 
-// Tests that UpgradeArc() handles arcvm-adbd stop failures properly.
-TEST_F(ArcVmClientAdapterTest, UpgradeArc_StopArcVmAdbdFailure) {
-  SetValidUserInfo();
-  StartMiniArc();
-
-  // Inject failure to FakeUpstartClient.
-  InjectUpstartStopJobFailure(kArcVmAdbdJobName);
-
-  // Upgrade should still succeed.
-  UpgradeArc(true);
-  EXPECT_TRUE(GetStartConciergeCalled());
-  EXPECT_TRUE(GetTestConciergeClient()->start_arc_vm_called());
-  EXPECT_FALSE(arc_instance_stopped_called());
-}
-
-// Tests that UpgradeArc() handles arcvm-adbd startup failures properly.
-TEST_F(ArcVmClientAdapterTest, UpgradeArc_StartArcVmAdbdFailure) {
-  SetValidUserInfo();
-  StartMiniArc();
-
-  // Inject failure to FakeUpstartClient.
-  InjectUpstartStartJobFailure(kArcVmAdbdJobName);
-
-  EnableAdbOverUsbForTesting();
-  UpgradeArc(false);
-  EXPECT_TRUE(GetStartConciergeCalled());
-  EXPECT_FALSE(GetTestConciergeClient()->start_arc_vm_called());
-  EXPECT_FALSE(arc_instance_stopped_called());
-
-  // Try to stop the VM. StopVm will fail in this case because
-  // no VM is running.
-  vm_tools::concierge::StopVmResponse response;
-  response.set_success(false);
-  GetTestConciergeClient()->set_stop_vm_response(response);
-  adapter()->StopArcInstance(/*on_shutdown=*/false,
-                             /*should_backup_log=*/false);
-  run_loop()->Run();
-  EXPECT_TRUE(GetTestConciergeClient()->stop_vm_called());
-  EXPECT_TRUE(arc_instance_stopped_called());
-}
-
 // Tests that UpgradeArc() handles arc-create-data startup failures properly.
 TEST_F(ArcVmClientAdapterTest, UpgradeArc_StartArcCreateDataFailure) {
   SetValidUserInfo();

components/arc/session/file_system_status.cc

@@ -20,7 +20,6 @@
 namespace arc {
 namespace {
 
-constexpr const char kAdbdJson[] = "/etc/arc/adbd.json";
 constexpr const char kArcVmConfigJsonPath[] = "/usr/share/arcvm/config.json";
 constexpr const char kBuiltinPath[] = "/opt/google/vms/android";
 constexpr const char kFstabPath[] = "/run/arcvm/host_generated/fstab";
@@ -43,8 +42,7 @@ FileSystemStatus::FileSystemStatus()
       vendor_image_path_(base::FilePath(kBuiltinPath).Append(kVendorImage)),
       guest_kernel_path_(base::FilePath(kBuiltinPath).Append(kKernel)),
       fstab_path_(kFstabPath),
-      is_system_image_ext_format_(IsSystemImageExtFormat(system_image_path_)),
-      has_adbd_json_(base::PathExists(base::FilePath(kAdbdJson))) {}
+      is_system_image_ext_format_(IsSystemImageExtFormat(system_image_path_)) {}
 
 // static
 bool FileSystemStatus::IsAndroidDebuggable(const base::FilePath& json_path) {

components/arc/session/file_system_status.h

@@ -30,7 +30,6 @@ class FileSystemStatus {
   bool is_system_image_ext_format() const {
     return is_system_image_ext_format_;
   }
-  bool has_adbd_json() const { return has_adbd_json_; }
   const base::FilePath& system_image_path() const { return system_image_path_; }
   const base::FilePath& vendor_image_path() const { return vendor_image_path_; }
   const base::FilePath& guest_kernel_path() const { return guest_kernel_path_; }
@@ -93,7 +92,6 @@ class FileSystemStatus {
   base::FilePath guest_kernel_path_;
   base::FilePath fstab_path_;
   bool is_system_image_ext_format_;
-  bool has_adbd_json_;
 };
 
 }  // namespace arc