update_engine: Move DaemonStateInterface implementation to UpdateAttempter
It seems like UpdateAttempter is the best option for implementation of
DaemonStateInterface. SystemState should only be doing state keeping not
doing these startup logics.
BUG=b:171829801
TEST=unittests
TEST=CQ passes
Change-Id: I47ec50107ffbeb544e061f39c900a1559f2cdcab
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2519843
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/cros/daemon_chromeos.cc b/cros/daemon_chromeos.cc
index a7cad8c..1e0e6d6 100644
--- a/cros/daemon_chromeos.cc
+++ b/cros/daemon_chromeos.cc
@@ -46,13 +46,13 @@
// avoiding the explicit re-usage of the |bus| instance, shared between
// D-Bus service and D-Bus client calls.
RealSystemState* real_system_state = new RealSystemState();
- daemon_state_.reset(real_system_state);
LOG_IF(ERROR, !real_system_state->Initialize())
<< "Failed to initialize system state.";
+ system_state_.reset(real_system_state);
// Create the DBus service.
dbus_adaptor_.reset(new UpdateEngineAdaptor(real_system_state));
- daemon_state_->AddObserver(dbus_adaptor_.get());
+ system_state_->update_attempter()->AddObserver(dbus_adaptor_.get());
dbus_adaptor_->RegisterAsync(
base::Bind(&DaemonChromeOS::OnDBusRegistered, base::Unretained(this)));
@@ -76,7 +76,7 @@
QuitWithExitCode(1);
return;
}
- daemon_state_->StartUpdater();
+ system_state_->update_attempter()->StartUpdater();
}
} // namespace chromeos_update_engine
diff --git a/cros/daemon_chromeos.h b/cros/daemon_chromeos.h
index 5d568c7..3b9c8de 100644
--- a/cros/daemon_chromeos.h
+++ b/cros/daemon_chromeos.h
@@ -47,9 +47,8 @@
// the main() function.
Subprocess subprocess_;
- // The daemon state with all the required daemon classes for the configured
- // platform.
- std::unique_ptr<DaemonStateInterface> daemon_state_;
+ // The global context sysetm state.
+ std::unique_ptr<SystemState> system_state_;
DISALLOW_COPY_AND_ASSIGN(DaemonChromeOS);
};
diff --git a/cros/real_system_state.cc b/cros/real_system_state.cc
index 4f57246..5715a39 100644
--- a/cros/real_system_state.cc
+++ b/cros/real_system_state.cc
@@ -39,20 +39,10 @@
#if USE_DBUS
#include "update_engine/cros/dbus_connection.h"
#endif // USE_DBUS
-#include "update_engine/update_boot_flags_action.h"
#include "update_engine/update_manager/state_factory.h"
-using brillo::MessageLoop;
-
namespace chromeos_update_engine {
-RealSystemState::~RealSystemState() {
- // Prevent any DBus communication from UpdateAttempter when shutting down the
- // daemon.
- if (update_attempter_)
- update_attempter_->ClearObservers();
-}
-
bool RealSystemState::Initialize() {
boot_control_ = boot_control::CreateBootControl();
if (!boot_control_) {
@@ -201,43 +191,4 @@
return true;
}
-bool RealSystemState::StartUpdater() {
- // Initiate update checks.
- update_attempter_->ScheduleUpdates();
-
- auto update_boot_flags_action =
- std::make_unique<UpdateBootFlagsAction>(boot_control_.get());
- processor_.EnqueueAction(std::move(update_boot_flags_action));
- // Update boot flags after 45 seconds.
- MessageLoop::current()->PostDelayedTask(
- FROM_HERE,
- base::Bind(&ActionProcessor::StartProcessing,
- base::Unretained(&processor_)),
- base::TimeDelta::FromSeconds(45));
-
- // Broadcast the update engine status on startup to ensure consistent system
- // state on crashes.
- MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&UpdateAttempter::BroadcastStatus,
- base::Unretained(update_attempter_.get())));
-
- // Run the UpdateEngineStarted() method on |update_attempter|.
- MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&UpdateAttempter::UpdateEngineStarted,
- base::Unretained(update_attempter_.get())));
- return true;
-}
-
-void RealSystemState::AddObserver(ServiceObserverInterface* observer) {
- CHECK(update_attempter_.get());
- update_attempter_->AddObserver(observer);
-}
-
-void RealSystemState::RemoveObserver(ServiceObserverInterface* observer) {
- CHECK(update_attempter_.get());
- update_attempter_->RemoveObserver(observer);
-}
-
} // namespace chromeos_update_engine
diff --git a/cros/real_system_state.h b/cros/real_system_state.h
index 798fca0..a93a9b7 100644
--- a/cros/real_system_state.h
+++ b/cros/real_system_state.h
@@ -48,29 +48,17 @@
// A real implementation of the SystemStateInterface which is
// used by the actual product code.
-class RealSystemState : public SystemState, public DaemonStateInterface {
+class RealSystemState : public SystemState {
public:
// Constructs all system objects that do not require separate initialization;
// see Initialize() below for the remaining ones.
RealSystemState() = default;
- ~RealSystemState() override;
+ ~RealSystemState() = default;
// Initializes and sets systems objects that require an initialization
// separately from construction. Returns |true| on success.
bool Initialize();
- // DaemonStateInterface overrides.
- // Start the periodic update attempts. Must be called at the beginning of the
- // program to start the periodic update check process.
- bool StartUpdater() override;
-
- void AddObserver(ServiceObserverInterface* observer) override;
- void RemoveObserver(ServiceObserverInterface* observer) override;
- const std::set<ServiceObserverInterface*>& service_observers() override {
- CHECK(update_attempter_.get());
- return update_attempter_->service_observers();
- }
-
// SystemState overrides.
inline void set_device_policy(
const policy::DevicePolicy* device_policy) override {
@@ -193,8 +181,6 @@
// rebooted. Important for tracking whether you are running instance of the
// update engine on first boot or due to a crash/restart.
bool system_rebooted_{false};
-
- ActionProcessor processor_;
};
} // namespace chromeos_update_engine
diff --git a/cros/update_attempter.cc b/cros/update_attempter.cc
index e9098de..e417457 100644
--- a/cros/update_attempter.cc
+++ b/cros/update_attempter.cc
@@ -134,6 +134,10 @@
is_install_(false) {}
UpdateAttempter::~UpdateAttempter() {
+ // Prevent any DBus communication from UpdateAttempter when shutting down the
+ // daemon.
+ ClearObservers();
+
// CertificateChecker might not be initialized in unittests.
if (cert_checker_)
cert_checker_->SetObserver(nullptr);
@@ -177,6 +181,33 @@
return true;
}
+bool UpdateAttempter::StartUpdater() {
+ // Initiate update checks.
+ ScheduleUpdates();
+
+ auto update_boot_flags_action =
+ std::make_unique<UpdateBootFlagsAction>(system_state_->boot_control());
+ processor_->EnqueueAction(std::move(update_boot_flags_action));
+ // Update boot flags after 45 seconds.
+ MessageLoop::current()->PostDelayedTask(
+ FROM_HERE,
+ base::Bind(&ActionProcessor::StartProcessing,
+ base::Unretained(processor_.get())),
+ base::TimeDelta::FromSeconds(45));
+
+ // Broadcast the update engine status on startup to ensure consistent system
+ // state on crashes.
+ MessageLoop::current()->PostTask(
+ FROM_HERE,
+ base::Bind(&UpdateAttempter::BroadcastStatus, base::Unretained(this)));
+
+ MessageLoop::current()->PostTask(
+ FROM_HERE,
+ base::Bind(&UpdateAttempter::UpdateEngineStarted,
+ base::Unretained(this)));
+ return true;
+}
+
void UpdateAttempter::CertificateChecked(ServerToCheck server_to_check,
CertificateCheckResult result) {
system_state_->metrics_reporter()->ReportCertificateCheckMetrics(
diff --git a/cros/update_attempter.h b/cros/update_attempter.h
index 0f4c952..bd0aef6 100644
--- a/cros/update_attempter.h
+++ b/cros/update_attempter.h
@@ -34,6 +34,7 @@
#include "update_engine/client_library/include/update_engine/update_status.h"
#include "update_engine/common/action_processor.h"
#include "update_engine/common/cpu_limiter.h"
+#include "update_engine/common/daemon_state_interface.h"
#include "update_engine/common/download_action.h"
#include "update_engine/common/excluder_interface.h"
#include "update_engine/common/proxy_resolver.h"
@@ -59,7 +60,8 @@
class UpdateAttempter : public ActionProcessorDelegate,
public DownloadActionDelegate,
public CertificateChecker::Observer,
- public PostinstallRunnerAction::DelegateInterface {
+ public PostinstallRunnerAction::DelegateInterface,
+ public DaemonStateInterface {
public:
using UpdateStatus = update_engine::UpdateStatus;
using UpdateAttemptFlags = update_engine::UpdateAttemptFlags;
@@ -219,15 +221,15 @@
// 'cros flash' to function properly).
bool IsAnyUpdateSourceAllowed() const;
- // Add and remove a service observer.
- void AddObserver(ServiceObserverInterface* observer) {
+ // |DaemonStateInterface| overrides.
+ bool StartUpdater() override;
+ void AddObserver(ServiceObserverInterface* observer) override {
service_observers_.insert(observer);
}
- void RemoveObserver(ServiceObserverInterface* observer) {
+ void RemoveObserver(ServiceObserverInterface* observer) override {
service_observers_.erase(observer);
}
-
- const std::set<ServiceObserverInterface*>& service_observers() {
+ const std::set<ServiceObserverInterface*>& service_observers() override {
return service_observers_;
}