Create default PluginVm shared dir and share on startup

Fetches VmInfo from concierge in order to get
seneschal server handle, then creates default
dir for sharing and shares it along with other
persisted shares.

Bug: 957477
Change-Id: I797d14c3590adc44cfdcefb0af25b37d64254f5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1631009
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Auto-Submit: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664688}

chrome/browser/chromeos/crostini/crostini_share_path.cc

@@ -16,6 +16,8 @@
 #include "chrome/browser/chromeos/drive/drive_integration_service.h"
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager.h"
+#include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h"
+#include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chromeos/components/drivefs/mojom/drivefs.mojom.h"
 #include "chromeos/constants/chromeos_features.h"
@@ -307,19 +309,27 @@ void CrostiniSharePath::CallSeneschalSharePath(const std::string& vm_name,
   request.mutable_shared_path()->set_path(relative_path.value());
   request.mutable_shared_path()->set_writable(true);
 
-  // Restart VM if not currently running.
-  auto* crostini_manager = crostini::CrostiniManager::GetForProfile(profile_);
-  base::Optional<crostini::VmInfo> vm_info =
-      crostini_manager->GetVmInfo(vm_name);
-  if (!vm_info || vm_info->state != crostini::VmState::STARTED) {
-    crostini_manager->RestartCrostini(
-        vm_name, crostini::kCrostiniDefaultContainerName,
-        base::BindOnce(&OnVmRestartedForSeneschal, profile_, vm_name,
-                       std::move(callback), std::move(request)));
-    return;
+  // Handle PluginVm.  TODO(joelhockey): If we ever require to (re)start
+  // PluginVm before sharing, we can detect that the VM is not started
+  // if handle == 0.
+  if (vm_name == plugin_vm::kPluginVmName) {
+    request.set_handle(plugin_vm::PluginVmManager::GetForProfile(profile_)
+                           ->seneschal_server_handle());
+  } else {
+    // Restart VM if not currently running.
+    auto* crostini_manager = crostini::CrostiniManager::GetForProfile(profile_);
+    base::Optional<crostini::VmInfo> vm_info =
+        crostini_manager->GetVmInfo(vm_name);
+    if (!vm_info || vm_info->state != crostini::VmState::STARTED) {
+      crostini_manager->RestartCrostini(
+          vm_name, crostini::kCrostiniDefaultContainerName,
+          base::BindOnce(&OnVmRestartedForSeneschal, profile_, vm_name,
+                         std::move(callback), std::move(request)));
+      return;
+    }
+    request.set_handle(vm_info->info.seneschal_server_handle());
   }
 
-  request.set_handle(vm_info->info.seneschal_server_handle());
   chromeos::DBusThreadManager::Get()->GetSeneschalClient()->SharePath(
       request,
       base::BindOnce(&OnSeneschalSharePathResponse, std::move(callback)));

chrome/browser/chromeos/crostini/crostini_share_path_unittest.cc

@@ -327,6 +327,19 @@ TEST_F(CrostiniSharePathTest, SuccessPersist) {
   run_loop()->Run();
 }
 
+TEST_F(CrostiniSharePathTest, SuccessPluginVm) {
+  features_.InitWithFeatures({chromeos::features::kMyFilesVolume}, {});
+  SetUpVolume();
+  crostini_share_path_->SharePath(
+      "PvmDefault", share_path_, PERSIST_NO,
+      base::BindOnce(&CrostiniSharePathTest::SharePathCallback,
+                     base::Unretained(this), "PvmDefault", Persist::NO,
+                     SeneschalClientCalled::YES,
+                     &vm_tools::seneschal::SharePathRequest::MY_FILES,
+                     "path-to-share", Success::YES, ""));
+  run_loop()->Run();
+}
+
 TEST_F(CrostiniSharePathTest, SuccessDriveFsMyDrive) {
   features_.InitWithFeatures({chromeos::features::kDriveFs}, {});
   SetUpVolume();

chrome/browser/chromeos/plugin_vm/plugin_vm_manager.cc

@@ -5,6 +5,8 @@
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h"
 
 #include "base/bind_helpers.h"
+#include "chrome/browser/chromeos/crostini/crostini_share_path.h"
+#include "chrome/browser/chromeos/plugin_vm/plugin_vm_files.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_metrics_util.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
@@ -73,6 +75,10 @@ void PluginVmManager::LaunchPluginVm() {
   // 2) Call ListVms to get the state of the VM
   // 3) Start the VM if necessary
   // 4) Show the UI.
+  // 5) Call Concierge::GetVmInfo to get seneschal server handle.
+  //    This happens in parallel with step 4.
+  // 6) Ensure default shared path exists.
+  // 7) Share paths with PluginVm
   chromeos::DBusThreadManager::Get()
       ->GetDebugDaemonClient()
       ->StartPluginVmDispatcher(
@@ -87,6 +93,9 @@ void PluginVmManager::StopPluginVm() {
 
   chromeos::DBusThreadManager::Get()->GetVmPluginDispatcherClient()->StopVm(
       std::move(request), base::DoNothing());
+
+  // Reset seneschal handle to indicate that it is no longer valid.
+  seneschal_server_handle_ = 0;
 }
 
 void PluginVmManager::OnStartPluginVmDispatcher(bool success) {
@@ -166,6 +175,15 @@ void PluginVmManager::ShowVm() {
   chromeos::DBusThreadManager::Get()->GetVmPluginDispatcherClient()->ShowVm(
       std::move(request), base::BindOnce(&PluginVmManager::OnShowVm,
                                          weak_ptr_factory_.GetWeakPtr()));
+
+  vm_tools::concierge::GetVmInfoRequest concierge_request;
+  concierge_request.set_owner_id(owner_id_);
+  concierge_request.set_name(kPluginVmName);
+
+  chromeos::DBusThreadManager::Get()->GetConciergeClient()->GetVmInfo(
+      std::move(concierge_request),
+      base::BindOnce(&PluginVmManager::OnGetVmInfo,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void PluginVmManager::OnShowVm(
@@ -180,4 +198,38 @@ void PluginVmManager::OnShowVm(
   RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kSuccess);
 }
 
+void PluginVmManager::OnGetVmInfo(
+    base::Optional<vm_tools::concierge::GetVmInfoResponse> reply) {
+  if (!reply.has_value()) {
+    LOG(ERROR) << "Failed to get concierge VM info.";
+    return;
+  }
+  if (!reply->success()) {
+    LOG(ERROR) << "VM not started, cannot share paths";
+    return;
+  }
+  seneschal_server_handle_ = reply->vm_info().seneschal_server_handle();
+
+  // Create and share default folder, and other persisted shares.
+  EnsureDefaultSharedDirExists(
+      profile_, base::BindOnce(&PluginVmManager::OnDefaultSharedDirExists,
+                               weak_ptr_factory_.GetWeakPtr()));
+  crostini::CrostiniSharePath::GetForProfile(profile_)->SharePersistedPaths(
+      kPluginVmName, base::DoNothing());
+}
+
+void PluginVmManager::OnDefaultSharedDirExists(const base::FilePath& dir,
+                                               bool exists) {
+  if (exists) {
+    crostini::CrostiniSharePath::GetForProfile(profile_)->SharePath(
+        kPluginVmName, dir, false,
+        base::BindOnce([](const base::FilePath& dir, bool success,
+                          std::string failure_reason) {
+          if (!success) {
+            LOG(ERROR) << "Error sharing PluginVm default dir " << dir.value()
+                       << ": " << failure_reason;
+          }
+        }));
+  }
+}
 }  // namespace plugin_vm

chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h

@@ -7,8 +7,10 @@
 
 #include <string>
 
+#include "base/files/file_path.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "chrome/browser/chromeos/crostini/crostini_manager.h"
 #include "chromeos/dbus/vm_plugin_dispatcher/vm_plugin_dispatcher.pb.h"
 #include "components/keyed_service/core/keyed_service.h"
 
@@ -28,6 +30,9 @@ class PluginVmManager : public KeyedService {
   void LaunchPluginVm();
   void StopPluginVm();
 
+  // Seneschal server handle to use for path sharing.
+  uint64_t seneschal_server_handle() { return seneschal_server_handle_; }
+
  private:
   // The flow to launch a Plugin Vm. We'll probably want to add additional
   // abstraction around starting the services in the future but this is
@@ -40,9 +45,13 @@ class PluginVmManager : public KeyedService {
   void ShowVm();
   void OnShowVm(
       base::Optional<vm_tools::plugin_dispatcher::ShowVmResponse> reply);
+  void OnGetVmInfo(
+      base::Optional<vm_tools::concierge::GetVmInfoResponse> reply);
+  void OnDefaultSharedDirExists(const base::FilePath& dir, bool exists);
 
   Profile* profile_;
   std::string owner_id_;
+  uint64_t seneschal_server_handle_ = 0;
 
   base::WeakPtrFactory<PluginVmManager> weak_ptr_factory_;
 

chrome/browser/chromeos/plugin_vm/plugin_vm_manager_unittest.cc

@@ -4,12 +4,16 @@
 
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h"
 
+#include "base/files/file_util.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_metrics_util.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_test_helper.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_concierge_client.h"
+#include "chromeos/dbus/fake_seneschal_client.h"
 #include "chromeos/dbus/fake_vm_plugin_dispatcher_client.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -30,6 +34,14 @@ class PluginVmManagerTest : public testing::Test {
     return *static_cast<chromeos::FakeVmPluginDispatcherClient*>(
         chromeos::DBusThreadManager::Get()->GetVmPluginDispatcherClient());
   }
+  chromeos::FakeConciergeClient& ConciergeClient() {
+    return *static_cast<chromeos::FakeConciergeClient*>(
+        chromeos::DBusThreadManager::Get()->GetConciergeClient());
+  }
+  chromeos::FakeSeneschalClient& SeneschalClient() {
+    return *static_cast<chromeos::FakeSeneschalClient*>(
+        chromeos::DBusThreadManager::Get()->GetSeneschalClient());
+  }
 
   void SetUp() override {
     histogram_tester_ = std::make_unique<base::HistogramTester>();
@@ -48,10 +60,13 @@ class PluginVmManagerTest : public testing::Test {
 TEST_F(PluginVmManagerTest, LaunchPluginVmRequiresPluginVmAllowed) {
   EXPECT_FALSE(IsPluginVmAllowedForProfile(&testing_profile_));
   plugin_vm_manager_.LaunchPluginVm();
-  base::RunLoop().RunUntilIdle();
+  thread_bundle_.RunUntilIdle();
   EXPECT_FALSE(VmPluginDispatcherClient().list_vms_called());
   EXPECT_FALSE(VmPluginDispatcherClient().start_vm_called());
   EXPECT_FALSE(VmPluginDispatcherClient().show_vm_called());
+  EXPECT_FALSE(ConciergeClient().get_vm_info_called());
+  EXPECT_FALSE(SeneschalClient().share_path_called());
+  EXPECT_EQ(plugin_vm_manager_.seneschal_server_handle(), 0ul);
 
   histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram,
                                         PluginVmLaunchResult::kError, 1);
@@ -68,16 +83,21 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmStartAndShow) {
   VmPluginDispatcherClient().set_list_vms_response(list_vms_response);
 
   plugin_vm_manager_.LaunchPluginVm();
-  base::RunLoop().RunUntilIdle();
+  thread_bundle_.RunUntilIdle();
   EXPECT_TRUE(VmPluginDispatcherClient().list_vms_called());
   EXPECT_TRUE(VmPluginDispatcherClient().start_vm_called());
   EXPECT_TRUE(VmPluginDispatcherClient().show_vm_called());
+  EXPECT_TRUE(ConciergeClient().get_vm_info_called());
+  EXPECT_TRUE(base::DirectoryExists(
+      file_manager::util::GetMyFilesFolderForProfile(&testing_profile_)));
+  EXPECT_TRUE(SeneschalClient().share_path_called());
+  EXPECT_EQ(plugin_vm_manager_.seneschal_server_handle(), 1ul);
 
   histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram,
                                         PluginVmLaunchResult::kSuccess, 1);
 }
 
-TEST_F(PluginVmManagerTest, LaunchPluginVmShow) {
+TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) {
   test_helper_.AllowPluginVm();
   EXPECT_TRUE(IsPluginVmAllowedForProfile(&testing_profile_));
 
@@ -88,10 +108,21 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmShow) {
   VmPluginDispatcherClient().set_list_vms_response(list_vms_response);
 
   plugin_vm_manager_.LaunchPluginVm();
-  base::RunLoop().RunUntilIdle();
+  thread_bundle_.RunUntilIdle();
   EXPECT_TRUE(VmPluginDispatcherClient().list_vms_called());
   EXPECT_FALSE(VmPluginDispatcherClient().start_vm_called());
   EXPECT_TRUE(VmPluginDispatcherClient().show_vm_called());
+  EXPECT_FALSE(VmPluginDispatcherClient().stop_vm_called());
+  EXPECT_TRUE(ConciergeClient().get_vm_info_called());
+  EXPECT_TRUE(base::DirectoryExists(
+      file_manager::util::GetMyFilesFolderForProfile(&testing_profile_)));
+  EXPECT_TRUE(SeneschalClient().share_path_called());
+  EXPECT_EQ(plugin_vm_manager_.seneschal_server_handle(), 1ul);
+
+  plugin_vm_manager_.StopPluginVm();
+  thread_bundle_.RunUntilIdle();
+  EXPECT_TRUE(VmPluginDispatcherClient().stop_vm_called());
+  EXPECT_EQ(plugin_vm_manager_.seneschal_server_handle(), 0ul);
 
   histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram,
                                         PluginVmLaunchResult::kSuccess, 1);