Commit a7d92c73 authored by Timothy Loh's avatar Timothy Loh Committed by Commit Bot

Wire up signal for plugin vm state changes

This CL wires up the signal for plugin vm state changes and uses it to
improve the launching experience. If we are in a suspending, stopping,
resetting, or pausing state, we now wait for the operation to complete
before starting the vm rather than simply failing to launch the vm. In
these case we also show a spinner in the shelf.

As we can now keep track of the state of the vm, we also update the
shelf and launcher context menu to not show the Shut Down option when
the VM is not actually running.

Bug: 940319
Change-Id: Ic0141275fce0ec792d6aa809ebb50d2f944be6c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1640777
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: default avatarRyo Hashimoto <hashimoto@chromium.org>
Reviewed-by: default avatarJoel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665785}
parent f4e9d749
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "chrome/browser/ui/ash/launcher/shelf_spinner_item_controller.h" #include "chrome/browser/ui/ash/launcher/shelf_spinner_item_controller.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/debug_daemon_client.h" #include "chromeos/dbus/debug_daemon_client.h"
#include "chromeos/dbus/vm_plugin_dispatcher_client.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h" #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
...@@ -53,6 +52,14 @@ class PluginVmManagerFactory : public BrowserContextKeyedServiceFactory { ...@@ -53,6 +52,14 @@ class PluginVmManagerFactory : public BrowserContextKeyedServiceFactory {
} }
}; };
// Checks if the VM is in a state in which we can't immediately start it.
bool VmIsStopping(vm_tools::plugin_dispatcher::VmState state) {
return state == vm_tools::plugin_dispatcher::VmState::VM_STATE_SUSPENDING ||
state == vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPING ||
state == vm_tools::plugin_dispatcher::VmState::VM_STATE_RESETTING ||
state == vm_tools::plugin_dispatcher::VmState::VM_STATE_PAUSING;
}
} // namespace } // namespace
PluginVmManager* PluginVmManager::GetForProfile(Profile* profile) { PluginVmManager* PluginVmManager::GetForProfile(Profile* profile) {
...@@ -62,9 +69,17 @@ PluginVmManager* PluginVmManager::GetForProfile(Profile* profile) { ...@@ -62,9 +69,17 @@ PluginVmManager* PluginVmManager::GetForProfile(Profile* profile) {
PluginVmManager::PluginVmManager(Profile* profile) PluginVmManager::PluginVmManager(Profile* profile)
: profile_(profile), : profile_(profile),
owner_id_(chromeos::ProfileHelper::GetUserIdHashFromProfile(profile)), owner_id_(chromeos::ProfileHelper::GetUserIdHashFromProfile(profile)),
weak_ptr_factory_(this) {} weak_ptr_factory_(this) {
chromeos::DBusThreadManager::Get()
->GetVmPluginDispatcherClient()
->AddObserver(this);
}
PluginVmManager::~PluginVmManager() {} PluginVmManager::~PluginVmManager() {
chromeos::DBusThreadManager::Get()
->GetVmPluginDispatcherClient()
->RemoveObserver(this);
}
void PluginVmManager::LaunchPluginVm() { void PluginVmManager::LaunchPluginVm() {
if (!IsPluginVmAllowedForProfile(profile_)) { if (!IsPluginVmAllowedForProfile(profile_)) {
...@@ -73,7 +88,10 @@ void PluginVmManager::LaunchPluginVm() { ...@@ -73,7 +88,10 @@ void PluginVmManager::LaunchPluginVm() {
return; return;
} }
if (!vm_has_been_launched_) { // Show a spinner for the first launch (state UNKNOWN) or if we will have to
// wait before starting the VM.
if (vm_state_ == vm_tools::plugin_dispatcher::VmState::VM_STATE_UNKNOWN ||
VmIsStopping(vm_state_)) {
ChromeLauncherController* chrome_controller = ChromeLauncherController* chrome_controller =
ChromeLauncherController::instance(); ChromeLauncherController::instance();
// Can be null in tests. // Can be null in tests.
...@@ -82,11 +100,10 @@ void PluginVmManager::LaunchPluginVm() { ...@@ -82,11 +100,10 @@ void PluginVmManager::LaunchPluginVm() {
kPluginVmAppId, kPluginVmAppId,
std::make_unique<ShelfSpinnerItemController>(kPluginVmAppId)); std::make_unique<ShelfSpinnerItemController>(kPluginVmAppId));
} }
vm_has_been_launched_ = true;
} }
// Launching Plugin Vm goes through the following steps: // Launching Plugin Vm goes through the following steps:
// 1) Start the Plugin Vm Dispatcher // 1) Start the Plugin Vm Dispatcher (no-op if already running)
// 2) Call ListVms to get the state of the VM // 2) Call ListVms to get the state of the VM
// 3) Start the VM if necessary // 3) Start the VM if necessary
// 4) Show the UI. // 4) Show the UI.
...@@ -113,6 +130,17 @@ void PluginVmManager::StopPluginVm() { ...@@ -113,6 +130,17 @@ void PluginVmManager::StopPluginVm() {
seneschal_server_handle_ = 0; seneschal_server_handle_ = 0;
} }
void PluginVmManager::OnVmStateChanged(
const vm_tools::plugin_dispatcher::VmStateChangedSignal& signal) {
if (signal.owner_id() != owner_id_ || signal.vm_name() != kPluginVmName)
return;
vm_state_ = signal.vm_state();
if (pending_start_vm_ && !VmIsStopping(vm_state_))
StartVm();
}
void PluginVmManager::OnStartPluginVmDispatcher(bool success) { void PluginVmManager::OnStartPluginVmDispatcher(bool success) {
if (!success) { if (!success) {
LOG(ERROR) << "Failed to start Plugin Vm Dispatcher."; LOG(ERROR) << "Failed to start Plugin Vm Dispatcher.";
...@@ -142,35 +170,48 @@ void PluginVmManager::OnListVms( ...@@ -142,35 +170,48 @@ void PluginVmManager::OnListVms(
return; return;
} }
auto vm_state = reply->vm_info(0).state(); // This is kept up to date in OnVmStateChanged, but the state will not yet be
switch (vm_state) { // set if we just started the dispatcher.
vm_state_ = reply->vm_info(0).state();
switch (vm_state_) {
case vm_tools::plugin_dispatcher::VmState::VM_STATE_SUSPENDING:
case vm_tools::plugin_dispatcher::VmState::VM_STATE_RESETTING:
case vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPING:
case vm_tools::plugin_dispatcher::VmState::VM_STATE_PAUSING:
pending_start_vm_ = true;
break;
case vm_tools::plugin_dispatcher::VmState::VM_STATE_STARTING: case vm_tools::plugin_dispatcher::VmState::VM_STATE_STARTING:
case vm_tools::plugin_dispatcher::VmState::VM_STATE_RUNNING: case vm_tools::plugin_dispatcher::VmState::VM_STATE_RUNNING:
case vm_tools::plugin_dispatcher::VmState::VM_STATE_CONTINUING:
case vm_tools::plugin_dispatcher::VmState::VM_STATE_RESUMING:
ShowVm(); ShowVm();
break; break;
case vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPED: case vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPED:
case vm_tools::plugin_dispatcher::VmState::VM_STATE_PAUSED: case vm_tools::plugin_dispatcher::VmState::VM_STATE_PAUSED:
case vm_tools::plugin_dispatcher::VmState::VM_STATE_SUSPENDED: { case vm_tools::plugin_dispatcher::VmState::VM_STATE_SUSPENDED:
vm_tools::plugin_dispatcher::StartVmRequest request; StartVm();
request.set_owner_id(owner_id_);
request.set_vm_name_uuid(kPluginVmName);
chromeos::DBusThreadManager::Get()
->GetVmPluginDispatcherClient()
->StartVm(std::move(request),
base::BindOnce(&PluginVmManager::OnStartVm,
weak_ptr_factory_.GetWeakPtr()));
break; break;
}
default: default:
// TODO(timloh): Handle states like stopping by waiting for a signal of LOG(ERROR) << "Didn't start VM as it is in unexpected state "
// completion, and show a spinner while we wait. << vm_state_;
LOG(ERROR) << "Didn't start VM as it is in state " << vm_state;
LaunchFailed(); LaunchFailed();
break; break;
} }
} }
void PluginVmManager::StartVm() {
pending_start_vm_ = false;
vm_tools::plugin_dispatcher::StartVmRequest request;
request.set_owner_id(owner_id_);
request.set_vm_name_uuid(kPluginVmName);
chromeos::DBusThreadManager::Get()->GetVmPluginDispatcherClient()->StartVm(
std::move(request), base::BindOnce(&PluginVmManager::OnStartVm,
weak_ptr_factory_.GetWeakPtr()));
}
void PluginVmManager::OnStartVm( void PluginVmManager::OnStartVm(
base::Optional<vm_tools::plugin_dispatcher::StartVmResponse> reply) { base::Optional<vm_tools::plugin_dispatcher::StartVmResponse> reply) {
if (!reply.has_value() || reply->error()) { if (!reply.has_value() || reply->error()) {
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/optional.h" #include "base/optional.h"
#include "chrome/browser/chromeos/crostini/crostini_manager.h" #include "chrome/browser/chromeos/crostini/crostini_manager.h"
#include "chromeos/dbus/vm_plugin_dispatcher/vm_plugin_dispatcher.pb.h" #include "chromeos/dbus/vm_plugin_dispatcher/vm_plugin_dispatcher.pb.h"
#include "chromeos/dbus/vm_plugin_dispatcher_client.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
class Profile; class Profile;
...@@ -20,7 +21,8 @@ namespace plugin_vm { ...@@ -20,7 +21,8 @@ namespace plugin_vm {
// The PluginVmManager is responsible for connecting to the D-Bus services to // The PluginVmManager is responsible for connecting to the D-Bus services to
// manage the Plugin Vm. // manage the Plugin Vm.
class PluginVmManager : public KeyedService { class PluginVmManager : public KeyedService,
public chromeos::VmPluginDispatcherClient::Observer {
public: public:
static PluginVmManager* GetForProfile(Profile* profile); static PluginVmManager* GetForProfile(Profile* profile);
...@@ -33,6 +35,12 @@ class PluginVmManager : public KeyedService { ...@@ -33,6 +35,12 @@ class PluginVmManager : public KeyedService {
// Seneschal server handle to use for path sharing. // Seneschal server handle to use for path sharing.
uint64_t seneschal_server_handle() { return seneschal_server_handle_; } uint64_t seneschal_server_handle() { return seneschal_server_handle_; }
// chromeos::VmPluginDispatcherClient::Observer:
void OnVmStateChanged(
const vm_tools::plugin_dispatcher::VmStateChangedSignal& signal) override;
vm_tools::plugin_dispatcher::VmState vm_state() const { return vm_state_; }
private: private:
// The flow to launch a Plugin Vm. We'll probably want to add additional // 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 // abstraction around starting the services in the future but this is
...@@ -40,6 +48,7 @@ class PluginVmManager : public KeyedService { ...@@ -40,6 +48,7 @@ class PluginVmManager : public KeyedService {
void OnStartPluginVmDispatcher(bool success); void OnStartPluginVmDispatcher(bool success);
void OnListVms( void OnListVms(
base::Optional<vm_tools::plugin_dispatcher::ListVmResponse> reply); base::Optional<vm_tools::plugin_dispatcher::ListVmResponse> reply);
void StartVm();
void OnStartVm( void OnStartVm(
base::Optional<vm_tools::plugin_dispatcher::StartVmResponse> reply); base::Optional<vm_tools::plugin_dispatcher::StartVmResponse> reply);
void ShowVm(); void ShowVm();
...@@ -56,8 +65,13 @@ class PluginVmManager : public KeyedService { ...@@ -56,8 +65,13 @@ class PluginVmManager : public KeyedService {
std::string owner_id_; std::string owner_id_;
uint64_t seneschal_server_handle_ = 0; uint64_t seneschal_server_handle_ = 0;
// We display a spinner on the first launch as it tends to take a bit longer. // State of the default VM, kept up-to-date by signals from the dispatcher.
bool vm_has_been_launched_ = false; vm_tools::plugin_dispatcher::VmState vm_state_ =
vm_tools::plugin_dispatcher::VmState::VM_STATE_UNKNOWN;
// We can't immediately start the VM when it is in states like suspending, so
// delay until an in progress operation finishes.
bool pending_start_vm_ = false;
base::WeakPtrFactory<PluginVmManager> weak_ptr_factory_; base::WeakPtrFactory<PluginVmManager> weak_ptr_factory_;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "chrome/browser/chromeos/plugin_vm/plugin_vm_metrics_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_test_helper.h"
#include "chrome/browser/chromeos/plugin_vm/plugin_vm_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/chrome_launcher_controller.h"
#include "chrome/browser/ui/ash/launcher/shelf_spinner_controller.h" #include "chrome/browser/ui/ash/launcher/shelf_spinner_controller.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -25,12 +26,20 @@ namespace plugin_vm { ...@@ -25,12 +26,20 @@ namespace plugin_vm {
class PluginVmManagerTest : public testing::Test { class PluginVmManagerTest : public testing::Test {
public: public:
PluginVmManagerTest() PluginVmManagerTest() {
: test_helper_(&testing_profile_), plugin_vm_manager_(&testing_profile_) {
chromeos::DBusThreadManager::Initialize(); chromeos::DBusThreadManager::Initialize();
testing_profile_ = std::make_unique<TestingProfile>();
test_helper_ = std::make_unique<PluginVmTestHelper>(testing_profile_.get());
plugin_vm_manager_ = PluginVmManager::GetForProfile(testing_profile_.get());
histogram_tester_ = std::make_unique<base::HistogramTester>();
} }
~PluginVmManagerTest() override { chromeos::DBusThreadManager::Shutdown(); } ~PluginVmManagerTest() override {
histogram_tester_.reset();
test_helper_.reset();
testing_profile_.reset();
chromeos::DBusThreadManager::Shutdown();
}
protected: protected:
chromeos::FakeVmPluginDispatcherClient& VmPluginDispatcherClient() { chromeos::FakeVmPluginDispatcherClient& VmPluginDispatcherClient() {
...@@ -46,14 +55,10 @@ class PluginVmManagerTest : public testing::Test { ...@@ -46,14 +55,10 @@ class PluginVmManagerTest : public testing::Test {
chromeos::DBusThreadManager::Get()->GetSeneschalClient()); chromeos::DBusThreadManager::Get()->GetSeneschalClient());
} }
void SetUp() override {
histogram_tester_ = std::make_unique<base::HistogramTester>();
}
content::TestBrowserThreadBundle thread_bundle_; content::TestBrowserThreadBundle thread_bundle_;
TestingProfile testing_profile_; std::unique_ptr<TestingProfile> testing_profile_;
PluginVmTestHelper test_helper_; std::unique_ptr<PluginVmTestHelper> test_helper_;
PluginVmManager plugin_vm_manager_; PluginVmManager* plugin_vm_manager_;
std::unique_ptr<base::HistogramTester> histogram_tester_; std::unique_ptr<base::HistogramTester> histogram_tester_;
private: private:
...@@ -61,23 +66,23 @@ class PluginVmManagerTest : public testing::Test { ...@@ -61,23 +66,23 @@ class PluginVmManagerTest : public testing::Test {
}; };
TEST_F(PluginVmManagerTest, LaunchPluginVmRequiresPluginVmAllowed) { TEST_F(PluginVmManagerTest, LaunchPluginVmRequiresPluginVmAllowed) {
EXPECT_FALSE(IsPluginVmAllowedForProfile(&testing_profile_)); EXPECT_FALSE(IsPluginVmAllowedForProfile(testing_profile_.get()));
plugin_vm_manager_.LaunchPluginVm(); plugin_vm_manager_->LaunchPluginVm();
thread_bundle_.RunUntilIdle(); thread_bundle_.RunUntilIdle();
EXPECT_FALSE(VmPluginDispatcherClient().list_vms_called()); EXPECT_FALSE(VmPluginDispatcherClient().list_vms_called());
EXPECT_FALSE(VmPluginDispatcherClient().start_vm_called()); EXPECT_FALSE(VmPluginDispatcherClient().start_vm_called());
EXPECT_FALSE(VmPluginDispatcherClient().show_vm_called()); EXPECT_FALSE(VmPluginDispatcherClient().show_vm_called());
EXPECT_FALSE(ConciergeClient().get_vm_info_called()); EXPECT_FALSE(ConciergeClient().get_vm_info_called());
EXPECT_FALSE(SeneschalClient().share_path_called()); EXPECT_FALSE(SeneschalClient().share_path_called());
EXPECT_EQ(plugin_vm_manager_.seneschal_server_handle(), 0ul); EXPECT_EQ(plugin_vm_manager_->seneschal_server_handle(), 0ul);
histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram, histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram,
PluginVmLaunchResult::kError, 1); PluginVmLaunchResult::kError, 1);
} }
TEST_F(PluginVmManagerTest, LaunchPluginVmStartAndShow) { TEST_F(PluginVmManagerTest, LaunchPluginVmStartAndShow) {
test_helper_.AllowPluginVm(); test_helper_->AllowPluginVm();
EXPECT_TRUE(IsPluginVmAllowedForProfile(&testing_profile_)); EXPECT_TRUE(IsPluginVmAllowedForProfile(testing_profile_.get()));
// The PluginVmManager calls StartVm when the VM is not yet running. // The PluginVmManager calls StartVm when the VM is not yet running.
vm_tools::plugin_dispatcher::ListVmResponse list_vms_response; vm_tools::plugin_dispatcher::ListVmResponse list_vms_response;
...@@ -85,24 +90,24 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmStartAndShow) { ...@@ -85,24 +90,24 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmStartAndShow) {
vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPED); vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPED);
VmPluginDispatcherClient().set_list_vms_response(list_vms_response); VmPluginDispatcherClient().set_list_vms_response(list_vms_response);
plugin_vm_manager_.LaunchPluginVm(); plugin_vm_manager_->LaunchPluginVm();
thread_bundle_.RunUntilIdle(); thread_bundle_.RunUntilIdle();
EXPECT_TRUE(VmPluginDispatcherClient().list_vms_called()); EXPECT_TRUE(VmPluginDispatcherClient().list_vms_called());
EXPECT_TRUE(VmPluginDispatcherClient().start_vm_called()); EXPECT_TRUE(VmPluginDispatcherClient().start_vm_called());
EXPECT_TRUE(VmPluginDispatcherClient().show_vm_called()); EXPECT_TRUE(VmPluginDispatcherClient().show_vm_called());
EXPECT_TRUE(ConciergeClient().get_vm_info_called()); EXPECT_TRUE(ConciergeClient().get_vm_info_called());
EXPECT_TRUE(base::DirectoryExists( EXPECT_TRUE(base::DirectoryExists(
file_manager::util::GetMyFilesFolderForProfile(&testing_profile_))); file_manager::util::GetMyFilesFolderForProfile(testing_profile_.get())));
EXPECT_TRUE(SeneschalClient().share_path_called()); EXPECT_TRUE(SeneschalClient().share_path_called());
EXPECT_EQ(plugin_vm_manager_.seneschal_server_handle(), 1ul); EXPECT_EQ(plugin_vm_manager_->seneschal_server_handle(), 1ul);
histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram, histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram,
PluginVmLaunchResult::kSuccess, 1); PluginVmLaunchResult::kSuccess, 1);
} }
TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) { TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) {
test_helper_.AllowPluginVm(); test_helper_->AllowPluginVm();
EXPECT_TRUE(IsPluginVmAllowedForProfile(&testing_profile_)); EXPECT_TRUE(IsPluginVmAllowedForProfile(testing_profile_.get()));
// The PluginVmManager skips calling StartVm when the VM is already running. // The PluginVmManager skips calling StartVm when the VM is already running.
vm_tools::plugin_dispatcher::ListVmResponse list_vms_response; vm_tools::plugin_dispatcher::ListVmResponse list_vms_response;
...@@ -110,7 +115,7 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) { ...@@ -110,7 +115,7 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) {
vm_tools::plugin_dispatcher::VmState::VM_STATE_RUNNING); vm_tools::plugin_dispatcher::VmState::VM_STATE_RUNNING);
VmPluginDispatcherClient().set_list_vms_response(list_vms_response); VmPluginDispatcherClient().set_list_vms_response(list_vms_response);
plugin_vm_manager_.LaunchPluginVm(); plugin_vm_manager_->LaunchPluginVm();
thread_bundle_.RunUntilIdle(); thread_bundle_.RunUntilIdle();
EXPECT_TRUE(VmPluginDispatcherClient().list_vms_called()); EXPECT_TRUE(VmPluginDispatcherClient().list_vms_called());
EXPECT_FALSE(VmPluginDispatcherClient().start_vm_called()); EXPECT_FALSE(VmPluginDispatcherClient().start_vm_called());
...@@ -118,14 +123,14 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) { ...@@ -118,14 +123,14 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) {
EXPECT_FALSE(VmPluginDispatcherClient().stop_vm_called()); EXPECT_FALSE(VmPluginDispatcherClient().stop_vm_called());
EXPECT_TRUE(ConciergeClient().get_vm_info_called()); EXPECT_TRUE(ConciergeClient().get_vm_info_called());
EXPECT_TRUE(base::DirectoryExists( EXPECT_TRUE(base::DirectoryExists(
file_manager::util::GetMyFilesFolderForProfile(&testing_profile_))); file_manager::util::GetMyFilesFolderForProfile(testing_profile_.get())));
EXPECT_TRUE(SeneschalClient().share_path_called()); EXPECT_TRUE(SeneschalClient().share_path_called());
EXPECT_EQ(plugin_vm_manager_.seneschal_server_handle(), 1ul); EXPECT_EQ(plugin_vm_manager_->seneschal_server_handle(), 1ul);
plugin_vm_manager_.StopPluginVm(); plugin_vm_manager_->StopPluginVm();
thread_bundle_.RunUntilIdle(); thread_bundle_.RunUntilIdle();
EXPECT_TRUE(VmPluginDispatcherClient().stop_vm_called()); EXPECT_TRUE(VmPluginDispatcherClient().stop_vm_called());
EXPECT_EQ(plugin_vm_manager_.seneschal_server_handle(), 0ul); EXPECT_EQ(plugin_vm_manager_->seneschal_server_handle(), 0ul);
histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram, histogram_tester_->ExpectUniqueSample(kPluginVmLaunchResultHistogram,
PluginVmLaunchResult::kSuccess, 1); PluginVmLaunchResult::kSuccess, 1);
...@@ -133,13 +138,13 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) { ...@@ -133,13 +138,13 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmShowAndStop) {
TEST_F(PluginVmManagerTest, LaunchPluginVmSpinner) { TEST_F(PluginVmManagerTest, LaunchPluginVmSpinner) {
ash::ShelfModel shelf_model; ash::ShelfModel shelf_model;
ChromeLauncherController chrome_launcher_controller(&testing_profile_, ChromeLauncherController chrome_launcher_controller(testing_profile_.get(),
&shelf_model); &shelf_model);
ShelfSpinnerController* spinner_controller = ShelfSpinnerController* spinner_controller =
chrome_launcher_controller.GetShelfSpinnerController(); chrome_launcher_controller.GetShelfSpinnerController();
test_helper_.AllowPluginVm(); test_helper_->AllowPluginVm();
EXPECT_TRUE(IsPluginVmAllowedForProfile(&testing_profile_)); EXPECT_TRUE(IsPluginVmAllowedForProfile(testing_profile_.get()));
// No spinner before doing anything // No spinner before doing anything
EXPECT_FALSE(spinner_controller->HasApp(kPluginVmAppId)); EXPECT_FALSE(spinner_controller->HasApp(kPluginVmAppId));
...@@ -149,8 +154,8 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmSpinner) { ...@@ -149,8 +154,8 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmSpinner) {
vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPED); vm_tools::plugin_dispatcher::VmState::VM_STATE_STOPPED);
VmPluginDispatcherClient().set_list_vms_response(list_vms_response); VmPluginDispatcherClient().set_list_vms_response(list_vms_response);
plugin_vm_manager_.LaunchPluginVm(); plugin_vm_manager_->LaunchPluginVm();
base::RunLoop().RunUntilIdle(); thread_bundle_.RunUntilIdle();
// Spinner exists for first launch. // Spinner exists for first launch.
EXPECT_TRUE(spinner_controller->HasApp(kPluginVmAppId)); EXPECT_TRUE(spinner_controller->HasApp(kPluginVmAppId));
...@@ -159,10 +164,52 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmSpinner) { ...@@ -159,10 +164,52 @@ TEST_F(PluginVmManagerTest, LaunchPluginVmSpinner) {
// the spinner. // the spinner.
spinner_controller->CloseSpinner(kPluginVmAppId); spinner_controller->CloseSpinner(kPluginVmAppId);
plugin_vm_manager_.LaunchPluginVm(); plugin_vm_manager_->LaunchPluginVm();
base::RunLoop().RunUntilIdle(); thread_bundle_.RunUntilIdle();
// A second launch shouldn't show a spinner. // A second launch shouldn't show a spinner.
EXPECT_FALSE(spinner_controller->HasApp(kPluginVmAppId)); EXPECT_FALSE(spinner_controller->HasApp(kPluginVmAppId));
} }
TEST_F(PluginVmManagerTest, LaunchPluginVmFromSuspending) {
// We cannot start a vm in states like SUSPENDING, so the StartVm call is
// delayed until an appropriate state change signal is received.
ash::ShelfModel shelf_model;
ChromeLauncherController chrome_launcher_controller(testing_profile_.get(),
&shelf_model);
ShelfSpinnerController* spinner_controller =
chrome_launcher_controller.GetShelfSpinnerController();
test_helper_->AllowPluginVm();
vm_tools::plugin_dispatcher::VmStateChangedSignal state_changed_signal;
state_changed_signal.set_owner_id(
chromeos::ProfileHelper::GetUserIdHashFromProfile(
testing_profile_.get()));
state_changed_signal.set_vm_name(kPluginVmName);
state_changed_signal.set_vm_state(
vm_tools::plugin_dispatcher::VmState::VM_STATE_SUSPENDING);
VmPluginDispatcherClient().NotifyVmStateChanged(state_changed_signal);
vm_tools::plugin_dispatcher::ListVmResponse list_vms_response;
list_vms_response.add_vm_info()->set_state(
vm_tools::plugin_dispatcher::VmState::VM_STATE_SUSPENDING);
VmPluginDispatcherClient().set_list_vms_response(list_vms_response);
plugin_vm_manager_->LaunchPluginVm();
thread_bundle_.RunUntilIdle();
EXPECT_TRUE(VmPluginDispatcherClient().list_vms_called());
EXPECT_FALSE(VmPluginDispatcherClient().start_vm_called());
EXPECT_FALSE(VmPluginDispatcherClient().show_vm_called());
EXPECT_TRUE(spinner_controller->HasApp(kPluginVmAppId));
// The launch process continues once the operation completes.
state_changed_signal.set_vm_state(
vm_tools::plugin_dispatcher::VmState::VM_STATE_SUSPENDED);
VmPluginDispatcherClient().NotifyVmStateChanged(state_changed_signal);
thread_bundle_.RunUntilIdle();
EXPECT_TRUE(VmPluginDispatcherClient().list_vms_called());
EXPECT_TRUE(VmPluginDispatcherClient().start_vm_called());
EXPECT_TRUE(VmPluginDispatcherClient().show_vm_called());
}
} // namespace plugin_vm } // namespace plugin_vm
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <utility> #include <utility>
#include "base/command_line.h" #include "base/command_line.h"
#include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h"
#include "chrome/browser/chromeos/plugin_vm/plugin_vm_pref_names.h" #include "chrome/browser/chromeos/plugin_vm/plugin_vm_pref_names.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/chromeos/settings/cros_settings.h" #include "chrome/browser/chromeos/settings/cros_settings.h"
...@@ -102,11 +103,10 @@ bool IsPluginVmEnabled(Profile* profile) { ...@@ -102,11 +103,10 @@ bool IsPluginVmEnabled(Profile* profile) {
} }
bool IsPluginVmRunning(Profile* profile) { bool IsPluginVmRunning(Profile* profile) {
// TODO(timloh): This should probably also check the state of the VM. Once we return plugin_vm::PluginVmManager::GetForProfile(profile)->vm_state() ==
// have a signal to update us on VM state changes, we should keep track of it vm_tools::plugin_dispatcher::VmState::VM_STATE_RUNNING &&
// for use here. ChromeLauncherController::instance()->IsOpen(
return ChromeLauncherController::instance()->IsOpen( ash::ShelfID(kPluginVmAppId));
ash::ShelfID(kPluginVmAppId));
} }
bool IsPluginVmWindow(const aura::Window* window) { bool IsPluginVmWindow(const aura::Window* window) {
......
...@@ -17,6 +17,14 @@ FakeVmPluginDispatcherClient::FakeVmPluginDispatcherClient() = default; ...@@ -17,6 +17,14 @@ FakeVmPluginDispatcherClient::FakeVmPluginDispatcherClient() = default;
FakeVmPluginDispatcherClient::~FakeVmPluginDispatcherClient() = default; FakeVmPluginDispatcherClient::~FakeVmPluginDispatcherClient() = default;
void FakeVmPluginDispatcherClient::AddObserver(Observer* observer) {
observer_list_.AddObserver(observer);
}
void FakeVmPluginDispatcherClient::RemoveObserver(Observer* observer) {
observer_list_.RemoveObserver(observer);
}
void FakeVmPluginDispatcherClient::StartVm( void FakeVmPluginDispatcherClient::StartVm(
const StartVmRequest& request, const StartVmRequest& request,
DBusMethodCallback<StartVmResponse> callback) { DBusMethodCallback<StartVmResponse> callback) {
...@@ -63,4 +71,11 @@ void FakeVmPluginDispatcherClient::WaitForServiceToBeAvailable( ...@@ -63,4 +71,11 @@ void FakeVmPluginDispatcherClient::WaitForServiceToBeAvailable(
FROM_HERE, base::BindOnce(std::move(callback), true)); FROM_HERE, base::BindOnce(std::move(callback), true));
} }
void FakeVmPluginDispatcherClient::NotifyVmStateChanged(
const vm_tools::plugin_dispatcher::VmStateChangedSignal& signal) {
for (auto& observer : observer_list_) {
observer.OnVmStateChanged(signal);
}
}
} // namespace chromeos } // namespace chromeos
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef CHROMEOS_DBUS_FAKE_VM_PLUGIN_DISPATCHER_CLIENT_H_ #ifndef CHROMEOS_DBUS_FAKE_VM_PLUGIN_DISPATCHER_CLIENT_H_
#define CHROMEOS_DBUS_FAKE_VM_PLUGIN_DISPATCHER_CLIENT_H_ #define CHROMEOS_DBUS_FAKE_VM_PLUGIN_DISPATCHER_CLIENT_H_
#include "base/observer_list.h"
#include "chromeos/dbus/vm_plugin_dispatcher_client.h" #include "chromeos/dbus/vm_plugin_dispatcher_client.h"
namespace chromeos { namespace chromeos {
...@@ -15,6 +16,9 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeVmPluginDispatcherClient ...@@ -15,6 +16,9 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeVmPluginDispatcherClient
FakeVmPluginDispatcherClient(); FakeVmPluginDispatcherClient();
~FakeVmPluginDispatcherClient() override; ~FakeVmPluginDispatcherClient() override;
void AddObserver(Observer* observer) override;
void RemoveObserver(Observer* observer) override;
void StartVm(const vm_tools::plugin_dispatcher::StartVmRequest& request, void StartVm(const vm_tools::plugin_dispatcher::StartVmRequest& request,
DBusMethodCallback<vm_tools::plugin_dispatcher::StartVmResponse> DBusMethodCallback<vm_tools::plugin_dispatcher::StartVmResponse>
callback) override; callback) override;
...@@ -51,6 +55,10 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeVmPluginDispatcherClient ...@@ -51,6 +55,10 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeVmPluginDispatcherClient
list_vms_response_ = response; list_vms_response_ = response;
} }
// Calls observers of the OnVmStateChanged signal
void NotifyVmStateChanged(
const vm_tools::plugin_dispatcher::VmStateChangedSignal& signal);
protected: protected:
void Init(dbus::Bus* bus) override {} void Init(dbus::Bus* bus) override {}
...@@ -63,6 +71,8 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeVmPluginDispatcherClient ...@@ -63,6 +71,8 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeVmPluginDispatcherClient
vm_tools::plugin_dispatcher::ListVmResponse list_vms_response_; vm_tools::plugin_dispatcher::ListVmResponse list_vms_response_;
base::ObserverList<Observer> observer_list_;
DISALLOW_COPY_AND_ASSIGN(FakeVmPluginDispatcherClient); DISALLOW_COPY_AND_ASSIGN(FakeVmPluginDispatcherClient);
}; };
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/location.h" #include "base/location.h"
#include "base/observer_list.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "dbus/bus.h" #include "dbus/bus.h"
#include "dbus/message.h" #include "dbus/message.h"
...@@ -25,6 +26,14 @@ class VmPluginDispatcherClientImpl : public VmPluginDispatcherClient { ...@@ -25,6 +26,14 @@ class VmPluginDispatcherClientImpl : public VmPluginDispatcherClient {
~VmPluginDispatcherClientImpl() override = default; ~VmPluginDispatcherClientImpl() override = default;
void AddObserver(Observer* observer) override {
observer_list_.AddObserver(observer);
}
void RemoveObserver(Observer* observer) override {
observer_list_.RemoveObserver(observer);
}
void StartVm(const StartVmRequest& request, void StartVm(const StartVmRequest& request,
DBusMethodCallback<StartVmResponse> callback) override { DBusMethodCallback<StartVmResponse> callback) override {
CallMethod(kStartVmMethod, request, std::move(callback)); CallMethod(kStartVmMethod, request, std::move(callback));
...@@ -66,6 +75,15 @@ class VmPluginDispatcherClientImpl : public VmPluginDispatcherClient { ...@@ -66,6 +75,15 @@ class VmPluginDispatcherClientImpl : public VmPluginDispatcherClient {
LOG(ERROR) << "Unable to get dbus proxy for " LOG(ERROR) << "Unable to get dbus proxy for "
<< kVmPluginDispatcherServiceName; << kVmPluginDispatcherServiceName;
} }
vm_plugin_dispatcher_proxy_->ConnectToSignal(
vm_tools::plugin_dispatcher::kVmPluginDispatcherInterface,
vm_tools::plugin_dispatcher::kVmStateChangedSignal,
base::BindRepeating(
&VmPluginDispatcherClientImpl::OnVmStateChangedSignal,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&VmPluginDispatcherClientImpl::OnSignalConnected,
weak_ptr_factory_.GetWeakPtr()));
} }
private: private:
...@@ -107,8 +125,37 @@ class VmPluginDispatcherClientImpl : public VmPluginDispatcherClient { ...@@ -107,8 +125,37 @@ class VmPluginDispatcherClientImpl : public VmPluginDispatcherClient {
std::move(callback).Run(std::move(reponse_proto)); std::move(callback).Run(std::move(reponse_proto));
} }
void OnVmStateChangedSignal(dbus::Signal* signal) {
DCHECK_EQ(signal->GetInterface(),
vm_tools::plugin_dispatcher::kVmPluginDispatcherInterface);
DCHECK_EQ(signal->GetMember(),
vm_tools::plugin_dispatcher::kVmStateChangedSignal);
vm_tools::plugin_dispatcher::VmStateChangedSignal vm_state_changed_signal;
dbus::MessageReader reader(signal);
if (!reader.PopArrayOfBytesAsProto(&vm_state_changed_signal)) {
LOG(ERROR) << "Failed to parse proto from DBus Signal";
return;
}
for (auto& observer : observer_list_) {
observer.OnVmStateChanged(vm_state_changed_signal);
}
}
void OnSignalConnected(const std::string& interface_name,
const std::string& signal_name,
bool is_connected) {
DCHECK_EQ(interface_name,
vm_tools::plugin_dispatcher::kVmPluginDispatcherInterface);
if (!is_connected)
LOG(ERROR) << "Failed to connect to signal: " << signal_name;
}
dbus::ObjectProxy* vm_plugin_dispatcher_proxy_ = nullptr; dbus::ObjectProxy* vm_plugin_dispatcher_proxy_ = nullptr;
base::ObserverList<Observer> observer_list_;
base::WeakPtrFactory<VmPluginDispatcherClientImpl> weak_ptr_factory_; base::WeakPtrFactory<VmPluginDispatcherClientImpl> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(VmPluginDispatcherClientImpl); DISALLOW_COPY_AND_ASSIGN(VmPluginDispatcherClientImpl);
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <memory> #include <memory>
#include "base/component_export.h" #include "base/component_export.h"
#include "base/observer_list_types.h"
#include "chromeos/dbus/dbus_client.h" #include "chromeos/dbus/dbus_client.h"
#include "chromeos/dbus/dbus_method_call_status.h" #include "chromeos/dbus/dbus_method_call_status.h"
#include "chromeos/dbus/vm_plugin_dispatcher/vm_plugin_dispatcher.pb.h" #include "chromeos/dbus/vm_plugin_dispatcher/vm_plugin_dispatcher.pb.h"
...@@ -20,6 +21,19 @@ namespace chromeos { ...@@ -20,6 +21,19 @@ namespace chromeos {
class COMPONENT_EXPORT(CHROMEOS_DBUS) VmPluginDispatcherClient class COMPONENT_EXPORT(CHROMEOS_DBUS) VmPluginDispatcherClient
: public DBusClient { : public DBusClient {
public: public:
// Used to observe changes to VM state.
class Observer : public base::CheckedObserver {
public:
virtual void OnVmStateChanged(
const vm_tools::plugin_dispatcher::VmStateChangedSignal& signal) = 0;
protected:
~Observer() override = default;
};
virtual void AddObserver(Observer* observer) = 0;
virtual void RemoveObserver(Observer* observer) = 0;
// Asynchronously starts a given VM. // Asynchronously starts a given VM.
virtual void StartVm( virtual void StartVm(
const vm_tools::plugin_dispatcher::StartVmRequest& request, const vm_tools::plugin_dispatcher::StartVmRequest& request,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment