Show a spinner for the first launch of Plugin VM

As Plugin VM can take a few seconds to start up, we should show a
spinner similar to ARC and Crostini. Subsequent launches in a session
tend to be slightly faster so we only show a spinner for the first
launch. A subsequent CL will make us also show a spinner if we have to
wait for the VM to finish shutting down or suspending before starting
it (which isn't yet implemented).

Bug: 940319
Change-Id: I9ef4078e21f15e4c926a2c23860de14095679f7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1634294Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665060}

chrome/browser/chromeos/plugin_vm/plugin_vm_manager.cc

@@ -10,6 +10,9 @@
 #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"
+#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
+#include "chrome/browser/ui/ash/launcher/shelf_spinner_controller.h"
+#include "chrome/browser/ui/ash/launcher/shelf_spinner_item_controller.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/debug_daemon_client.h"
 #include "chromeos/dbus/vm_plugin_dispatcher_client.h"
@@ -66,10 +69,22 @@ PluginVmManager::~PluginVmManager() {}
 void PluginVmManager::LaunchPluginVm() {
   if (!IsPluginVmAllowedForProfile(profile_)) {
     LOG(ERROR) << "Attempted to launch PluginVm when it is not allowed";
-    RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kError);
+    LaunchFailed();
     return;
   }
 
+  if (!vm_has_been_launched_) {
+    ChromeLauncherController* chrome_controller =
+        ChromeLauncherController::instance();
+    // Can be null in tests.
+    if (chrome_controller) {
+      chrome_controller->GetShelfSpinnerController()->AddSpinnerToShelf(
+          kPluginVmAppId,
+          std::make_unique<ShelfSpinnerItemController>(kPluginVmAppId));
+    }
+    vm_has_been_launched_ = true;
+  }
+
   // Launching Plugin Vm goes through the following steps:
   // 1) Start the Plugin Vm Dispatcher
   // 2) Call ListVms to get the state of the VM
@@ -101,7 +116,7 @@ void PluginVmManager::StopPluginVm() {
 void PluginVmManager::OnStartPluginVmDispatcher(bool success) {
   if (!success) {
     LOG(ERROR) << "Failed to start Plugin Vm Dispatcher.";
-    RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kError);
+    LaunchFailed();
     return;
   }
 
@@ -118,12 +133,12 @@ void PluginVmManager::OnListVms(
     base::Optional<vm_tools::plugin_dispatcher::ListVmResponse> reply) {
   if (!reply.has_value() || reply->error()) {
     LOG(ERROR) << "Failed to list VMs.";
-    RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kError);
+    LaunchFailed();
     return;
   }
   if (reply->vm_info_size() != 1) {
     LOG(ERROR) << "Default VM is missing.";
-    RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kError);
+    LaunchFailed();
     return;
   }
 
@@ -148,10 +163,10 @@ void PluginVmManager::OnListVms(
       break;
     }
     default:
-      // TODO(timloh): We need some way to handle states like stopping, perhaps
-      // we should retry after a few seconds?
+      // TODO(timloh): Handle states like stopping by waiting for a signal of
+      // completion, and show a spinner while we wait.
       LOG(ERROR) << "Didn't start VM as it is in state " << vm_state;
-      RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kError);
+      LaunchFailed();
       break;
   }
 }
@@ -160,7 +175,7 @@ void PluginVmManager::OnStartVm(
     base::Optional<vm_tools::plugin_dispatcher::StartVmResponse> reply) {
   if (!reply.has_value() || reply->error()) {
     LOG(ERROR) << "Failed to start VM.";
-    RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kError);
+    LaunchFailed();
     return;
   }
 
@@ -190,7 +205,7 @@ void PluginVmManager::OnShowVm(
     base::Optional<vm_tools::plugin_dispatcher::ShowVmResponse> reply) {
   if (!reply.has_value() || reply->error()) {
     LOG(ERROR) << "Failed to show VM.";
-    RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kError);
+    LaunchFailed();
     return;
   }
 
@@ -232,4 +247,16 @@ void PluginVmManager::OnDefaultSharedDirExists(const base::FilePath& dir,
         }));
   }
 }
+
+void PluginVmManager::LaunchFailed() {
+  RecordPluginVmLaunchResultHistogram(PluginVmLaunchResult::kError);
+
+  ChromeLauncherController* chrome_controller =
+      ChromeLauncherController::instance();
+  if (chrome_controller) {
+    chrome_controller->GetShelfSpinnerController()->CloseSpinner(
+        kPluginVmAppId);
+  }
+}
+
 }  // namespace plugin_vm

chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h

@@ -49,10 +49,16 @@ class PluginVmManager : public KeyedService {
       base::Optional<vm_tools::concierge::GetVmInfoResponse> reply);
   void OnDefaultSharedDirExists(const base::FilePath& dir, bool exists);
 
+  // Called when LaunchPluginVm() is unsuccessful.
+  void LaunchFailed();
+
   Profile* profile_;
   std::string owner_id_;
   uint64_t seneschal_server_handle_ = 0;
 
+  // We display a spinner on the first launch as it tends to take a bit longer.
+  bool vm_has_been_launched_ = false;
+
   base::WeakPtrFactory<PluginVmManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PluginVmManager);

chrome/browser/chromeos/plugin_vm/plugin_vm_manager_unittest.cc

@@ -4,12 +4,15 @@
 
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h"
 
+#include "ash/public/cpp/shelf_model.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/browser/ui/ash/launcher/chrome_launcher_controller.h"
+#include "chrome/browser/ui/ash/launcher/shelf_spinner_controller.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/fake_concierge_client.h"
@@ -128,4 +131,38 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) {
                                         PluginVmLaunchResult::kSuccess, 1);
 }
 
+TEST_F(PluginVmManagerTest, LaunchPluginVmSpinner) {
+  ash::ShelfModel shelf_model;
+  ChromeLauncherController chrome_launcher_controller(&testing_profile_,
+                                                      &shelf_model);
+  ShelfSpinnerController* spinner_controller =
+      chrome_launcher_controller.GetShelfSpinnerController();
+
+  test_helper_.AllowPluginVm();
+  EXPECT_TRUE(IsPluginVmAllowedForProfile(&testing_profile_));
+
+  // No spinner before doing anything
+  EXPECT_FALSE(spinner_controller->HasApp(kPluginVmAppId));
+
+  vm_tools::plugin_dispatcher::ListVmResponse list_vms_response;
+  list_vms_response.add_vm_info()->set_state(
+      vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPED);
+  VmPluginDispatcherClient().set_list_vms_response(list_vms_response);
+
+  plugin_vm_manager_.LaunchPluginVm();
+  base::RunLoop().RunUntilIdle();
+
+  // Spinner exists for first launch.
+  EXPECT_TRUE(spinner_controller->HasApp(kPluginVmAppId));
+  // Under normal operation, the Plugin VM window would appear and close the
+  // spinner. Since the ShowVm call doesn't actually do this, manually close
+  // the spinner.
+  spinner_controller->CloseSpinner(kPluginVmAppId);
+
+  plugin_vm_manager_.LaunchPluginVm();
+  base::RunLoop().RunUntilIdle();
+  // A second launch shouldn't show a spinner.
+  EXPECT_FALSE(spinner_controller->HasApp(kPluginVmAppId));
+}
+
 }  // namespace plugin_vm