Track Vm state while 'running' (starting, started, stopping)

This allows CrostiniManager to handle states where a VM hasn't finished
starting/stopping fully.

BUG=880165

Change-Id: Ia94bf4059c4bfbf41c3fe920873fa7c8bc2aea18
Reviewed-on: https://chromium-review.googlesource.com/1212206Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Josh Pratt <jopra@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589449}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -381,26 +381,46 @@ class CrostiniManager::CrostiniRestarter
 CrostiniManager::RestartId
     CrostiniManager::CrostiniRestarter::next_restart_id_ = 0;
 
+void CrostiniManager::SetVmState(std::string vm_name, VmState vm_state) {
+  auto vm_info = running_vms_.find(std::move(vm_name));
+  if (vm_info != running_vms_.end()) {
+    vm_info->second.first = vm_state;
+    return;
+  }
+  // This can happen normally when StopVm is called right after start up.
+  LOG(WARNING) << "Attempted to set state for unknown vm: " << vm_name;
+}
+
 bool CrostiniManager::IsVmRunning(std::string vm_name) {
-  return running_vms_.find(std::move(vm_name)) != running_vms_.end();
+  auto vm_info = running_vms_.find(std::move(vm_name));
+  if (vm_info != running_vms_.end()) {
+    return vm_info->second.first == VmState::STARTED;
+  }
+  return false;
 }
 
 base::Optional<vm_tools::concierge::VmInfo> CrostiniManager::GetVmInfo(
     std::string vm_name) {
   auto it = running_vms_.find(std::move(vm_name));
   if (it != running_vms_.end())
-    return it->second;
+    return it->second.second;
   return base::nullopt;
 }
 
 void CrostiniManager::AddRunningVmForTesting(
     std::string vm_name,
     vm_tools::concierge::VmInfo vm_info) {
-  running_vms_[std::move(vm_name)] = std::move(vm_info);
+  running_vms_[std::move(vm_name)] =
+      std::make_pair(VmState::STARTED, std::move(vm_info));
 }
 
 bool CrostiniManager::IsContainerRunning(std::string vm_name,
                                          std::string container_name) {
+  if (!IsVmRunning(vm_name)) {
+    return false;
+  }
+  // TODO(jopra): Ensure the container not marked running if the vm is not
+  // running.
   auto range = running_containers_.equal_range(std::move(vm_name));
   for (auto it = range.first; it != range.second; ++it) {
     if (it->second == container_name) {
@@ -674,8 +694,7 @@ void CrostiniManager::DestroyDiskImage(
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void CrostiniManager::ListVmDisks(
-    ListVmDisksCallback callback) {
+void CrostiniManager::ListVmDisks(ListVmDisksCallback callback) {
   vm_tools::concierge::ListVmDisksRequest request;
   request.set_cryptohome_id(CryptohomeIdForProfile(profile_));
   request.set_storage_location(vm_tools::concierge::STORAGE_CRYPTOHOME_ROOT);
@@ -726,6 +745,8 @@ void CrostiniManager::StopVm(std::string name, StopVmCallback callback) {
     return;
   }
 
+  SetVmState(name, VmState::STOPPING);
+
   vm_tools::concierge::StopVmRequest request;
   request.set_owner_id(owner_id_);
   request.set_name(name);
@@ -1149,31 +1170,34 @@ void CrostiniManager::OnStartTerminaVm(
     std::move(callback).Run(ConciergeClientResult::VM_START_FAILED);
     return;
   }
-  // Wait for the Tremplin signal if the vm isn't already marked "running".
-  if (running_vms_.find(vm_name) == running_vms_.end()) {
-    VLOG(1) << "Awaiting TremplinStartedSignal for " << owner_id_ << ", "
-            << vm_name;
-
-    // Record the container start and run the callback after the VM starts.
-    tremplin_started_callbacks_.emplace(
-        vm_name,
-        base::BindOnce(&CrostiniManager::OnStartTremplin,
-                       weak_ptr_factory_.GetWeakPtr(), vm_name,
-                       std::move(response.vm_info()), std::move(callback),
-                       ConciergeClientResult::SUCCESS));
+
+  // If the vm is already marked "running" run the callback.
+  if (IsVmRunning(vm_name)) {
+    std::move(callback).Run(ConciergeClientResult::SUCCESS);
     return;
   }
-  std::move(callback).Run(ConciergeClientResult::SUCCESS);
+
+  // Otherwise, record the container start and run the callback after the VM
+  // starts.
+  VLOG(1) << "Awaiting TremplinStartedSignal for " << owner_id_ << ", "
+          << vm_name;
+  running_vms_[vm_name] =
+      std::make_pair(VmState::STARTING, std::move(response.vm_info()));
+
+  tremplin_started_callbacks_.emplace(
+      vm_name,
+      base::BindOnce(&CrostiniManager::OnStartTremplin,
+                     weak_ptr_factory_.GetWeakPtr(), vm_name,
+                     std::move(callback), ConciergeClientResult::SUCCESS));
 }
 
 void CrostiniManager::OnStartTremplin(std::string vm_name,
-                                      vm_tools::concierge::VmInfo vm_info,
                                       StartTerminaVmCallback callback,
                                       ConciergeClientResult result) {
   // Record the running vm.
   VLOG(1) << "Received TremplinStartedSignal, VM: " << owner_id_ << ", "
           << vm_name;
-  running_vms_[vm_name] = std::move(vm_info);
+  SetVmState(vm_name, VmState::STARTED);
 
   // Run the original callback.
   std::move(callback).Run(result);

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -55,6 +55,12 @@ enum class InstallLinuxPackageProgressStatus {
   INSTALLING,
 };
 
+enum class VmState {
+  STARTING,
+  STARTED,
+  STOPPING,
+};
+
 // Return type when getting app icons from within a container.
 struct Icon {
   std::string desktop_file_id;
@@ -347,7 +353,9 @@ class CrostiniManager : public KeyedService,
                       std::string container_name,
                       RemoveCrostiniCallback callback);
 
+  void SetVmState(std::string vm_name, VmState vm_state);
   bool IsVmRunning(std::string vm_name);
+
   // Returns null if VM is not running.
   base::Optional<vm_tools::concierge::VmInfo> GetVmInfo(std::string vm_name);
   void AddRunningVmForTesting(std::string vm_name,
@@ -381,8 +389,9 @@ class CrostiniManager : public KeyedService,
       base::Optional<vm_tools::concierge::ListVmDisksResponse> reply);
 
   // Callback for ConciergeClient::StartTerminaVm. Called after the Concierge
-  // service method finishes. |callback| is called if the container has already
-  // been started, otherwise it is passed to OnStartTremplin.
+  // service method finishes.  Updates running containers list then calls the
+  // |callback| if the container has already been started, otherwise passes the
+  // callback to OnStartTremplin.
   void OnStartTerminaVm(
       std::string vm_name,
       StartTerminaVmCallback callback,
@@ -392,7 +401,6 @@ class CrostiniManager : public KeyedService,
   // Tremplin service starts. Updates running containers list and then calls the
   // |callback|.
   void OnStartTremplin(std::string vm_name,
-                       vm_tools::concierge::VmInfo vm_info,
                        StartTerminaVmCallback callback,
                        ConciergeClientResult result);
 
@@ -506,7 +514,8 @@ class CrostiniManager : public KeyedService,
   // start.
   std::multimap<std::string, base::OnceClosure> tremplin_started_callbacks_;
 
-  std::map<std::string, vm_tools::concierge::VmInfo> running_vms_;
+  std::map<std::string, std::pair<VmState, vm_tools::concierge::VmInfo>>
+      running_vms_;
 
   // Running containers as keyed by vm name.
   std::multimap<std::string, std::string> running_containers_;