Add functionality to launch borealis apps from the launcher

We add a BorealisAppLauncher class with associated tests, and hook this
class into the BorealisApps service.

This needed minor refactorings of the BorealisContext.

Bug: b/162562622
Change-Id: I3b6bc49b6d494fef4f8a03810048ceb3fd9122b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485998
Auto-Submit: Nic Hollingum <hollingum@google.com>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Daniel Ng <danielng@google.com>
Commit-Queue: Nic Hollingum <hollingum@google.com>
Cr-Commit-Position: refs/heads/master@{#820638}

chrome/browser/apps/app_service/borealis_apps.cc

@@ -5,8 +5,11 @@
 #include "chrome/browser/apps/app_service/borealis_apps.h"
 
 #include "ash/public/cpp/app_menu_constants.h"
+#include "base/bind.h"
 #include "chrome/browser/apps/app_service/app_icon_factory.h"
 #include "chrome/browser/apps/app_service/menu_util.h"
+#include "chrome/browser/chromeos/borealis/borealis_app_launcher.h"
+#include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
 #include "chrome/browser/chromeos/borealis/borealis_context_manager_factory.h"
 #include "chrome/browser/chromeos/borealis/borealis_context_manager_impl.h"
 #include "chrome/browser/chromeos/borealis/borealis_features.h"
@@ -172,7 +175,18 @@ void BorealisApps::Launch(const std::string& app_id,
           ->Features()
           .IsEnabled()) {
     borealis::BorealisContextManagerFactory::GetForProfile(profile_)
-        ->StartBorealis(base::DoNothing());
+        ->StartBorealis(base::BindOnce(
+            [](const std::string& app_id,
+               borealis::BorealisContextManager::Result result) {
+              if (!result.Ok()) {
+                LOG(ERROR) << "Failed to launch " << app_id << ": "
+                           << result.FailureReason();
+                return;
+              }
+              borealis::BorealisAppLauncher::Launch(result.Success(), app_id,
+                                                    base::DoNothing());
+            },
+            app_id));
     return;
   }
   borealis::ShowBorealisInstallerView(profile_);

chrome/browser/chromeos/BUILD.gn

@@ -838,6 +838,8 @@ source_set("chromeos") {
     "bluetooth/debug_logs_manager_factory.h",
     "boot_times_recorder.cc",
     "boot_times_recorder.h",
+    "borealis/borealis_app_launcher.cc",
+    "borealis/borealis_app_launcher.h",
     "borealis/borealis_context.cc",
     "borealis/borealis_context.h",
     "borealis/borealis_context_manager.cc",
@@ -3276,6 +3278,7 @@ source_set("unit_tests") {
     "authpolicy/authpolicy_helper.unittest.cc",
     "base/file_flusher_unittest.cc",
     "bluetooth/debug_logs_manager_unittest.cc",
+    "borealis/borealis_app_launcher_unittest.cc",
     "borealis/borealis_context_manager_unittest.cc",
     "borealis/borealis_features_unittest.cc",
     "borealis/borealis_installer_unittest.cc",

chrome/browser/chromeos/borealis/borealis_app_launcher.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/borealis/borealis_app_launcher.h"
+
+#include "chrome/browser/chromeos/borealis/borealis_context.h"
+#include "chrome/browser/chromeos/borealis/borealis_util.h"
+#include "chrome/browser/chromeos/guest_os/guest_os_registry_service.h"
+#include "chrome/browser/chromeos/guest_os/guest_os_registry_service_factory.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chromeos/dbus/cicerone/cicerone_service.pb.h"
+#include "chromeos/dbus/cicerone_client.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+
+namespace borealis {
+
+void BorealisAppLauncher::Launch(const BorealisContext& ctx,
+                                 const std::string& app_id,
+                                 OnLaunchedCallback callback) {
+  // Do not launch anything when using the installer app.
+  //
+  // TODO(b/170677773): Launch a _certain_ application...
+  if (app_id == kBorealisAppId) {
+    std::move(callback).Run(kSuccess);
+    return;
+  }
+
+  base::Optional<guest_os::GuestOsRegistryService::Registration> reg =
+      guest_os::GuestOsRegistryServiceFactory::GetForProfile(ctx.profile())
+          ->GetRegistration(app_id);
+
+  if (!reg) {
+    std::move(callback).Run(kUnknownApp);
+    return;
+  }
+
+  vm_tools::cicerone::LaunchContainerApplicationRequest request;
+  request.set_owner_id(
+      chromeos::ProfileHelper::GetUserIdHashFromProfile(ctx.profile()));
+  request.set_vm_name(ctx.vm_name());
+  request.set_container_name(ctx.container_name());
+  request.set_desktop_file_id(reg->DesktopFileId());
+
+  chromeos::DBusThreadManager::Get()
+      ->GetCiceroneClient()
+      ->LaunchContainerApplication(
+          std::move(request),
+          base::BindOnce(
+              [](OnLaunchedCallback callback,
+                 base::Optional<
+                     vm_tools::cicerone::LaunchContainerApplicationResponse>
+                     response) {
+                if (!response) {
+                  LOG(ERROR)
+                      << "Failed to launch app: No response from cicerone";
+                  std::move(callback).Run(kNoResponse);
+                  return;
+                }
+                if (!response->success()) {
+                  LOG(ERROR)
+                      << "Failed to launch app: " << response->failure_reason();
+                  std::move(callback).Run(kError);
+                  return;
+                }
+                std::move(callback).Run(kSuccess);
+              },
+              std::move(callback)));
+}
+
+}  // namespace borealis

chrome/browser/chromeos/borealis/borealis_app_launcher.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_APP_LAUNCHER_H_
+#define CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_APP_LAUNCHER_H_
+
+#include <string>
+
+#include "base/bind_helpers.h"
+
+namespace borealis {
+
+class BorealisContext;
+
+// Helper class responsible for launching borealis' apps.
+class BorealisAppLauncher {
+ public:
+  enum LaunchResult {
+    kSuccess,
+    kUnknownApp,
+    kNoResponse,
+    kError,
+  };
+  using OnLaunchedCallback = base::OnceCallback<void(LaunchResult)>;
+  // Launch the app with the given |app_id| in the borealis instance referred to
+  // by |ctx|.
+  static void Launch(const BorealisContext& ctx,
+                     const std::string& app_id,
+                     OnLaunchedCallback callback);
+};
+
+}  // namespace borealis
+
+#endif  // CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_APP_LAUNCHER_H_

chrome/browser/chromeos/borealis/borealis_app_launcher_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/borealis/borealis_app_launcher.h"
+#include <memory>
+
+#include "base/bind.h"
+#include "base/callback_forward.h"
+#include "base/test/bind_test_util.h"
+#include "chrome/browser/chromeos/borealis/borealis_context.h"
+#include "chrome/browser/chromeos/borealis/borealis_util.h"
+#include "chrome/browser/chromeos/guest_os/guest_os_registry_service.h"
+#include "chrome/browser/chromeos/guest_os/guest_os_registry_service_factory.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_cicerone_client.h"
+#include "chromeos/dbus/vm_applications/apps.pb.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace borealis {
+namespace {
+
+class CallbackFactory
+    : public testing::StrictMock<
+          testing::MockFunction<void(BorealisAppLauncher::LaunchResult)>> {
+ public:
+  base::OnceCallback<void(BorealisAppLauncher::LaunchResult)> BindOnce() {
+    return base::BindOnce(&CallbackFactory::Call, base::Unretained(this));
+  }
+};
+
+class ScopedCiceroneFake {
+ public:
+  ScopedCiceroneFake() { chromeos::DBusThreadManager::Initialize(); }
+
+  ~ScopedCiceroneFake() { chromeos::DBusThreadManager::Shutdown(); }
+
+  chromeos::FakeCiceroneClient* Get() {
+    return reinterpret_cast<chromeos::FakeCiceroneClient*>(
+        chromeos::DBusThreadManager::Get()->GetCiceroneClient());
+  }
+};
+
+class BorealisAppLauncherTest : public testing::Test {
+ public:
+  BorealisAppLauncherTest()
+      : ctx_(BorealisContext::CreateBorealisContextForTesting(&profile_)) {
+    ctx_->set_vm_name("test_vm_name");
+    ctx_->set_container_name("test_container_name");
+  }
+
+ protected:
+  const BorealisContext& Context() { return *ctx_; }
+
+  chromeos::FakeCiceroneClient* Cicerone() { return cicerone_.Get(); }
+
+  // Sets up the registry with a single app. Returns its app id.
+  std::string SetDummyApp(const std::string& desktop_file_id) {
+    vm_tools::apps::ApplicationList list;
+    list.set_vm_name(Context().vm_name());
+    list.set_container_name(Context().container_name());
+    vm_tools::apps::App* app = list.add_apps();
+    app->set_desktop_file_id(desktop_file_id);
+    vm_tools::apps::App::LocaleString::Entry* entry =
+        app->mutable_name()->add_values();
+    entry->set_locale(std::string());
+    entry->set_value(desktop_file_id);
+    app->set_no_display(false);
+    guest_os::GuestOsRegistryServiceFactory::GetForProfile(&profile_)
+        ->UpdateApplicationList(list);
+    return guest_os::GuestOsRegistryService::GenerateAppId(
+        desktop_file_id, list.vm_name(), list.container_name());
+  }
+
+ private:
+  content::BrowserTaskEnvironment task_environment_;
+  ScopedCiceroneFake cicerone_;
+  TestingProfile profile_;
+  std::unique_ptr<BorealisContext> ctx_;
+};
+
+TEST_F(BorealisAppLauncherTest, LauncherAppAlwaysWorks) {
+  CallbackFactory callback_check;
+  EXPECT_CALL(callback_check, Call(BorealisAppLauncher::kSuccess));
+  BorealisAppLauncher::Launch(Context(), kBorealisAppId,
+                              callback_check.BindOnce());
+}
+
+TEST_F(BorealisAppLauncherTest, UnknownAppCausesError) {
+  CallbackFactory callback_check;
+  EXPECT_CALL(callback_check, Call(BorealisAppLauncher::kUnknownApp));
+  BorealisAppLauncher::Launch(Context(), "non_existent_app",
+                              callback_check.BindOnce());
+}
+
+TEST_F(BorealisAppLauncherTest, NoResponseCausesError) {
+  CallbackFactory callback_check;
+  EXPECT_CALL(callback_check, Call(BorealisAppLauncher::kNoResponse));
+  std::string baz_id = SetDummyApp("foo.desktop");
+  Cicerone()->SetOnLaunchContainerApplicationCallback(
+      base::BindLambdaForTesting(
+          [&](const vm_tools::cicerone::LaunchContainerApplicationRequest&
+                  request,
+              chromeos::DBusMethodCallback<
+                  vm_tools::cicerone::LaunchContainerApplicationResponse>
+                  callback) {
+            EXPECT_EQ(request.desktop_file_id(), "foo.desktop");
+            std::move(callback).Run({});
+          }));
+  BorealisAppLauncher::Launch(Context(), baz_id, callback_check.BindOnce());
+}
+
+TEST_F(BorealisAppLauncherTest, ErrorResponseIsPropagated) {
+  CallbackFactory callback_check;
+  EXPECT_CALL(callback_check, Call(BorealisAppLauncher::kError));
+  std::string baz_id = SetDummyApp("bar.desktop");
+  Cicerone()->SetOnLaunchContainerApplicationCallback(
+      base::BindLambdaForTesting(
+          [&](const vm_tools::cicerone::LaunchContainerApplicationRequest&
+                  request,
+              chromeos::DBusMethodCallback<
+                  vm_tools::cicerone::LaunchContainerApplicationResponse>
+                  callback) {
+            EXPECT_EQ(request.desktop_file_id(), "bar.desktop");
+            vm_tools::cicerone::LaunchContainerApplicationResponse response;
+            response.set_success(false);
+            std::move(callback).Run(response);
+          }));
+  BorealisAppLauncher::Launch(Context(), baz_id, callback_check.BindOnce());
+}
+
+TEST_F(BorealisAppLauncherTest, SuccessfulLaunchHasSuccessResponse) {
+  CallbackFactory callback_check;
+  EXPECT_CALL(callback_check, Call(BorealisAppLauncher::kSuccess));
+  std::string baz_id = SetDummyApp("baz.desktop");
+  Cicerone()->SetOnLaunchContainerApplicationCallback(
+      base::BindLambdaForTesting(
+          [&](const vm_tools::cicerone::LaunchContainerApplicationRequest&
+                  request,
+              chromeos::DBusMethodCallback<
+                  vm_tools::cicerone::LaunchContainerApplicationResponse>
+                  callback) {
+            EXPECT_EQ(request.desktop_file_id(), "baz.desktop");
+            vm_tools::cicerone::LaunchContainerApplicationResponse response;
+            response.set_success(true);
+            std::move(callback).Run(response);
+          }));
+  BorealisAppLauncher::Launch(Context(), baz_id, callback_check.BindOnce());
+}
+
+}  // namespace
+}  // namespace borealis

chrome/browser/chromeos/borealis/borealis_context.cc

@@ -3,11 +3,19 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/borealis/borealis_context.h"
+#include "base/memory/ptr_util.h"
 
 namespace borealis {
 
 BorealisContext::~BorealisContext() = default;
-BorealisContext::BorealisContext() = default;
+
 BorealisContext::BorealisContext(Profile* profile) : profile_(profile) {}
 
+std::unique_ptr<BorealisContext>
+BorealisContext::CreateBorealisContextForTesting(Profile* profile) {
+  // Construct out-of-place because the constructor is private.
+  BorealisContext* ptr = new BorealisContext(profile);
+  return base::WrapUnique(ptr);
+}
+
 }  // namespace borealis

chrome/browser/chromeos/borealis/borealis_context.h

@@ -5,10 +5,12 @@
 #ifndef CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_CONTEXT_H_
 #define CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_CONTEXT_H_
 
+#include <memory>
 #include <string>
 
 #include "base/files/file_path.h"
-#include "chrome/browser/profiles/profile.h"
+
+class Profile;
 
 namespace borealis {
 
@@ -21,12 +23,10 @@ class BorealisContext {
   BorealisContext& operator=(const BorealisContext&) = delete;
   ~BorealisContext();
 
-  static BorealisContext* CreateBorealisContextForTesting() {
-    return new BorealisContext();
-  }
+  static std::unique_ptr<BorealisContext> CreateBorealisContextForTesting(
+      Profile* profile);
 
-  Profile* profile() { return profile_; }
-  void set_profile(Profile* profile) { profile_ = profile; }
+  Profile* profile() const { return profile_; }
 
   bool borealis_running() const { return borealis_running_; }
   void set_borealis_running(bool success) { borealis_running_ = success; }
@@ -48,10 +48,9 @@ class BorealisContext {
  private:
   friend class BorealisContextManagerImpl;
 
-  BorealisContext();
   explicit BorealisContext(Profile* profile);
 
-  Profile* profile_ = nullptr;
+  Profile* const profile_;
   bool borealis_running_ = false;
   std::string vm_name_;
   std::string container_name_;

chrome/browser/chromeos/borealis/borealis_task_unittest.cc

@@ -50,8 +50,7 @@ class BorealisTasksTest : public testing::Test {
     fake_concierge_client_ = static_cast<chromeos::FakeConciergeClient*>(
         chromeos::DBusThreadManager::Get()->GetConciergeClient());
     CreateProfile();
-    context_ = BorealisContext::CreateBorealisContextForTesting();
-    context_->set_profile(profile_.get());
+    context_ = BorealisContext::CreateBorealisContextForTesting(profile_.get());
 
     chromeos::DlcserviceClient::InitializeFake();
     fake_dlcservice_client_ = static_cast<chromeos::FakeDlcserviceClient*>(
@@ -66,7 +65,7 @@ class BorealisTasksTest : public testing::Test {
   }
 
   std::unique_ptr<TestingProfile> profile_;
-  BorealisContext* context_;
+  std::unique_ptr<BorealisContext> context_;
   content::BrowserTaskEnvironment task_environment_;
   // Owned by chromeos::DBusThreadManager
   chromeos::FakeConciergeClient* fake_concierge_client_;
@@ -89,7 +88,7 @@ TEST_F(BorealisTasksTest, MountDlcSucceedsAndCallbackRanWithResults) {
   EXPECT_CALL(callback, Callback(BorealisContextManager::kSuccess, _));
 
   MountDlc task;
-  task.Run(context_, callback.GetCallback());
+  task.Run(context_.get(), callback.GetCallback());
   task_environment_.RunUntilIdle();
 
   EXPECT_EQ(context_->root_path(), "test/path");
@@ -107,7 +106,7 @@ TEST_F(BorealisTasksTest, CreateDiskSucceedsAndCallbackRanWithResults) {
   EXPECT_CALL(callback, Callback(BorealisContextManager::kSuccess, _));
 
   CreateDiskImage task;
-  task.Run(context_, callback.GetCallback());
+  task.Run(context_.get(), callback.GetCallback());
   task_environment_.RunUntilIdle();
 
   EXPECT_TRUE(fake_concierge_client_->create_disk_image_called());
@@ -127,7 +126,7 @@ TEST_F(BorealisTasksTest,
   EXPECT_CALL(callback, Callback(BorealisContextManager::kSuccess, _));
 
   CreateDiskImage task;
-  task.Run(context_, callback.GetCallback());
+  task.Run(context_.get(), callback.GetCallback());
   task_environment_.RunUntilIdle();
 
   EXPECT_TRUE(fake_concierge_client_->create_disk_image_called());
@@ -143,7 +142,7 @@ TEST_F(BorealisTasksTest, StartBorealisVmSucceedsAndCallbackRanWithResults) {
   EXPECT_CALL(callback, Callback(BorealisContextManager::kSuccess, _));
 
   StartBorealisVm task;
-  task.Run(context_, callback.GetCallback());
+  task.Run(context_.get(), callback.GetCallback());
   task_environment_.RunUntilIdle();
 
   EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
@@ -159,7 +158,7 @@ TEST_F(BorealisTasksTest,
   EXPECT_CALL(callback, Callback(BorealisContextManager::kSuccess, _));
 
   StartBorealisVm task;
-  task.Run(context_, callback.GetCallback());
+  task.Run(context_.get(), callback.GetCallback());
   task_environment_.RunUntilIdle();
 
   EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
@@ -175,7 +174,7 @@ TEST_P(BorealisTasksTestDlc, MountDlcFailsAndCallbackRanWithResults) {
               Callback(BorealisContextManager::kMountFailed, StrNe("")));
 
   MountDlc task;
-  task.Run(context_, callback.GetCallback());
+  task.Run(context_.get(), callback.GetCallback());
   task_environment_.RunUntilIdle();
 }
 
@@ -204,7 +203,7 @@ TEST_P(BorealisTasksTestDiskImage, CreateDiskFailsAndCallbackRanWithResults) {
               Callback(BorealisContextManager::kDiskImageFailed, StrNe("")));
 
   CreateDiskImage task;
-  task.Run(context_, callback.GetCallback());
+  task.Run(context_.get(), callback.GetCallback());
   task_environment_.RunUntilIdle();
 
   EXPECT_TRUE(fake_concierge_client_->create_disk_image_called());
@@ -236,7 +235,7 @@ TEST_P(BorealisTasksTestsStartBorealisVm,
               Callback(BorealisContextManager::kStartVmFailed, StrNe("")));
 
   StartBorealisVm task;
-  task.Run(context_, callback.GetCallback());
+  task.Run(context_.get(), callback.GetCallback());
   task_environment_.RunUntilIdle();
 
   EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());

chrome/browser/chromeos/crostini/crostini_test_helper.cc

@@ -150,8 +150,8 @@ std::string CrostiniTestHelper::GenerateAppId(
     const std::string& desktop_file_id,
     const std::string& vm_name,
     const std::string& container_name) {
-  return crx_file::id_util::GenerateId("crostini:" + vm_name + "/" +
-                                       container_name + "/" + desktop_file_id);
+  return guest_os::GuestOsRegistryService::GenerateAppId(
+      desktop_file_id, vm_name, container_name);
 }
 
 // static

chrome/browser/chromeos/guest_os/guest_os_registry_service.cc

@@ -57,16 +57,6 @@ constexpr char kCrostiniAppsInstalledHistogram[] =
 constexpr char kPluginVmAppsInstalledHistogram[] =
     "PluginVm.AppsInstalledAtLogin";
 
-std::string GenerateAppId(const std::string& desktop_file_id,
-                          const std::string& vm_name,
-                          const std::string& container_name) {
-  // These can collide in theory because the user could choose VM and container
-  // names which contain slashes, but this will only result in apps missing from
-  // the launcher.
-  return crx_file::id_util::GenerateId(kCrostiniAppIdPrefix + vm_name + "/" +
-                                       container_name + "/" + desktop_file_id);
-}
-
 base::Value ProtoToDictionary(const App::LocaleString& locale_string) {
   base::Value result(base::Value::Type::DICTIONARY);
   for (const App::LocaleString::Entry& entry : locale_string.values()) {
@@ -1040,6 +1030,18 @@ void GuestOsRegistryService::SetAppScaled(const std::string& app_id,
   app->SetKey(guest_os::prefs::kAppScaledKey, base::Value(scaled));
 }
 
+// static
+std::string GuestOsRegistryService::GenerateAppId(
+    const std::string& desktop_file_id,
+    const std::string& vm_name,
+    const std::string& container_name) {
+  // These can collide in theory because the user could choose VM and container
+  // names which contain slashes, but this will only result in apps missing from
+  // the launcher.
+  return crx_file::id_util::GenerateId(kCrostiniAppIdPrefix + vm_name + "/" +
+                                       container_name + "/" + desktop_file_id);
+}
+
 void GuestOsRegistryService::RequestContainerAppIcon(
     const std::string& app_id,
     ui::ScaleFactor scale_factor) {

chrome/browser/chromeos/guest_os/guest_os_registry_service.h

@@ -218,6 +218,12 @@ class GuestOsRegistryService : public KeyedService {
 
   void SetClockForTesting(base::Clock* clock) { clock_ = clock; }
 
+  // Returns the AppId that will be used to refer to the given GuestOs
+  // application.
+  static std::string GenerateAppId(const std::string& desktop_file_id,
+                                   const std::string& vm_name,
+                                   const std::string& container_name);
+
  private:
   // Run start up tasks for the registry (e.g. recording metrics).
   void RecordStartupMetrics();