CrostiniManager uses status for StartTerminaVm.

Only if the response proto status is "STARTING" do we wait for the
TremplinStartedSignal before continuing.

Bug: 884576
Change-Id: Id02a5863c2971329991d47bf156639fdc7f281bd
Reviewed-on: https://chromium-review.googlesource.com/1248065
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594629}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -1199,20 +1199,24 @@ void CrostiniManager::OnStartTerminaVm(
   }
   vm_tools::concierge::StartVmResponse response = reply.value();
 
-  if (!response.success()) {
+  if (response.status() == vm_tools::concierge::VM_STATUS_FAILURE ||
+      response.status() == vm_tools::concierge::VM_STATUS_UNKNOWN) {
     LOG(ERROR) << "Failed to start VM: " << response.failure_reason();
     std::move(callback).Run(ConciergeClientResult::VM_START_FAILED);
     return;
   }
 
   // If the vm is already marked "running" run the callback.
-  if (IsVmRunning(vm_name)) {
+  if (response.status() == vm_tools::concierge::VM_STATUS_RUNNING) {
+    running_vms_[vm_name] =
+        std::make_pair(VmState::STARTED, std::move(response.vm_info()));
     std::move(callback).Run(ConciergeClientResult::SUCCESS);
     return;
   }
 
   // Otherwise, record the container start and run the callback after the VM
   // starts.
+  DCHECK_EQ(response.status(), vm_tools::concierge::VM_STATUS_STARTING);
   VLOG(1) << "Awaiting TremplinStartedSignal for " << owner_id_ << ", "
           << vm_name;
   running_vms_[vm_name] =

chrome/browser/chromeos/crostini/crostini_share_path_unittest.cc

@@ -150,7 +150,7 @@ class CrostiniSharePathTest : public testing::Test {
 
 TEST_F(CrostiniSharePathTest, Success) {
   vm_tools::concierge::StartVmResponse start_vm_response;
-  start_vm_response.set_success(true);
+  start_vm_response.set_status(vm_tools::concierge::VM_STATUS_RUNNING);
   start_vm_response.mutable_vm_info()->set_seneschal_server_handle(123);
   fake_concierge_client_->set_start_vm_response(start_vm_response);
 
@@ -164,7 +164,7 @@ TEST_F(CrostiniSharePathTest, Success) {
 
 TEST_F(CrostiniSharePathTest, SharePathErrorSeneschal) {
   vm_tools::concierge::StartVmResponse start_vm_response;
-  start_vm_response.set_success(true);
+  start_vm_response.set_status(vm_tools::concierge::VM_STATUS_RUNNING);
   start_vm_response.mutable_vm_info()->set_seneschal_server_handle(123);
   fake_concierge_client_->set_start_vm_response(start_vm_response);
 

chrome/browser/ui/views/crostini/crostini_installer_view_browsertest.cc

@@ -220,7 +220,7 @@ IN_PROC_BROWSER_TEST_F(CrostiniInstallerViewBrowserTest, ErrorThenCancel) {
   ShowUi("default");
   EXPECT_NE(nullptr, ActiveView());
   vm_tools::concierge::StartVmResponse response;
-  response.set_success(false);
+  response.set_status(vm_tools::concierge::VM_STATUS_FAILURE);
   waiting_fake_concierge_client_->set_start_vm_response(std::move(response));
 
   ActiveView()->GetDialogClientView()->AcceptWindow();

chromeos/dbus/fake_concierge_client.cc

@@ -67,6 +67,11 @@ void FakeConciergeClient::StartTerminaVm(
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(std::move(callback), start_vm_response_));
 
+  if (start_vm_response_.status() != vm_tools::concierge::VM_STATUS_STARTING) {
+    // Don't send the tremplin signal unless the VM was STARTING.
+    return;
+  }
+
   // Trigger CiceroneClient::Observer::NotifyTremplinStartedSignal.
   vm_tools::cicerone::TremplinStartedSignal tremplin_started_signal;
   tremplin_started_signal.set_vm_name(request.name());
@@ -123,7 +128,7 @@ void FakeConciergeClient::InitializeProtoResponses() {
   list_vm_disks_response_.set_success(true);
 
   start_vm_response_.Clear();
-  start_vm_response_.set_success(true);
+  start_vm_response_.set_status(vm_tools::concierge::VM_STATUS_STARTING);
 
   stop_vm_response_.Clear();
   stop_vm_response_.set_success(true);