borealis: Integrating launch watcher into context manager

Integrating the launch watcher into the context manager so that the
startup process can await on borealis starting.

Tests: Built and ran tests, tested on DUT.
Bug: b/171662734
Change-Id: I917440d9c190242bc7d4e28120e716702496f60c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2497344
Commit-Queue: Daniel Ng <danielng@google.com>
Reviewed-by: Nic Hollingum <hollingum@google.com>
Cr-Commit-Position: refs/heads/master@{#820648}

chrome/browser/chromeos/borealis/borealis_context_manager.h

@@ -22,6 +22,7 @@ class BorealisContextManager : public KeyedService {
     kMountFailed = 1,
     kDiskImageFailed = 2,
     kStartVmFailed = 3,
+    kAwaitBorealisStartupFailed = 4,
   };
 
   // An attempt to launch borealis. If the launch succeeds, holds a reference to

chrome/browser/chromeos/borealis/borealis_context_manager_impl.cc

@@ -23,6 +23,8 @@ std::ostream& operator<<(std::ostream& stream,
       return stream << "Disk Image Failed";
     case borealis::BorealisContextManager::kStartVmFailed:
       return stream << "Start VM Failed";
+    case borealis::BorealisContextManager::kAwaitBorealisStartupFailed:
+      return stream << "Await Borealis Startup Failed";
   }
 }
 
