Skip to content

Commit

Permalink
Rename CrostiniStabilityMonitor to GuestOsStabilityMonitor.
Browse files Browse the repository at this point in the history
Move VmShutdownObserver to its own file, adjacent VmStartingObserver.

Bug: b:173546714
Change-Id: Id88ee7be80bcd7ff98a69e92f132878502d7e5dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576662
Auto-Submit: Chloe Pelling <cpelling@google.com>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Nic Hollingum <hollingum@google.com>
Cr-Commit-Position: refs/heads/master@{#834804}
  • Loading branch information
cpelling authored and Chromium LUCI CQ committed Dec 8, 2020
1 parent 033b5d6 commit 92f599c
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 101 deletions.
8 changes: 5 additions & 3 deletions chrome/browser/chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1107,8 +1107,6 @@ source_set("chromeos") {
"crostini/crostini_shelf_utils.cc",
"crostini/crostini_shelf_utils.h",
"crostini/crostini_simple_types.h",
"crostini/crostini_stability_monitor.cc",
"crostini/crostini_stability_monitor.h",
"crostini/crostini_terminal.cc",
"crostini/crostini_terminal.h",
"crostini/crostini_unsupported_action_notifier.cc",
Expand Down Expand Up @@ -1454,6 +1452,8 @@ source_set("chromeos") {
"guest_os/guest_os_share_path.h",
"guest_os/guest_os_share_path_factory.cc",
"guest_os/guest_os_share_path_factory.h",
"guest_os/guest_os_stability_monitor.cc",
"guest_os/guest_os_stability_monitor.h",
"hats/hats_dialog.cc",
"hats/hats_dialog.h",
"hats/hats_finch_helper.cc",
Expand Down Expand Up @@ -2868,6 +2868,8 @@ source_set("chromeos") {
"usb/cros_usb_detector.h",
"virtual_machines/virtual_machines_util.cc",
"virtual_machines/virtual_machines_util.h",
"vm_shutdown_observer.h",
"vm_starting_observer.h",
"web_applications/camera_system_web_app_info.cc",
"web_applications/camera_system_web_app_info.h",
"web_applications/chrome_camera_app_ui_delegate.cc",
Expand Down Expand Up @@ -3462,7 +3464,6 @@ source_set("unit_tests") {
"crostini/crostini_port_forwarder_unittest.cc",
"crostini/crostini_reporting_util_unittest.cc",
"crostini/crostini_shelf_utils_unittest.cc",
"crostini/crostini_stability_monitor_unittest.cc",
"crostini/crostini_unsupported_action_notifier_unittest.cc",
"crostini/crostini_upgrade_available_notification_unittest.cc",
"crostini/crostini_util_unittest.cc",
Expand Down Expand Up @@ -3572,6 +3573,7 @@ source_set("unit_tests") {
"guest_os/guest_os_external_protocol_handler_unittest.cc",
"guest_os/guest_os_registry_service_unittest.cc",
"guest_os/guest_os_share_path_unittest.cc",
"guest_os/guest_os_stability_monitor_unittest.cc",
"hats/hats_finch_helper_unittest.cc",
"hats/hats_notification_controller_unittest.cc",
"input_method/assistive_suggester_unittest.cc",
Expand Down
20 changes: 11 additions & 9 deletions chrome/browser/chromeos/crostini/crostini_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@
#include "chrome/browser/chromeos/crostini/crostini_pref_names.h"
#include "chrome/browser/chromeos/crostini/crostini_remover.h"
#include "chrome/browser/chromeos/crostini/crostini_reporting_util.h"
#include "chrome/browser/chromeos/crostini/crostini_stability_monitor.h"
#include "chrome/browser/chromeos/crostini/crostini_types.mojom.h"
#include "chrome/browser/chromeos/crostini/crostini_upgrade_available_notification.h"
#include "chrome/browser/chromeos/crostini/throttle/crostini_throttle.h"
#include "chrome/browser/chromeos/file_manager/path_util.h"
#include "chrome/browser/chromeos/file_manager/volume_manager.h"
#include "chrome/browser/chromeos/guest_os/guest_os_share_path.h"
#include "chrome/browser/chromeos/guest_os/guest_os_stability_monitor.h"
#include "chrome/browser/chromeos/policy/powerwash_requirements_checker.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/chromeos/scheduler_configuration_manager.h"
Expand Down Expand Up @@ -152,7 +152,7 @@ CrostiniManager::RestartOptions& CrostiniManager::RestartOptions::operator=(
RestartOptions&&) = default;

class CrostiniManager::CrostiniRestarter
: public crostini::VmShutdownObserver,
: public chromeos::VmShutdownObserver,
public chromeos::disks::DiskMountManager::Observer,
public chromeos::SchedulerConfigurationManagerBase::Observer {
public:
Expand Down Expand Up @@ -208,7 +208,7 @@ class CrostiniManager::CrostiniRestarter
}
}

// crostini::VmShutdownObserver
// chromeos::VmShutdownObserver
void OnVmShutdown(const std::string& vm_name) override {
if (ReturnEarlyIfAborted()) {
return;
Expand Down Expand Up @@ -922,8 +922,8 @@ CrostiniManager::CrostiniManager(Profile* profile)
chromeos::PowerManagerClient::Get()->AddObserver(this);
}
CrostiniThrottle::GetForBrowserContext(profile_);
crostini_stability_monitor_ =
std::make_unique<CrostiniStabilityMonitor>(this);
guest_os_stability_monitor_ =
std::make_unique<guest_os::GuestOsStabilityMonitor>(this);
}

CrostiniManager::~CrostiniManager() {
Expand All @@ -949,9 +949,9 @@ void CrostiniManager::RemoveDBusObservers() {
if (chromeos::PowerManagerClient::Get()) {
chromeos::PowerManagerClient::Get()->RemoveObserver(this);
}
// CrostiniStabilityMonitor needs to be destructed here so it can unregister
// GuestOsStabilityMonitor needs to be destructed here so it can unregister
// itself from the DBus clients that may no longer exist later.
crostini_stability_monitor_.reset();
guest_os_stability_monitor_.reset();
}

// static
Expand Down Expand Up @@ -2150,10 +2150,12 @@ void CrostiniManager::RemoveUpgradeContainerProgressObserver(
upgrade_container_progress_observers_.RemoveObserver(observer);
}

void CrostiniManager::AddVmShutdownObserver(VmShutdownObserver* observer) {
void CrostiniManager::AddVmShutdownObserver(
chromeos::VmShutdownObserver* observer) {
vm_shutdown_observers_.AddObserver(observer);
}
void CrostiniManager::RemoveVmShutdownObserver(VmShutdownObserver* observer) {
void CrostiniManager::RemoveVmShutdownObserver(
chromeos::VmShutdownObserver* observer) {
vm_shutdown_observers_.RemoveObserver(observer);
}

Expand Down
21 changes: 10 additions & 11 deletions chrome/browser/chromeos/crostini/crostini_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "chrome/browser/chromeos/crostini/crostini_types.mojom-forward.h"
#include "chrome/browser/chromeos/crostini/crostini_util.h"
#include "chrome/browser/chromeos/crostini/termina_installer.h"
#include "chrome/browser/chromeos/vm_shutdown_observer.h"
#include "chrome/browser/chromeos/vm_starting_observer.h"
#include "chrome/browser/component_updater/cros_component_installer_chromeos.h"
#include "chrome/browser/ui/browser.h"
Expand All @@ -38,9 +39,12 @@

class Profile;

namespace guest_os {
class GuestOsStabilityMonitor;
}

namespace crostini {

class CrostiniStabilityMonitor;
class CrostiniUpgradeAvailableNotification;

class LinuxPackageOperationProgressObserver {
Expand Down Expand Up @@ -126,12 +130,6 @@ class CrostiniContainerPropertiesObserver : public base::CheckedObserver {
bool can_upgrade) = 0;
};

class VmShutdownObserver : public base::CheckedObserver {
public:
// Called when the given VM has shutdown.
virtual void OnVmShutdown(const std::string& vm_name) = 0;
};

class ContainerStartedObserver : public base::CheckedObserver {
public:
// Called when the container has started.
Expand Down Expand Up @@ -503,8 +501,8 @@ class CrostiniManager : public KeyedService,
UpgradeContainerProgressObserver* observer);

// Add/remove vm shutdown observers.
void AddVmShutdownObserver(VmShutdownObserver* observer);
void RemoveVmShutdownObserver(VmShutdownObserver* observer);
void AddVmShutdownObserver(chromeos::VmShutdownObserver* observer);
void RemoveVmShutdownObserver(chromeos::VmShutdownObserver* observer);

// Add/remove vm starting observers.
void AddVmStartingObserver(chromeos::VmStartingObserver* observer);
Expand Down Expand Up @@ -887,7 +885,7 @@ class CrostiniManager : public KeyedService,
base::ObserverList<UpgradeContainerProgressObserver>::Unchecked
upgrade_container_progress_observers_;

base::ObserverList<VmShutdownObserver> vm_shutdown_observers_;
base::ObserverList<chromeos::VmShutdownObserver> vm_shutdown_observers_;
base::ObserverList<chromeos::VmStartingObserver> vm_starting_observers_;

// Only one restarter flow is actually running for a given container, other
Expand Down Expand Up @@ -924,7 +922,8 @@ class CrostiniManager : public KeyedService,

base::Time time_of_last_disk_type_metric_;

std::unique_ptr<CrostiniStabilityMonitor> crostini_stability_monitor_;
std::unique_ptr<guest_os::GuestOsStabilityMonitor>
guest_os_stability_monitor_;

std::unique_ptr<CrostiniUpgradeAvailableNotification>
upgrade_available_notification_;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/chromeos/crostini/crostini_package_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace crostini {
class CrostiniPackageService : public KeyedService,
public LinuxPackageOperationProgressObserver,
public PendingAppListUpdatesObserver,
public VmShutdownObserver {
public chromeos::VmShutdownObserver {
public:
using StateChangeCallback =
base::RepeatingCallback<void(PackageOperationStatus)>;
Expand Down Expand Up @@ -68,7 +68,7 @@ class CrostiniPackageService : public KeyedService,
void OnPendingAppListUpdates(const ContainerId& container_id,
int count) override;

// VmShutdownObserver
// chromeos::VmShutdownObserver
void OnVmShutdown(const std::string& vm_name) override;

// (Eventually) install a Linux package. If successfully started, a system
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/chromeos/crostini/crostini_stability_monitor.h"
#include "chrome/browser/chromeos/guest_os/guest_os_stability_monitor.h"

#include "base/metrics/histogram_functions.h"
#include "chromeos/dbus/dbus_thread_manager.h"

namespace crostini {
namespace guest_os {

const char kCrostiniStabilityHistogram[] = "Crostini.Stability";

CrostiniStabilityMonitor::CrostiniStabilityMonitor(
CrostiniManager* crostini_manager)
GuestOsStabilityMonitor::GuestOsStabilityMonitor(
crostini::CrostiniManager* crostini_manager)
: concierge_observer_(this),
cicerone_observer_(this),
seneschal_observer_(this),
Expand All @@ -23,96 +23,96 @@ CrostiniStabilityMonitor::CrostiniStabilityMonitor(
chromeos::DBusThreadManager::Get()->GetConciergeClient();
DCHECK(concierge_client);
concierge_client->WaitForServiceToBeAvailable(
base::BindOnce(&CrostiniStabilityMonitor::ConciergeStarted,
base::BindOnce(&GuestOsStabilityMonitor::ConciergeStarted,
weak_ptr_factory_.GetWeakPtr()));

auto* cicerone_client =
chromeos::DBusThreadManager::Get()->GetCiceroneClient();
DCHECK(cicerone_client);
cicerone_client->WaitForServiceToBeAvailable(
base::BindOnce(&CrostiniStabilityMonitor::CiceroneStarted,
base::BindOnce(&GuestOsStabilityMonitor::CiceroneStarted,
weak_ptr_factory_.GetWeakPtr()));

auto* seneschal_client =
chromeos::DBusThreadManager::Get()->GetSeneschalClient();
DCHECK(seneschal_client);
seneschal_client->WaitForServiceToBeAvailable(
base::BindOnce(&CrostiniStabilityMonitor::SeneschalStarted,
base::BindOnce(&GuestOsStabilityMonitor::SeneschalStarted,
weak_ptr_factory_.GetWeakPtr()));

auto* chunneld_client =
chromeos::DBusThreadManager::Get()->GetChunneldClient();
DCHECK(chunneld_client);
chunneld_client->WaitForServiceToBeAvailable(
base::BindOnce(&CrostiniStabilityMonitor::ChunneldStarted,
base::BindOnce(&GuestOsStabilityMonitor::ChunneldStarted,
weak_ptr_factory_.GetWeakPtr()));

vm_stopped_observer_.Add(crostini_manager);
vm_stopped_observer_.Observe(crostini_manager);
}

CrostiniStabilityMonitor::~CrostiniStabilityMonitor() {}
GuestOsStabilityMonitor::~GuestOsStabilityMonitor() {}

void CrostiniStabilityMonitor::ConciergeStarted(bool is_available) {
void GuestOsStabilityMonitor::ConciergeStarted(bool is_available) {
DCHECK(is_available);

auto* concierge_client =
chromeos::DBusThreadManager::Get()->GetConciergeClient();
DCHECK(concierge_client);
concierge_observer_.Add(concierge_client);
concierge_observer_.Observe(concierge_client);
}

void CrostiniStabilityMonitor::CiceroneStarted(bool is_available) {
void GuestOsStabilityMonitor::CiceroneStarted(bool is_available) {
DCHECK(is_available);

auto* cicerone_client =
chromeos::DBusThreadManager::Get()->GetCiceroneClient();
DCHECK(cicerone_client);
cicerone_observer_.Add(cicerone_client);
cicerone_observer_.Observe(cicerone_client);
}

void CrostiniStabilityMonitor::SeneschalStarted(bool is_available) {
void GuestOsStabilityMonitor::SeneschalStarted(bool is_available) {
DCHECK(is_available);

auto* seneschal_client =
chromeos::DBusThreadManager::Get()->GetSeneschalClient();
DCHECK(seneschal_client);
seneschal_observer_.Add(seneschal_client);
seneschal_observer_.Observe(seneschal_client);
}

void CrostiniStabilityMonitor::ChunneldStarted(bool is_available) {
void GuestOsStabilityMonitor::ChunneldStarted(bool is_available) {
DCHECK(is_available);

auto* chunneld_client =
chromeos::DBusThreadManager::Get()->GetChunneldClient();
DCHECK(chunneld_client);
chunneld_observer_.Add(chunneld_client);
chunneld_observer_.Observe(chunneld_client);
}

void CrostiniStabilityMonitor::ConciergeServiceStopped() {
void GuestOsStabilityMonitor::ConciergeServiceStopped() {
base::UmaHistogramEnumeration(kCrostiniStabilityHistogram,
FailureClasses::ConciergeStopped);
}
void CrostiniStabilityMonitor::ConciergeServiceStarted() {}
void GuestOsStabilityMonitor::ConciergeServiceStarted() {}

void CrostiniStabilityMonitor::CiceroneServiceStopped() {
void GuestOsStabilityMonitor::CiceroneServiceStopped() {
base::UmaHistogramEnumeration(kCrostiniStabilityHistogram,
FailureClasses::CiceroneStopped);
}
void CrostiniStabilityMonitor::CiceroneServiceStarted() {}
void GuestOsStabilityMonitor::CiceroneServiceStarted() {}

void CrostiniStabilityMonitor::SeneschalServiceStopped() {
void GuestOsStabilityMonitor::SeneschalServiceStopped() {
base::UmaHistogramEnumeration(kCrostiniStabilityHistogram,
FailureClasses::SeneschalStopped);
}
void CrostiniStabilityMonitor::SeneschalServiceStarted() {}
void GuestOsStabilityMonitor::SeneschalServiceStarted() {}

void CrostiniStabilityMonitor::ChunneldServiceStopped() {
void GuestOsStabilityMonitor::ChunneldServiceStopped() {
base::UmaHistogramEnumeration(kCrostiniStabilityHistogram,
FailureClasses::ChunneldStopped);
}
void CrostiniStabilityMonitor::ChunneldServiceStarted() {}
void GuestOsStabilityMonitor::ChunneldServiceStarted() {}

void CrostiniStabilityMonitor::OnVmShutdown(const std::string& vm_name) {
void GuestOsStabilityMonitor::OnVmShutdown(const std::string& vm_name) {
// CrostiniManager calls this observer method before removing the VM from its
// tracking list, so this list will tell us what state the VM was believed to
// be in before the stop signal was received.
Expand All @@ -128,4 +128,4 @@ void CrostiniStabilityMonitor::OnVmShutdown(const std::string& vm_name) {
}
}

} // namespace crostini
} // namespace guest_os
Loading

0 comments on commit 92f599c

Please sign in to comment.