Mark VMs as running in callback after Tremplin has started

This adds a second stage to OnStartTerminaVm in the form of a callback,
fired when Tremplin starts.

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

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -1328,19 +1328,33 @@ void CrostiniManager::OnStartTerminaVm(
   // Wait for the Tremplin signal if the vm isn't already marked "running".
   auto key = std::make_pair(owner_id, vm_name);
   if (running_vms_.find(key) == running_vms_.end()) {
-    // Record the running vm.
-    running_vms_[key] = std::move(response.vm_info());
-
     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(
-        key,
-        base::BindOnce(std::move(callback), ConciergeClientResult::SUCCESS));
+        key, base::BindOnce(&CrostiniManager::OnStartTremplin,
+                            weak_ptr_factory_.GetWeakPtr(), key,
+                            std::move(response.vm_info()), std::move(callback),
+                            ConciergeClientResult::SUCCESS));
     return;
   }
   std::move(callback).Run(ConciergeClientResult::SUCCESS);
 }
 
+void CrostiniManager::OnStartTremplin(std::pair<std::string, std::string> key,
+                                      vm_tools::concierge::VmInfo vm_info,
+                                      StartTerminaVmCallback callback,
+                                      ConciergeClientResult result) {
+  // Record the running vm.
+  VLOG(1) << "Received TremplinStartedSignal, VM: " << key.first << ", "
+          << key.second;
+  running_vms_[key] = std::move(vm_info);
+
+  // Run the original callback.
+  std::move(callback).Run(result);
+}
+
 void CrostiniManager::OnStopVm(
     std::string owner_id,
     std::string vm_name,

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -404,13 +404,22 @@ class CrostiniManager : public chromeos::ConciergeClient::Observer,
       base::Optional<vm_tools::concierge::ListVmDisksResponse> reply);
 
   // Callback for ConciergeClient::StartTerminaVm. Called after the Concierge
-  // service method finishes.
+  // service method finishes. |callback| is called if the container has already
+  // been started, otherwise it is passed to OnStartTremplin.
   void OnStartTerminaVm(
       std::string owner_id,
       std::string vm_name,
       StartTerminaVmCallback callback,
       base::Optional<vm_tools::concierge::StartVmResponse> reply);
 
+  // Callback for ConciergeClient::TremplinStartedSignal. Called after the
+  // Tremplin service starts. Updates running containers list and then calls the
+  // |callback|.
+  void OnStartTremplin(std::pair<std::string, std::string> key,
+                       vm_tools::concierge::VmInfo vm_info,
+                       StartTerminaVmCallback callback,
+                       ConciergeClientResult result);
+
   // Callback for ConciergeClient::StopVm. Called after the Concierge
   // service method finishes.
   void OnStopVm(std::string owner_id,

chrome/browser/chromeos/crostini/crostini_manager_unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
+#include <memory>
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
@@ -91,6 +92,14 @@ class CrostiniManagerTest : public testing::Test {
     std::move(closure).Run();
   }
 
+  void OnStartTremplinRecordsRunningVmCallback(base::OnceClosure closure,
+                                               ConciergeClientResult result) {
+    // Check that running_vms_ contains the running vm.
+    EXPECT_TRUE(
+        CrostiniManager::GetInstance()->IsVmRunning(profile(), kVmName));
+    std::move(closure).Run();
+  }
+
   void StopVmSuccessCallback(base::OnceClosure closure,
                              ConciergeClientResult result) {
     EXPECT_TRUE(fake_concierge_client_->stop_vm_called());
@@ -300,6 +309,23 @@ TEST_F(CrostiniManagerTest, StartTerminaVmSuccess) {
   run_loop()->Run();
 }
 
+TEST_F(CrostiniManagerTest, OnStartTremplinRecordsRunningVm) {
+  const base::FilePath& disk_path = base::FilePath(kVmName);
+  const std::string owner_id = CryptohomeIdForProfile(profile());
+
+  // Start the Vm.
+  CrostiniManager::GetInstance()->StartTerminaVm(
+      owner_id, kVmName, disk_path,
+      base::BindOnce(
+          &CrostiniManagerTest::OnStartTremplinRecordsRunningVmCallback,
+          base::Unretained(this), run_loop()->QuitClosure()));
+
+  // Check that the Vm start is not recorded (without tremplin start).
+  EXPECT_FALSE(CrostiniManager::GetInstance()->IsVmRunning(profile(), kVmName));
+
+  run_loop()->Run();
+}
+
 TEST_F(CrostiniManagerTest, StopVmNameError) {
   CrostiniManager::GetInstance()->StopVm(
       profile(), "",