arcvm: Ignore stale VmStopped signals from Concierge

As noted in b/143229899, a stale VmStopped signal can be sent to
Chrome if StopAllVms() call sent by session_manager takes a long
time to finish. Without this CL, such a signal could confuse the
VM adapter.

BUG=b:143229899
TEST=new tests pass

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

components/arc/session/arc_vm_client_adapter.cc

@@ -57,6 +57,8 @@ constexpr const char kKernel[] = "vmlinux";
 constexpr const char kRootFs[] = "system.raw.img";
 constexpr const char kVendorImage[] = "vendor.raw.img";
 
+constexpr int64_t kInvalidCid = -1;
+
 // A move-only class to hold status of the host file system.
 class FileSystemStatus {
  public:
@@ -357,7 +359,14 @@ class ArcVmClientAdapter : public ArcClientAdapter,
       const vm_tools::concierge::VmStoppedSignal& signal) override {
     if (signal.name() != kArcVmName)
       return;
-    VLOG(1) << "OnVmStopped: ARCVM";
+    const int64_t cid = signal.cid();
+    if (cid != current_cid_) {
+      VLOG(1) << "Ignoring VmStopped signal: current CID=" << current_cid_
+              << ", stopped CID=" << cid;
+      return;
+    }
+    VLOG(1) << "OnVmStopped: ARCVM cid=" << cid;
+    current_cid_ = kInvalidCid;
     OnArcInstanceStopped();
   }
 
@@ -568,7 +577,9 @@ class ArcVmClientAdapter : public ArcClientAdapter,
       std::move(callback).Run(false);
       return;
     }
-    VLOG(1) << "arcvm started.";
+    current_cid_ = response.vm_info().cid();
+
+    VLOG(1) << "ARCVM started cid=" << current_cid_;
     std::move(callback).Run(true);
   }
 
@@ -616,6 +627,7 @@ class ArcVmClientAdapter : public ArcClientAdapter,
 
   StartParams start_params_;
   bool should_notify_observers_ = false;
+  int64_t current_cid_ = kInvalidCid;
 
   // For callbacks.
   base::WeakPtrFactory<ArcVmClientAdapter> weak_factory_{this};

components/arc/session/arc_vm_client_adapter_unittest.cc

@@ -27,6 +27,7 @@ namespace arc {
 namespace {
 
 constexpr const char kUserIdHash[] = "this_is_a_valid_user_id_hash";
+constexpr int64_t kCid = 123;
 
 // A debugd client that can fail to start Concierge.
 // TODO(yusukes): Merge the feature to FakeDebugDaemonClient.
@@ -107,6 +108,8 @@ class ArcVmClientAdapterTest : public testing::Test,
     // to VM_STATUS_RUNNING which is used by ARCVM.
     vm_tools::concierge::StartVmResponse start_vm_response;
     start_vm_response.set_status(vm_tools::concierge::VM_STATUS_RUNNING);
+    auto* vm_info = start_vm_response.mutable_vm_info();
+    vm_info->set_cid(kCid);
     GetTestConciergeClient()->set_start_vm_response(start_vm_response);
   }
 
@@ -161,14 +164,17 @@ class ArcVmClientAdapterTest : public testing::Test,
     RecreateRunLoop();
   }
 
-  void SendVmStoppedSignal() {
+  void SendVmStoppedSignalForCid(int64_t cid) {
     vm_tools::concierge::VmStoppedSignal signal;
     signal.set_name(kArcVmName);
+    signal.set_cid(kCid);
     auto& vm_observers = GetTestConciergeClient()->vm_observer_list();
     for (auto& observer : vm_observers)
       observer.OnVmStopped(signal);
   }
 
+  void SendVmStoppedSignal() { SendVmStoppedSignalForCid(kCid); }
+
   void SendNameOwnerChangedSignal() {
     auto& observers = GetTestConciergeClient()->observer_list();
     for (auto& observer : observers)
@@ -222,7 +228,10 @@ TEST_F(ArcVmClientAdapterTest, StartMiniArc) {
 
 // Tests that StopArcInstance() eventually notifies the observer.
 TEST_F(ArcVmClientAdapterTest, StopArcInstance) {
+  SetValidUserIdHash();
   StartMiniArc();
+  UpgradeArc(true);
+
   adapter()->StopArcInstance();
   run_loop()->RunUntilIdle();
   EXPECT_TRUE(GetTestConciergeClient()->stop_vm_called());
@@ -443,6 +452,27 @@ TEST_F(ArcVmClientAdapterTest, CrosvmAndConciergeCrashes) {
   EXPECT_FALSE(arc_instance_stopped_called());
 }
 
+// Tests the case where a unknown VmStopped signal is sent to Chrome.
+TEST_F(ArcVmClientAdapterTest, VmStoppedSignal_UnknownCid) {
+  SetValidUserIdHash();
+  StartMiniArc();
+  UpgradeArc(true);
+  EXPECT_TRUE(GetStartConciergeCalled());
+  EXPECT_TRUE(GetTestConciergeClient()->start_arc_vm_called());
+  EXPECT_FALSE(arc_instance_stopped_called());
+
+  SendVmStoppedSignalForCid(42);  // unknown CID
+  run_loop()->RunUntilIdle();
+  EXPECT_TRUE(arc_instance_stopped_called());
+}
+
+// Tests the case where a stale VmStopped signal is sent to Chrome.
+TEST_F(ArcVmClientAdapterTest, VmStoppedSignal_Stale) {
+  SendVmStoppedSignalForCid(42);
+  run_loop()->RunUntilIdle();
+  EXPECT_FALSE(arc_instance_stopped_called());
+}
+
 // Tests if androidboot.debuggable is set properly.
 TEST_F(ArcVmClientAdapterTest, IsAndroidDebuggable) {
   constexpr const char kAndroidDebuggableTrueJson[] = R"json({