arcvm: Stop arcvm-server-proxy in StartMiniInstance()

This is to make sure that starting the proxy in UpgradeInstance()
will always succeed even after Chrome crashes and restarting.

Once b/142140355 is fixed, the proxy will detect Chrome crashes by
itself and exit, but in theory, the proxy may exit after restarted
Chrome calls UpgradeInstance(). This CL is to prevent the race.

BUG=b:143175381
TEST=kill Chrome, verify restarted Chrome can start ARCVM

Change-Id: Id408c6525f5b7a53d095ce36777a4300ec313cb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1874788Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708656}

components/arc/session/arc_vm_client_adapter.cc

@@ -30,10 +30,10 @@
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "base/threading/scoped_blocking_call.h"
-#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chromeos/dbus/concierge_client.h"
+#include "chromeos/dbus/dbus_method_call_status.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/debug_daemon/debug_daemon_client.h"
 #include "chromeos/dbus/upstart/upstart_client.h"
@@ -418,10 +418,24 @@ class ArcVmClientAdapter : public ArcClientAdapter,
  private:
   void OnIsDevMode(chromeos::VoidDBusMethodCallback callback,
                    bool is_dev_mode) {
-    base::SequencedTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(std::move(callback), true));
+    // Make sure to kill a stale arcvm-server-proxy job (if any).
+    chromeos::UpstartClient::Get()->StopJob(
+        kArcVmServerProxyJobName, /*environment=*/{},
+        base::BindOnce(&ArcVmClientAdapter::OnArcVmServerProxyJobStopped,
+                       weak_factory_.GetWeakPtr(), std::move(callback)));
     is_dev_mode_ = is_dev_mode;
+  }
+
+  void OnArcVmServerProxyJobStopped(chromeos::VoidDBusMethodCallback callback,
+                                    bool result) {
+    VLOG(1) << "OnArcVmServerProxyJobStopped: job "
+            << (result ? "stopped" : "not running?");
+
     should_notify_observers_ = true;
+    // Always run the |callback| with true ignoring the |result|. |result| can
+    // be false when the proxy job has already been stopped for other reasons,
+    // but it's not considered as an error.
+    std::move(callback).Run(true);
   }
 
   void OnConciergeStarted(UpgradeParams params,
@@ -443,7 +457,6 @@ class ArcVmClientAdapter : public ArcClientAdapter,
         base::BindOnce(&base::SysInfo::AmountOfFreeDiskSpace,
                        base::FilePath(kHomeDirectory)),
         base::BindOnce(&ArcVmClientAdapter::CreateDiskImageAfterSizeCheck,
-
                        weak_factory_.GetWeakPtr(), std::move(params),
                        std::move(callback)));
   }
@@ -550,8 +563,7 @@ class ArcVmClientAdapter : public ArcClientAdapter,
     // implemented.
     chromeos::UpstartClient::Get()->StopJob(
         kArcVmServerProxyJobName, /*environment=*/{},
-        base::BindOnce(&ArcVmClientAdapter::OnArcVmServerProxyJobStopped,
-                       weak_factory_.GetWeakPtr()));
+        chromeos::EmptyVoidDBusMethodCallback());
 
     // If this method is called before even mini VM is started (e.g. very early
     // vm_concierge crash), or this method is called twice (e.g. crosvm crash
@@ -593,10 +605,6 @@ class ArcVmClientAdapter : public ArcClientAdapter,
                        std::move(callback)));
   }
 
-  void OnArcVmServerProxyJobStopped(bool result) {
-    VLOG(1) << "OnArcVmServerProxyJobStopped result=" << result;
-  }
-
   const version_info::Channel channel_;
 
   base::Optional<bool> is_dev_mode_;