@@ -50,11 +52,15 @@ void BorealisContextManagerImpl::StartBorealis(ResultCallback callback) {
 
 base::queue<std::unique_ptr<BorealisTask>>
 BorealisContextManagerImpl::GetTasks() {
+  // We use a hard-coded name. When multi-instance becomes a feature we'll
+  // need to determine the name instead.
+  context_.set_vm_name("borealis");
   base::queue<std::unique_ptr<BorealisTask>> task_queue;
   task_queue.push(std::make_unique<MountDlc>());
   task_queue.push(std::make_unique<CreateDiskImage>());
   task_queue.push(std::make_unique<StartBorealisVm>());
-  task_queue.push(std::make_unique<AwaitBorealisStartup>());
+  task_queue.push(std::make_unique<AwaitBorealisStartup>(context_.profile(),
+                                                         context_.vm_name()));
   return task_queue;
 }
 

chrome/browser/chromeos/borealis/borealis_launch_watcher.h

@@ -45,7 +45,7 @@ class BorealisLaunchWatcher : public chromeos::CiceroneClient::Observer {
 
   std::string owner_id_;
   std::string vm_name_;
-  base::TimeDelta timeout_ = base::TimeDelta::FromMilliseconds(5000);
+  base::TimeDelta timeout_ = base::TimeDelta::FromMilliseconds(30000);
   base::Optional<vm_tools::cicerone::ContainerStartedSignal>
       container_started_signal_;
   base::queue<OnLaunchCallback> callback_queue_;

chrome/browser/chromeos/borealis/borealis_task.cc

@@ -48,9 +48,6 @@ CreateDiskImage::~CreateDiskImage() = default;
 
 void CreateDiskImage::Run(BorealisContext* context,
                           CompletionStatusCallback callback) {
-  // We use a hard-coded name. When multi-instance becomes a feature we'll
-  // need to determine the name instead.
-  context->set_vm_name("borealis");
   vm_tools::concierge::CreateDiskImageRequest request;
   request.set_disk_path(context->vm_name());
   request.set_cryptohome_id(
@@ -147,12 +144,32 @@ void StartBorealisVm::OnStartBorealisVm(
           base::NumberToString(response->status()) + ")");
 }
 
+AwaitBorealisStartup::AwaitBorealisStartup(Profile* profile,
+                                           std::string vm_name)
+    : watcher_(profile, vm_name) {}
+AwaitBorealisStartup::~AwaitBorealisStartup() = default;
+
 void AwaitBorealisStartup::Run(BorealisContext* context,
                                CompletionStatusCallback callback) {
-  // TODO(b/170696557): Refactor to use the LaunchWatcher which is not finished
-  // yet. In our case the name is hard-coded, so we can use that.
-  context->set_container_name("penguin");
-  std::move(callback).Run(BorealisContextManager::kSuccess, "");
+  watcher_.AwaitLaunch(
+      base::BindOnce(&AwaitBorealisStartup::OnAwaitBorealisStartup,
+                     weak_factory_.GetWeakPtr(), context, std::move(callback)));
 }
 
+BorealisLaunchWatcher& AwaitBorealisStartup::GetWatcherForTesting() {
+  return watcher_;
+}
+
+void AwaitBorealisStartup::OnAwaitBorealisStartup(
+    BorealisContext* context,
+    CompletionStatusCallback callback,
+    base::Optional<std::string> container) {
+  if (!container) {
+    std::move(callback).Run(BorealisContextManager::kAwaitBorealisStartupFailed,
+                            "Awaiting for Borealis launch failed: timed out");
+    return;
+  }
+  context->set_container_name(container.value());
+  std::move(callback).Run(BorealisContextManager::kSuccess, "");
+}
 }  // namespace borealis

chrome/browser/chromeos/borealis/borealis_task.h

@@ -7,6 +7,7 @@
 
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
+#include "chrome/browser/chromeos/borealis/borealis_launch_watcher.h"
 #include "chromeos/dbus/concierge_client.h"
 #include "chromeos/dbus/dlcservice/dlcservice_client.h"
 
@@ -82,8 +83,18 @@ class StartBorealisVm : public BorealisTask {
 // Waits for the startup daemon to signal completion.
 class AwaitBorealisStartup : public BorealisTask {
  public:
+  AwaitBorealisStartup(Profile* profile, std::string vm_name);
+  ~AwaitBorealisStartup() override;
   void Run(BorealisContext* context,
            CompletionStatusCallback callback) override;
+  BorealisLaunchWatcher& GetWatcherForTesting();
+
+ private:
+  void OnAwaitBorealisStartup(BorealisContext* context,
+                              CompletionStatusCallback callback,
+                              base::Optional<std::string> container);
+  BorealisLaunchWatcher watcher_;
+  base::WeakPtrFactory<AwaitBorealisStartup> weak_factory_{this};
 };
 
 }  // namespace borealis

chrome/browser/chromeos/borealis/borealis_task_unittest.cc

@@ -8,9 +8,11 @@
 
 #include "chrome/browser/chromeos/borealis/borealis_context.h"
 #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/dlcservice/fake_dlcservice_client.h"
+#include "chromeos/dbus/fake_cicerone_client.h"
 #include "chromeos/dbus/fake_concierge_client.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -49,8 +51,11 @@ class BorealisTasksTest : public testing::Test {
     chromeos::DBusThreadManager::Initialize();
     fake_concierge_client_ = static_cast<chromeos::FakeConciergeClient*>(
         chromeos::DBusThreadManager::Get()->GetConciergeClient());
+    fake_cicerone_client_ = static_cast<chromeos::FakeCiceroneClient*>(
+        chromeos::DBusThreadManager::Get()->GetCiceroneClient());
     CreateProfile();
     context_ = BorealisContext::CreateBorealisContextForTesting(profile_.get());
+    context_->set_vm_name("borealis");
 
     chromeos::DlcserviceClient::InitializeFake();
     fake_dlcservice_client_ = static_cast<chromeos::FakeDlcserviceClient*>(
@@ -69,6 +74,7 @@ class BorealisTasksTest : public testing::Test {
   content::BrowserTaskEnvironment task_environment_;
   // Owned by chromeos::DBusThreadManager
   chromeos::FakeConciergeClient* fake_concierge_client_;
+  chromeos::FakeCiceroneClient* fake_cicerone_client_;
   chromeos::FakeDlcserviceClient* fake_dlcservice_client_;
 
  private:
@@ -164,6 +170,55 @@ TEST_F(BorealisTasksTest,
   EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
 }
 
+TEST_F(BorealisTasksTest,
+       AwaitBorealisStartupSucceedsAndCallbackRanWithResults) {
+  vm_tools::cicerone::ContainerStartedSignal signal;
+  signal.set_owner_id(
+      chromeos::ProfileHelper::GetUserIdHashFromProfile(context_->profile()));
+  signal.set_vm_name(context_->vm_name());
+  signal.set_container_name("penguin");
+
+  testing::StrictMock<CallbackForTesting> callback;
+  EXPECT_CALL(callback, Callback(BorealisContextManager::kSuccess, _));
+
+  AwaitBorealisStartup task(context_->profile(), context_->vm_name());
+  task.Run(context_.get(), callback.GetCallback());
+  fake_cicerone_client_->NotifyContainerStarted(std::move(signal));
+
+  task_environment_.RunUntilIdle();
+}
+
+TEST_F(BorealisTasksTest,
+       AwaitBorealisStartupContainerAlreadyStartedAndCallbackRanWithResults) {
+  vm_tools::cicerone::ContainerStartedSignal signal;
+  signal.set_owner_id(
+      chromeos::ProfileHelper::GetUserIdHashFromProfile(context_->profile()));
+  signal.set_vm_name(context_->vm_name());
+  signal.set_container_name("penguin");
+
+  testing::StrictMock<CallbackForTesting> callback;
+  EXPECT_CALL(callback, Callback(BorealisContextManager::kSuccess, _));
+
+  AwaitBorealisStartup task(context_->profile(), context_->vm_name());
+  fake_cicerone_client_->NotifyContainerStarted(std::move(signal));
+  task.Run(context_.get(), callback.GetCallback());
+  task_environment_.RunUntilIdle();
+}
+
+TEST_F(BorealisTasksTest,
+       AwaitBorealisStartupTimesOutAndCallbackRanWithResults) {
+  testing::StrictMock<CallbackForTesting> callback;
+  EXPECT_CALL(
+      callback,
+      Callback(BorealisContextManager::kAwaitBorealisStartupFailed, StrNe("")));
+
+  AwaitBorealisStartup task(context_->profile(), context_->vm_name());
+  task.GetWatcherForTesting().SetTimeoutForTesting(
+      base::TimeDelta::FromMilliseconds(0));
+  task.Run(context_.get(), callback.GetCallback());
+  task_environment_.RunUntilIdle();
+}
+
 class BorealisTasksTestDlc : public BorealisTasksTest,
                              public testing::WithParamInterface<std::string> {};