Revise the SystemState hierarchy.
* Removed all #includes from SystemState; added includes in .cc files
that use the various objects (MetricsLibrary, DevicePolicy, etc).
* MockSystemState:
- Regulated the set of getters/setters: foo() returns the current Foo
object interface; this object can be overridden by set_foo();
mock_foo() or fake_foo() returns the default (internal) mock/fake
equivalent, and fails if it is different from foo() (safety).
- Make member declaration order consistent with that of API.
- Removed MOCK_METHOD declarations for two methods and replaced them
with fake getter/setter. This means that MockSystemState is now
reduced to a fake, and can be renamed (separate CL). This also means
that a few tests have a slightly different semantics now.
* All virtual overrides are qualified as such. However, removed the
'const' method qualified from all getters: it made little sense,
especially when considering that getters are handing addresses of
internal mock members.
* Made the UpdateAttempter a contained member of both
{Real,Mock}SystemState, resolving initialization dependencies. In
general, the invariant is that all members of the SystemState that
rely on it being fully populated by the time of their initialization,
need to export a separate Init() method, that will be called (by the
SystemState implementation constructor or Init() method) only after
all members are set.
* Made the mock GPIO handler and connection manager contained members of
MockSystemState; the destructor could safely be moved.
* Cleanup in UpdateAttempter (part of resolving dependencies):
- Ordinary member initialization done via default initializers
(constants) or initializer list in the constructor (parameters).
- Init() method only does work that cannot be done during
construction, with appropriate comment documenting the need for it.
- Better reuse via constructor delegation.
BUG=chromium:358278
TEST=Unit tests.
Change-Id: I96ff6fc7e7400b0a9feb6cc8d4ffe97a51000f91
Reviewed-on: https://chromium-review.googlesource.com/193587
Reviewed-by: Gilad Arnold <[email protected]>
Tested-by: Gilad Arnold <[email protected]>
Commit-Queue: David Zeuthen <[email protected]>
diff --git a/certificate_checker.cc b/certificate_checker.cc
index 4992fda..02b5661 100644
--- a/certificate_checker.cc
+++ b/certificate_checker.cc
@@ -11,10 +11,10 @@
#include <base/strings/stringprintf.h>
#include <base/logging.h>
#include <curl/curl.h>
-#include <metrics/metrics_library.h>
#include <openssl/evp.h>
#include <openssl/ssl.h>
+#include "metrics/metrics_library.h"
#include "update_engine/constants.h"
#include "update_engine/prefs_interface.h"
#include "update_engine/utils.h"
diff --git a/connection_manager.cc b/connection_manager.cc
index 49d19d4..45c971a 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -11,6 +11,7 @@
#include <chromeos/dbus/service_constants.h>
#include <dbus/dbus-glib.h>
#include <glib.h>
+#include <policy/device_policy.h>
#include "update_engine/prefs.h"
#include "update_engine/system_state.h"
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
index 3dc065e..d574408 100644
--- a/connection_manager_unittest.cc
+++ b/connection_manager_unittest.cc
@@ -229,26 +229,21 @@
}
TEST_F(ConnectionManagerTest, AllowUpdatesOverEthernetTest) {
- EXPECT_CALL(mock_system_state_, device_policy()).Times(0);
-
// Updates over Ethernet are allowed even if there's no policy.
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetEthernet,
NetworkTethering::kUnknown));
}
TEST_F(ConnectionManagerTest, AllowUpdatesOverWifiTest) {
- EXPECT_CALL(mock_system_state_, device_policy()).Times(0);
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetWifi, NetworkTethering::kUnknown));
}
TEST_F(ConnectionManagerTest, AllowUpdatesOverWimaxTest) {
- EXPECT_CALL(mock_system_state_, device_policy()).Times(0);
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetWimax,
NetworkTethering::kUnknown));
}
TEST_F(ConnectionManagerTest, BlockUpdatesOverBluetoothTest) {
- EXPECT_CALL(mock_system_state_, device_policy()).Times(0);
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetBluetooth,
NetworkTethering::kUnknown));
}
@@ -256,9 +251,7 @@
TEST_F(ConnectionManagerTest, AllowUpdatesOnlyOver3GPerPolicyTest) {
policy::MockDevicePolicy allow_3g_policy;
- EXPECT_CALL(mock_system_state_, device_policy())
- .Times(1)
- .WillOnce(Return(&allow_3g_policy));
+ mock_system_state_.set_device_policy(&allow_3g_policy);
// This test tests cellular (3G) being the only connection type being allowed.
set<string> allowed_set;
@@ -275,9 +268,7 @@
TEST_F(ConnectionManagerTest, AllowUpdatesOver3GAndOtherTypesPerPolicyTest) {
policy::MockDevicePolicy allow_3g_policy;
- EXPECT_CALL(mock_system_state_, device_policy())
- .Times(3)
- .WillRepeatedly(Return(&allow_3g_policy));
+ mock_system_state_.set_device_policy(&allow_3g_policy);
// This test tests multiple connection types being allowed, with
// 3G one among them. Only Cellular is currently enforced by the policy
@@ -310,13 +301,11 @@
}
TEST_F(ConnectionManagerTest, BlockUpdatesOverCellularByDefaultTest) {
- EXPECT_CALL(mock_system_state_, device_policy()).Times(1);
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular,
NetworkTethering::kUnknown));
}
TEST_F(ConnectionManagerTest, BlockUpdatesOverTetheredNetworkByDefaultTest) {
- EXPECT_CALL(mock_system_state_, device_policy()).Times(2);
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetWifi,
NetworkTethering::kConfirmed));
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetEthernet,
@@ -328,9 +317,7 @@
TEST_F(ConnectionManagerTest, BlockUpdatesOver3GPerPolicyTest) {
policy::MockDevicePolicy block_3g_policy;
- EXPECT_CALL(mock_system_state_, device_policy())
- .Times(1)
- .WillOnce(Return(&block_3g_policy));
+ mock_system_state_.set_device_policy(&block_3g_policy);
// Test that updates for 3G are blocked while updates are allowed
// over several other types.
@@ -350,9 +337,7 @@
TEST_F(ConnectionManagerTest, BlockUpdatesOver3GIfErrorInPolicyFetchTest) {
policy::MockDevicePolicy allow_3g_policy;
- EXPECT_CALL(mock_system_state_, device_policy())
- .Times(1)
- .WillOnce(Return(&allow_3g_policy));
+ mock_system_state_.set_device_policy(&allow_3g_policy);
set<string> allowed_set;
allowed_set.insert(cmut_.StringForConnectionType(kNetCellular));
@@ -372,9 +357,7 @@
policy::MockDevicePolicy no_policy;
testing::NiceMock<PrefsMock>* prefs = mock_system_state_.mock_prefs();
- EXPECT_CALL(mock_system_state_, device_policy())
- .Times(3)
- .WillRepeatedly(Return(&no_policy));
+ mock_system_state_.set_device_policy(&no_policy);
// No setting enforced by the device policy, user prefs should be used.
EXPECT_CALL(no_policy, GetAllowedConnectionTypesForUpdate(_))
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index f221fc0..cbd831d 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -1267,7 +1267,7 @@
DeltaPerformer *performer = new DeltaPerformer(&prefs,
&mock_system_state,
&install_plan);
- FakeHardware* fake_hardware = mock_system_state.get_fake_hardware();
+ FakeHardware* fake_hardware = mock_system_state.fake_hardware();
string temp_dir;
EXPECT_TRUE(utils::MakeTempDirectory("PublicKeyFromResponseTests.XXXXXX",
diff --git a/download_action.cc b/download_action.cc
index 8508670..8d211ea 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -13,6 +13,7 @@
#include <base/strings/stringprintf.h>
#include "update_engine/action_pipe.h"
+#include "update_engine/omaha_request_params.h"
#include "update_engine/p2p_manager.h"
#include "update_engine/subprocess.h"
#include "update_engine/utils.h"
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index 91a0d10..a37becd 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -272,7 +272,7 @@
// Speed up test execution.
ret->set_idle_seconds(1);
ret->set_retry_seconds(1);
- mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(false);
+ mock_system_state_.fake_hardware()->SetIsOfficialBuild(false);
return ret;
}
@@ -323,7 +323,7 @@
// Speed up test execution.
ret->set_idle_seconds(1);
ret->set_retry_seconds(1);
- mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(false);
+ mock_system_state_.fake_hardware()->SetIsOfficialBuild(false);
return ret;
}
@@ -1051,7 +1051,7 @@
LOG(INFO) << "added range: " << tmp_str;
}
dynamic_cast<MockSystemState*>(fetcher_in->GetSystemState())
- ->get_fake_hardware()->SetIsOfficialBuild(false);
+ ->fake_hardware()->SetIsOfficialBuild(false);
multi_fetcher->set_delegate(&delegate);
StartTransferArgs start_xfer_args = {multi_fetcher, url};
@@ -1240,7 +1240,7 @@
LOG(INFO) << "is_official_build: " << is_official_build;
// NewLargeFetcher creates the HttpFetcher* with a MockSystemState.
dynamic_cast<MockSystemState*>(fetcher->GetSystemState())
- ->get_fake_hardware()->SetIsOfficialBuild(is_official_build);
+ ->fake_hardware()->SetIsOfficialBuild(is_official_build);
fetcher->set_delegate(&delegate);
StartTransferArgs start_xfer_args =
diff --git a/mock_system_state.cc b/mock_system_state.cc
index e7ec886..a821836 100644
--- a/mock_system_state.cc
+++ b/mock_system_state.cc
@@ -12,28 +12,26 @@
// Mock the SystemStateInterface so that we could lie that
// OOBE is completed even when there's no such marker file, etc.
MockSystemState::MockSystemState()
- : default_request_params_(this),
- clock_(&default_clock_),
- hardware_(&default_hardware_),
+ : mock_connection_manager_(this),
+ mock_update_attempter_(this, &dbus_),
+ default_request_params_(this),
+ clock_(&fake_clock_),
+ connection_manager_(&mock_connection_manager_),
+ hardware_(&fake_hardware_),
+ metrics_lib_(&mock_metrics_lib_),
prefs_(&mock_prefs_),
powerwash_safe_prefs_(&mock_powerwash_safe_prefs_),
- p2p_manager_(&mock_p2p_manager_),
payload_state_(&mock_payload_state_),
- policy_manager_(&fake_policy_manager_) {
- request_params_ = &default_request_params_;
+ gpio_handler_(&mock_gpio_handler_),
+ update_attempter_(&mock_update_attempter_),
+ request_params_(&default_request_params_),
+ p2p_manager_(&mock_p2p_manager_),
+ policy_manager_(&fake_policy_manager_),
+ device_policy_(nullptr),
+ fake_system_rebooted_(false) {
mock_payload_state_.Initialize(this);
- mock_gpio_handler_ = new testing::NiceMock<MockGpioHandler>();
- mock_update_attempter_ = new testing::NiceMock<UpdateAttempterMock>(
- this, &dbus_);
- mock_connection_manager_ = new testing::NiceMock<MockConnectionManager>(this);
- connection_manager_ = mock_connection_manager_;
+ mock_update_attempter_.Init();
fake_policy_manager_.Init(FakeState::Construct());
}
-MockSystemState::~MockSystemState() {
- delete mock_connection_manager_;
- delete mock_update_attempter_;
- delete mock_gpio_handler_;
-}
-
} // namespace chromeos_update_engine
diff --git a/mock_system_state.h b/mock_system_state.h
index 73987d5..7d79b77 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -5,12 +5,12 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_SYSTEM_STATE_H_
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_SYSTEM_STATE_H_
+#include <base/logging.h>
#include <gmock/gmock.h>
-
-#include <metrics/metrics_library_mock.h>
#include <policy/mock_device_policy.h>
-#include "update_engine/clock.h"
+#include "metrics/metrics_library_mock.h"
+#include "update_engine/fake_clock.h"
#include "update_engine/fake_hardware.h"
#include "update_engine/mock_connection_manager.h"
#include "update_engine/mock_dbus_wrapper.h"
@@ -24,154 +24,240 @@
namespace chromeos_update_engine {
-class UpdateAttempterMock;
-
// Mock the SystemStateInterface so that we could lie that
// OOBE is completed even when there's no such marker file, etc.
class MockSystemState : public SystemState {
public:
MockSystemState();
- virtual ~MockSystemState();
+ // Base class overrides. All getters return the current implementation of
+ // various members, either the default (fake/mock) or the one set to override
+ // it by client code.
- MOCK_METHOD1(set_device_policy, void(const policy::DevicePolicy*));
- MOCK_CONST_METHOD0(device_policy, const policy::DevicePolicy*());
- MOCK_METHOD0(system_rebooted, bool());
-
- inline virtual ClockInterface* clock() {
+ virtual inline ClockInterface* clock() override {
return clock_;
}
- inline virtual ConnectionManager* connection_manager() {
+ virtual inline void set_device_policy(
+ const policy::DevicePolicy* device_policy) override {
+ device_policy_ = device_policy;
+ }
+
+ virtual inline const policy::DevicePolicy* device_policy() override {
+ return device_policy_;
+ }
+
+ virtual inline ConnectionManager* connection_manager() override {
return connection_manager_;
}
- inline virtual HardwareInterface* hardware() {
+ virtual inline HardwareInterface* hardware() override {
return hardware_;
}
- inline virtual MetricsLibraryInterface* metrics_lib() {
- return &mock_metrics_lib_;
+ virtual inline MetricsLibraryInterface* metrics_lib() override {
+ return metrics_lib_;
}
- inline virtual PrefsInterface* prefs() {
+ virtual inline PrefsInterface* prefs() override {
return prefs_;
}
- inline virtual PrefsInterface* powerwash_safe_prefs() {
+ virtual inline PrefsInterface* powerwash_safe_prefs() override {
return powerwash_safe_prefs_;
}
- inline virtual PayloadStateInterface* payload_state() {
+ virtual inline PayloadStateInterface* payload_state() override {
return payload_state_;
}
- inline virtual GpioHandler* gpio_handler() const {
- return mock_gpio_handler_;
+ virtual inline GpioHandler* gpio_handler() override {
+ return gpio_handler_;
}
- inline virtual UpdateAttempterMock* update_attempter() const override {
- return mock_update_attempter_;
+ virtual inline UpdateAttempter* update_attempter() override {
+ return update_attempter_;
}
- inline virtual OmahaRequestParams* request_params() {
+ virtual inline OmahaRequestParams* request_params() override {
return request_params_;
}
- inline virtual P2PManager* p2p_manager() {
+ virtual inline P2PManager* p2p_manager() override {
return p2p_manager_;
}
- inline virtual chromeos_policy_manager::PolicyManager* policy_manager() {
+ virtual inline chromeos_policy_manager::PolicyManager* policy_manager()
+ override {
return policy_manager_;
}
- // MockSystemState-specific public method.
- inline void set_connection_manager(ConnectionManager* connection_manager) {
- connection_manager_ = connection_manager;
+ virtual inline bool system_rebooted() override {
+ return fake_system_rebooted_;
}
- inline MetricsLibraryMock* mock_metrics_lib() {
- return &mock_metrics_lib_;
- }
+ // Setters for the various members, can be used for overriding the default
+ // implementations. For convenience, setting to a null pointer will restore
+ // the default implementation.
inline void set_clock(ClockInterface* clock) {
- clock_ = clock;
+ clock_ = clock ? clock : &fake_clock_;
+ }
+
+ inline void set_connection_manager(ConnectionManager* connection_manager) {
+ connection_manager_ = (connection_manager ? connection_manager :
+ &mock_connection_manager_);
}
inline void set_hardware(HardwareInterface* hardware) {
- hardware_ = hardware;
+ hardware_ = hardware ? hardware : &fake_hardware_;
}
- inline FakeHardware* get_fake_hardware() {
- return &default_hardware_;
+ inline void set_metrics_lib(MetricsLibraryInterface* metrics_lib) {
+ metrics_lib_ = metrics_lib ? metrics_lib : &mock_metrics_lib_;
}
inline void set_prefs(PrefsInterface* prefs) {
- prefs_ = prefs;
+ prefs_ = prefs ? prefs : &mock_prefs_;
}
- inline void set_powerwash_safe_prefs(PrefsInterface* prefs) {
- powerwash_safe_prefs_ = prefs;
+ inline void set_powerwash_safe_prefs(PrefsInterface* powerwash_safe_prefs) {
+ powerwash_safe_prefs_ = (powerwash_safe_prefs ? powerwash_safe_prefs :
+ &mock_powerwash_safe_prefs_);
}
- inline testing::NiceMock<PrefsMock> *mock_prefs() {
- return &mock_prefs_;
+ inline void set_payload_state(PayloadStateInterface *payload_state) {
+ payload_state_ = payload_state ? payload_state : &mock_payload_state_;
}
- inline testing::NiceMock<PrefsMock> *mock_powerwash_safe_prefs() {
- return &mock_powerwash_safe_prefs_;
+ inline void set_gpio_handler(GpioHandler* gpio_handler) {
+ gpio_handler_ = gpio_handler ? gpio_handler : &mock_gpio_handler_;
}
- inline MockPayloadState* mock_payload_state() {
- return &mock_payload_state_;
+ inline void set_update_attempter(UpdateAttempter* update_attempter) {
+ update_attempter_ = (update_attempter ? update_attempter :
+ &mock_update_attempter_);
}
- inline void set_request_params(OmahaRequestParams* params) {
- request_params_ = params;
+ inline void set_request_params(OmahaRequestParams* request_params) {
+ request_params_ = (request_params ? request_params :
+ &default_request_params_);
}
inline void set_p2p_manager(P2PManager *p2p_manager) {
- p2p_manager_ = p2p_manager;
+ p2p_manager_ = p2p_manager ? p2p_manager : &mock_p2p_manager_;
}
inline void set_policy_manager(
chromeos_policy_manager::PolicyManager *policy_manager) {
- policy_manager_ = policy_manager;
+ policy_manager_ = policy_manager ? policy_manager : &fake_policy_manager_;
}
- inline void set_payload_state(PayloadStateInterface *payload_state) {
- payload_state_ = payload_state;
+ inline void set_system_rebooted(bool system_rebooted) {
+ fake_system_rebooted_ = system_rebooted;
+ }
+
+ // Getters for the built-in default implementations. These return the actual
+ // concrete type of each implementation. For additional safety, they will fail
+ // whenever the requested default was overridden by a different
+ // implementation.
+
+ inline FakeClock* fake_clock() {
+ CHECK(clock_ == &fake_clock_);
+ return &fake_clock_;
+ }
+
+ inline testing::NiceMock<MockConnectionManager>* mock_connection_manager() {
+ CHECK(connection_manager_ == &mock_connection_manager_);
+ return &mock_connection_manager_;
+ }
+
+ inline FakeHardware* fake_hardware() {
+ CHECK(hardware_ == &fake_hardware_);
+ return &fake_hardware_;
+ }
+
+ inline testing::NiceMock<MetricsLibraryMock>* mock_metrics_lib() {
+ CHECK(metrics_lib_ == &mock_metrics_lib_);
+ return &mock_metrics_lib_;
+ }
+
+ inline testing::NiceMock<PrefsMock> *mock_prefs() {
+ CHECK(prefs_ == &mock_prefs_);
+ return &mock_prefs_;
+ }
+
+ inline testing::NiceMock<PrefsMock> *mock_powerwash_safe_prefs() {
+ CHECK(powerwash_safe_prefs_ == &mock_powerwash_safe_prefs_);
+ return &mock_powerwash_safe_prefs_;
+ }
+
+ inline testing::NiceMock<MockPayloadState>* mock_payload_state() {
+ CHECK(payload_state_ == &mock_payload_state_);
+ return &mock_payload_state_;
+ }
+
+ inline testing::NiceMock<MockGpioHandler>* mock_gpio_handler() {
+ CHECK(gpio_handler_ == &mock_gpio_handler_);
+ return &mock_gpio_handler_;
+ }
+
+ inline testing::NiceMock<UpdateAttempterMock>* mock_update_attempter() {
+ CHECK(update_attempter_ == &mock_update_attempter_);
+ return &mock_update_attempter_;
+ }
+
+ inline OmahaRequestParams* default_request_params() {
+ CHECK(request_params_ == &default_request_params_);
+ return &default_request_params_;
+ }
+
+ inline testing::NiceMock<MockP2PManager>* mock_p2p_manager() {
+ CHECK(p2p_manager_ == &mock_p2p_manager_);
+ return &mock_p2p_manager_;
+ }
+
+ inline chromeos_policy_manager::FakePolicyManager* fake_policy_manager() {
+ CHECK(policy_manager_ == &fake_policy_manager_);
+ return &fake_policy_manager_;
}
private:
- // These are Mock objects or objects we own.
+ // Default mock/fake implementations (owned).
+ FakeClock fake_clock_;
+ testing::NiceMock<MockConnectionManager> mock_connection_manager_;
+ FakeHardware fake_hardware_;
testing::NiceMock<MetricsLibraryMock> mock_metrics_lib_;
testing::NiceMock<PrefsMock> mock_prefs_;
testing::NiceMock<PrefsMock> mock_powerwash_safe_prefs_;
- testing::NiceMock<MockP2PManager> mock_p2p_manager_;
testing::NiceMock<MockPayloadState> mock_payload_state_;
- testing::NiceMock<MockGpioHandler>* mock_gpio_handler_;
- testing::NiceMock<UpdateAttempterMock>* mock_update_attempter_;
- testing::NiceMock<MockConnectionManager>* mock_connection_manager_;
- MockDBusWrapper dbus_;
-
- // These are the other object we own.
- Clock default_clock_;
- FakeHardware default_hardware_;
- chromeos_policy_manager::FakePolicyManager fake_policy_manager_;
+ testing::NiceMock<MockGpioHandler> mock_gpio_handler_;
+ testing::NiceMock<UpdateAttempterMock> mock_update_attempter_;
OmahaRequestParams default_request_params_;
+ testing::NiceMock<MockP2PManager> mock_p2p_manager_;
+ chromeos_policy_manager::FakePolicyManager fake_policy_manager_;
- // These are pointers to objects which caller can override.
+ // Pointers to objects that client code can override. They are initialized to
+ // the default implementations above.
ClockInterface* clock_;
+ ConnectionManager* connection_manager_;
HardwareInterface* hardware_;
+ MetricsLibraryInterface* metrics_lib_;
PrefsInterface* prefs_;
PrefsInterface* powerwash_safe_prefs_;
- ConnectionManager* connection_manager_;
+ PayloadStateInterface* payload_state_;
+ GpioHandler* gpio_handler_;
+ UpdateAttempter* update_attempter_;
OmahaRequestParams* request_params_;
P2PManager* p2p_manager_;
- PayloadStateInterface* payload_state_;
chromeos_policy_manager::PolicyManager* policy_manager_;
+
+ // Other object pointers (not preinitialized).
+ const policy::DevicePolicy* device_policy_;
+
+ // Other data members.
+ MockDBusWrapper dbus_;
+ bool fake_system_rebooted_;
};
} // namespace chromeos_update_engine
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index b73209c..ea0b4c2 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -2243,7 +2243,7 @@
// If there is no prefs and OOBE is not complete, we should not
// report anything to Omaha.
{
- NiceMock<MockSystemState> system_state;
+ MockSystemState system_state;
system_state.set_prefs(&prefs);
EXPECT_EQ(OmahaRequestAction::GetInstallDate(&system_state), -1);
EXPECT_FALSE(prefs.Exists(kPrefsInstallDateDays));
@@ -2254,11 +2254,11 @@
// prefs. However, first try with an invalid date and check we do
// nothing.
{
- NiceMock<MockSystemState> mock_system_state;
+ MockSystemState mock_system_state;
mock_system_state.set_prefs(&prefs);
Time oobe_date = Time::FromTimeT(42); // Dec 31, 1969 16:00:42 PST.
- mock_system_state.get_fake_hardware()->SetIsOOBEComplete(oobe_date);
+ mock_system_state.fake_hardware()->SetIsOOBEComplete(oobe_date);
EXPECT_EQ(OmahaRequestAction::GetInstallDate(&mock_system_state), -1);
EXPECT_FALSE(prefs.Exists(kPrefsInstallDateDays));
}
@@ -2266,11 +2266,11 @@
// Then check with a valid date. The date Jan 20, 2007 0:00 PST
// should yield an InstallDate of 14.
{
- NiceMock<MockSystemState> mock_system_state;
+ MockSystemState mock_system_state;
mock_system_state.set_prefs(&prefs);
Time oobe_date = Time::FromTimeT(1169280000); // Jan 20, 2007 0:00 PST.
- mock_system_state.get_fake_hardware()->SetIsOOBEComplete(oobe_date);
+ mock_system_state.fake_hardware()->SetIsOOBEComplete(oobe_date);
EXPECT_EQ(OmahaRequestAction::GetInstallDate(&mock_system_state), 14);
EXPECT_TRUE(prefs.Exists(kPrefsInstallDateDays));
@@ -2284,11 +2284,11 @@
// 2007 0:00 PST should yield an InstallDate of 28... but since
// there's a prefs file, we should still get 14.
{
- NiceMock<MockSystemState> mock_system_state;
+ MockSystemState mock_system_state;
mock_system_state.set_prefs(&prefs);
Time oobe_date = Time::FromTimeT(1170144000); // Jan 30, 2007 0:00 PST.
- mock_system_state.get_fake_hardware()->SetIsOOBEComplete(oobe_date);
+ mock_system_state.fake_hardware()->SetIsOOBEComplete(oobe_date);
EXPECT_EQ(OmahaRequestAction::GetInstallDate(&mock_system_state), 14);
int64_t prefs_days;
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 47c1143..e80dcb0 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -8,10 +8,12 @@
#include <base/logging.h>
#include <base/strings/string_util.h>
+#include <policy/device_policy.h>
#include "update_engine/constants.h"
#include "update_engine/delta_performer.h"
#include "update_engine/hardware_interface.h"
+#include "update_engine/omaha_request_params.h"
#include "update_engine/payload_state_interface.h"
#include "update_engine/prefs_interface.h"
#include "update_engine/utils.h"
diff --git a/payload_state.cc b/payload_state.cc
index 2685130..51041ef 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -10,11 +10,13 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
#include <base/format_macros.h>
+#include <policy/device_policy.h>
#include "update_engine/clock.h"
#include "update_engine/constants.h"
#include "update_engine/hardware_interface.h"
#include "update_engine/install_plan.h"
+#include "update_engine/omaha_request_params.h"
#include "update_engine/prefs.h"
#include "update_engine/real_dbus_wrapper.h"
#include "update_engine/system_state.h"
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index a6d6d83..be00d00 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -1032,12 +1032,12 @@
payload_state.UpdateRestarted();
EXPECT_EQ(0, payload_state.GetNumReboots());
- EXPECT_CALL(mock_system_state, system_rebooted()).WillOnce(Return(true));
+ mock_system_state.set_system_rebooted(true);
payload_state.UpdateResumed();
// Num reboots should be incremented because system rebooted detected.
EXPECT_EQ(1, payload_state.GetNumReboots());
- EXPECT_CALL(mock_system_state, system_rebooted()).WillOnce(Return(false));
+ mock_system_state.set_system_rebooted(false);
payload_state.UpdateResumed();
// Num reboots should now be 1 as reboot was not detected.
EXPECT_EQ(1, payload_state.GetNumReboots());
@@ -1191,8 +1191,7 @@
EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
metrics::kMetricTimeToRebootMinutes,
7, _, _, _));
- EXPECT_CALL(mock_system_state, system_rebooted())
- .WillRepeatedly(Return(true));
+ mock_system_state.set_system_rebooted(true);
payload_state2.UpdateEngineStarted();
@@ -1223,8 +1222,7 @@
SendToUMA(_, _, _, _, _)).Times(0);
// Simulate an update_engine restart without a reboot.
- EXPECT_CALL(mock_system_state, system_rebooted())
- .WillRepeatedly(Return(false));
+ mock_system_state.set_system_rebooted(false);
payload_state.UpdateEngineStarted();
}
@@ -1235,8 +1233,7 @@
PayloadState payload_state;
policy::MockDevicePolicy disable_http_policy;
- EXPECT_CALL(mock_system_state, device_policy())
- .WillRepeatedly(Return(&disable_http_policy));
+ mock_system_state.set_device_policy(&disable_http_policy);
EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
// Test with no device policy. Should default to allowing http.
@@ -1277,8 +1274,7 @@
// Now, pretend that the HTTP policy is turned on. We want to make sure
// the new policy is honored.
policy::MockDevicePolicy enable_http_policy;
- EXPECT_CALL(mock_system_state, device_policy())
- .WillRepeatedly(Return(&enable_http_policy));
+ mock_system_state.set_device_policy(&enable_http_policy);
EXPECT_CALL(enable_http_policy, GetHttpDownloadsEnabled(_))
.WillRepeatedly(DoAll(SetArgumentPointee<0>(true), Return(true)));
@@ -1419,7 +1415,7 @@
prefs.Init(base::FilePath(temp_dir));
mock_system_state.set_prefs(&prefs);
- FakeHardware* fake_hardware = mock_system_state.get_fake_hardware();
+ FakeHardware* fake_hardware = mock_system_state.fake_hardware();
fake_hardware->SetBootDevice("/dev/sda3");
EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
@@ -1475,7 +1471,7 @@
prefs.Init(base::FilePath(temp_dir));
mock_system_state.set_prefs(&prefs);
- FakeHardware* fake_hardware = mock_system_state.get_fake_hardware();
+ FakeHardware* fake_hardware = mock_system_state.fake_hardware();
fake_hardware->SetBootDevice("/dev/sda3");
EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
diff --git a/policy_manager/real_updater_provider_unittest.cc b/policy_manager/real_updater_provider_unittest.cc
index ac21566..2778c2a 100644
--- a/policy_manager/real_updater_provider_unittest.cc
+++ b/policy_manager/real_updater_provider_unittest.cc
@@ -116,7 +116,8 @@
kUpdateBootTime + kDurationSinceUpdate :
kUpdateBootTime - kDurationSinceUpdate);
const Time kCurrWallclockTime = FixedTime();
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetBootTimeAtUpdate(_))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetBootTimeAtUpdate(_))
.WillOnce(DoAll(SetArgPointee<0>(kUpdateBootTime), Return(true)));
fake_clock_.SetBootTime(kCurrBootTime);
fake_clock_.SetWallclockTime(kCurrWallclockTime);
@@ -130,63 +131,73 @@
};
TEST_F(PmRealUpdaterProviderTest, GetLastCheckedTimeOkay) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<0>(FixedTime().ToTimeT()), Return(true)));
TestGetValueOkay(provider_->var_last_checked_time(),
RoundedToSecond(FixedTime()));
}
TEST_F(PmRealUpdaterProviderTest, GetLastCheckedTimeFailNoValue) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(Return(false));
TestGetValueFail(provider_->var_last_checked_time());
}
TEST_F(PmRealUpdaterProviderTest, GetProgressOkayMin) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<1>(0.0), Return(true)));
TestGetValueOkay(provider_->var_progress(), 0.0);
}
TEST_F(PmRealUpdaterProviderTest, GetProgressOkayMid) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<1>(0.3), Return(true)));
TestGetValueOkay(provider_->var_progress(), 0.3);
}
TEST_F(PmRealUpdaterProviderTest, GetProgressOkayMax) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<1>(1.0), Return(true)));
TestGetValueOkay(provider_->var_progress(), 1.0);
}
TEST_F(PmRealUpdaterProviderTest, GetProgressFailNoValue) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(Return(false));
TestGetValueFail(provider_->var_progress());
}
TEST_F(PmRealUpdaterProviderTest, GetProgressFailTooSmall) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<1>(-2.0), Return(true)));
TestGetValueFail(provider_->var_progress());
}
TEST_F(PmRealUpdaterProviderTest, GetProgressFailTooBig) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<1>(2.0), Return(true)));
TestGetValueFail(provider_->var_progress());
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayIdle) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<2>(update_engine::kUpdateStatusIdle),
Return(true)));
TestGetValueOkay(provider_->var_stage(), Stage::kIdle);
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayCheckingForUpdate) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(
SetArgPointee<2>(update_engine::kUpdateStatusCheckingForUpdate),
Return(true)));
@@ -194,7 +205,8 @@
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayUpdateAvailable) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(
SetArgPointee<2>(update_engine::kUpdateStatusUpdateAvailable),
Return(true)));
@@ -202,28 +214,32 @@
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayDownloading) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<2>(update_engine::kUpdateStatusDownloading),
Return(true)));
TestGetValueOkay(provider_->var_stage(), Stage::kDownloading);
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayVerifying) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<2>(update_engine::kUpdateStatusVerifying),
Return(true)));
TestGetValueOkay(provider_->var_stage(), Stage::kVerifying);
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayFinalizing) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<2>(update_engine::kUpdateStatusFinalizing),
Return(true)));
TestGetValueOkay(provider_->var_stage(), Stage::kFinalizing);
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayUpdatedNeedReboot) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(
SetArgPointee<2>(update_engine::kUpdateStatusUpdatedNeedReboot),
Return(true)));
@@ -231,7 +247,8 @@
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayReportingErrorEvent) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(
SetArgPointee<2>(update_engine::kUpdateStatusReportingErrorEvent),
Return(true)));
@@ -239,7 +256,8 @@
}
TEST_F(PmRealUpdaterProviderTest, GetStageOkayAttemptingRollback) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(
SetArgPointee<2>(update_engine::kUpdateStatusAttemptingRollback),
Return(true)));
@@ -247,64 +265,74 @@
}
TEST_F(PmRealUpdaterProviderTest, GetStageFailNoValue) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(Return(false));
TestGetValueFail(provider_->var_stage());
}
TEST_F(PmRealUpdaterProviderTest, GetStageFailUnknown) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<2>("FooUpdateEngineState"),
Return(true)));
TestGetValueFail(provider_->var_stage());
}
TEST_F(PmRealUpdaterProviderTest, GetStageFailEmpty) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<2>(""), Return(true)));
TestGetValueFail(provider_->var_stage());
}
TEST_F(PmRealUpdaterProviderTest, GetNewVersionOkay) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<3>("1.2.0"), Return(true)));
TestGetValueOkay(provider_->var_new_version(), string("1.2.0"));
}
TEST_F(PmRealUpdaterProviderTest, GetNewVersionFailNoValue) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(Return(false));
TestGetValueFail(provider_->var_new_version());
}
TEST_F(PmRealUpdaterProviderTest, GetPayloadSizeOkayZero) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<4>(static_cast<int64_t>(0)), Return(true)));
TestGetValueOkay(provider_->var_payload_size(), static_cast<size_t>(0));
}
TEST_F(PmRealUpdaterProviderTest, GetPayloadSizeOkayArbitrary) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<4>(static_cast<int64_t>(567890)),
Return(true)));
TestGetValueOkay(provider_->var_payload_size(), static_cast<size_t>(567890));
}
TEST_F(PmRealUpdaterProviderTest, GetPayloadSizeOkayTwoGigabytes) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<4>(static_cast<int64_t>(1) << 31),
Return(true)));
TestGetValueOkay(provider_->var_payload_size(), static_cast<size_t>(1) << 31);
}
TEST_F(PmRealUpdaterProviderTest, GetPayloadSizeFailNoValue) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(Return(false));
TestGetValueFail(provider_->var_payload_size());
}
TEST_F(PmRealUpdaterProviderTest, GetPayloadSizeFailNegative) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetStatus(_, _, _, _, _))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(),
+ GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<4>(static_cast<int64_t>(-1024)),
Return(true)));
TestGetValueFail(provider_->var_payload_size());
@@ -402,7 +430,7 @@
}
TEST_F(PmRealUpdaterProviderTest, GetUpdateCompletedTimeFailNoValue) {
- EXPECT_CALL(*mock_sys_state_.update_attempter(), GetBootTimeAtUpdate(_))
+ EXPECT_CALL(*mock_sys_state_.mock_update_attempter(), GetBootTimeAtUpdate(_))
.WillOnce(Return(false));
TestGetValueFail(provider_->var_update_completed_time());
}
diff --git a/policy_manager/state_factory.h b/policy_manager/state_factory.h
index 58e6813..4272258 100644
--- a/policy_manager/state_factory.h
+++ b/policy_manager/state_factory.h
@@ -5,6 +5,7 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_STATE_FACTORY_H_
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_STATE_FACTORY_H_
+#include "update_engine/dbus_wrapper_interface.h"
#include "update_engine/policy_manager/state.h"
#include "update_engine/system_state.h"
diff --git a/real_system_state.cc b/real_system_state.cc
index eb8a2ba..b06830f 100644
--- a/real_system_state.cc
+++ b/real_system_state.cc
@@ -5,7 +5,6 @@
#include "update_engine/real_system_state.h"
#include <base/file_util.h>
-#include <base/time/time.h>
#include "update_engine/constants.h"
#include "update_engine/policy_manager/state_factory.h"
@@ -16,8 +15,8 @@
RealSystemState::RealSystemState()
: device_policy_(nullptr),
connection_manager_(this),
+ update_attempter_(this, &dbus_),
request_params_(this),
- p2p_manager_(),
system_rebooted_(false) {}
bool RealSystemState::Initialize(bool enable_gpio) {
@@ -45,11 +44,16 @@
kMaxP2PFilesToKeep));
// Initialize the PolicyManager using the default State Factory.
- policy_manager_.Init(
- chromeos_policy_manager::DefaultStateFactory(&dbus_, this));
-
- if (!payload_state_.Initialize(this))
+ if (!policy_manager_.Init(
+ chromeos_policy_manager::DefaultStateFactory(&dbus_, this))) {
+ LOG(ERROR) << "Failed to initialize the policy manager.";
return false;
+ }
+
+ if (!payload_state_.Initialize(this)) {
+ LOG(ERROR) << "Failed to initialize the payload state object.";
+ return false;
+ }
// Initialize the GPIO handler as instructed.
if (enable_gpio) {
@@ -67,8 +71,8 @@
gpio_handler_.reset(new NoopGpioHandler(false));
}
- // Create the update attempter.
- update_attempter_.reset(new UpdateAttempter(this, &dbus_));
+ // Initialize the update attempter.
+ update_attempter_.Init();
// All is well. Initialization successful.
return true;
diff --git a/real_system_state.h b/real_system_state.h
index df1280f..b9c0a26 100644
--- a/real_system_state.h
+++ b/real_system_state.h
@@ -7,6 +7,9 @@
#include "update_engine/system_state.h"
+#include <policy/device_policy.h>
+
+#include "metrics/metrics_library.h"
#include "update_engine/clock.h"
#include "update_engine/connection_manager.h"
#include "update_engine/gpio_handler.h"
@@ -24,77 +27,76 @@
// used by the actual product code.
class RealSystemState : public SystemState {
public:
- // Constructors and destructors.
+ // Constructs all system objects that do not require separate initialization;
+ // see Initialize() below for the remaining ones.
RealSystemState();
- virtual ~RealSystemState() {}
+
+ // Initializes and sets systems objects that require an initialization
+ // separately from construction. Returns |true| on success.
+ bool Initialize(bool enable_gpio);
virtual inline void set_device_policy(
- const policy::DevicePolicy* device_policy) {
+ const policy::DevicePolicy* device_policy) override {
device_policy_ = device_policy;
}
- virtual inline const policy::DevicePolicy* device_policy() const {
+ virtual inline const policy::DevicePolicy* device_policy() override {
return device_policy_;
}
- virtual inline ClockInterface* clock() {
+ virtual inline ClockInterface* clock() override {
return &clock_;
}
- virtual inline ConnectionManager* connection_manager() {
+ virtual inline ConnectionManager* connection_manager() override {
return &connection_manager_;
}
- virtual inline HardwareInterface* hardware() {
+ virtual inline HardwareInterface* hardware() override {
return &hardware_;
}
- virtual inline MetricsLibraryInterface* metrics_lib() {
+ virtual inline MetricsLibraryInterface* metrics_lib() override {
return &metrics_lib_;
}
- virtual inline PrefsInterface* prefs() {
+ virtual inline PrefsInterface* prefs() override {
return &prefs_;
}
- virtual inline PrefsInterface* powerwash_safe_prefs() {
+ virtual inline PrefsInterface* powerwash_safe_prefs() override {
return &powerwash_safe_prefs_;
}
- virtual inline PayloadStateInterface* payload_state() {
+ virtual inline PayloadStateInterface* payload_state() override {
return &payload_state_;
}
- virtual inline GpioHandler* gpio_handler() const {
+ virtual inline GpioHandler* gpio_handler() override {
return gpio_handler_.get();
}
- virtual inline UpdateAttempter* update_attempter() const override {
- return update_attempter_.get();
+ virtual inline UpdateAttempter* update_attempter() override {
+ return &update_attempter_;
}
- // Returns a pointer to the object that stores the parameters that are
- // common to all Omaha requests.
- virtual inline OmahaRequestParams* request_params() {
+ virtual inline OmahaRequestParams* request_params() override {
return &request_params_;
}
- virtual inline P2PManager* p2p_manager() {
+ virtual inline P2PManager* p2p_manager() override {
return p2p_manager_.get();
}
- virtual inline chromeos_policy_manager::PolicyManager* policy_manager() {
+ virtual inline chromeos_policy_manager::PolicyManager* policy_manager()
+ override {
return &policy_manager_;
}
- virtual inline bool system_rebooted() {
+ virtual inline bool system_rebooted() override {
return system_rebooted_;
}
- // Initializes this concrete object. Other methods should be invoked only
- // if the object has been initialized successfully.
- bool Initialize(bool enable_gpio);
-
private:
// Interface for the clock.
Clock clock_;
@@ -133,7 +135,7 @@
RealDBusWrapper dbus_;
// Pointer to the update attempter object.
- scoped_ptr<UpdateAttempter> update_attempter_;
+ UpdateAttempter update_attempter_;
// Common parameters for all Omaha requests.
OmahaRequestParams request_params_;
diff --git a/system_state.h b/system_state.h
index 987cb6c..de098ee 100644
--- a/system_state.h
+++ b/system_state.h
@@ -5,12 +5,7 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_SYSTEM_STATE_H_
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_SYSTEM_STATE_H_
-#include "metrics/metrics_library.h"
-#include <policy/device_policy.h>
-#include <policy/libpolicy.h>
-
-#include "update_engine/gpio_handler.h"
-#include "update_engine/omaha_request_params.h"
+class MetricsLibraryInterface;
namespace chromeos_policy_manager {
@@ -18,6 +13,12 @@
} // namespace chromeos_policy_manager
+namespace policy {
+
+class DevicePolicy;
+
+} // namespace policy
+
namespace chromeos_update_engine {
// SystemState is the root class within the update engine. So we should avoid
@@ -27,6 +28,7 @@
class ConnectionManager;
class GpioHandler;
class HardwareInterface;
+class OmahaRequestParams;
class P2PManager;
class PayloadStateInterface;
class PrefsInterface;
@@ -46,7 +48,7 @@
// Sets or gets the latest device policy.
virtual void set_device_policy(const policy::DevicePolicy* device_policy) = 0;
- virtual const policy::DevicePolicy* device_policy() const = 0;
+ virtual const policy::DevicePolicy* device_policy() = 0;
// Gets the interface object for the clock.
virtual ClockInterface* clock() = 0;
@@ -73,10 +75,10 @@
virtual PayloadStateInterface* payload_state() = 0;
// Returns a pointer to the GPIO handler.
- virtual GpioHandler* gpio_handler() const = 0;
+ virtual GpioHandler* gpio_handler() = 0;
// Returns a pointer to the update attempter object.
- virtual UpdateAttempter* update_attempter() const = 0;
+ virtual UpdateAttempter* update_attempter() = 0;
// Returns a pointer to the object that stores the parameters that are
// common to all Omaha requests.
diff --git a/update_attempter.cc b/update_attempter.cc
index be76a8e..891cc96 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -120,47 +120,15 @@
UpdateAttempter::UpdateAttempter(SystemState* system_state,
DBusWrapperInterface* dbus_iface)
- : chrome_proxy_resolver_(dbus_iface) {
- Init(system_state, kUpdateCompletedMarker);
-}
+ : UpdateAttempter(system_state, dbus_iface, kUpdateCompletedMarker) {}
UpdateAttempter::UpdateAttempter(SystemState* system_state,
DBusWrapperInterface* dbus_iface,
const std::string& update_completed_marker)
- : chrome_proxy_resolver_(dbus_iface) {
- Init(system_state, update_completed_marker);
-}
-
-
-void UpdateAttempter::Init(SystemState* system_state,
- const std::string& update_completed_marker) {
- // Initialite data members.
- processor_.reset(new ActionProcessor());
- system_state_ = system_state;
- dbus_service_ = NULL;
- update_check_scheduler_ = NULL;
- fake_update_success_ = false;
- http_response_code_ = 0;
- shares_ = utils::kCpuSharesNormal;
- manage_shares_source_ = NULL;
- download_active_ = false;
- download_progress_ = 0.0;
- last_checked_time_ = 0;
- new_version_ = "0.0.0.0";
- new_payload_size_ = 0;
- proxy_manual_checks_ = 0;
- obeying_proxies_ = true;
- updated_boot_flags_ = false;
- update_boot_flags_running_ = false;
- start_action_processor_ = false;
- is_using_test_url_ = false;
- is_test_mode_ = false;
- is_test_update_attempted_ = false;
- update_completed_marker_ = update_completed_marker;
-
- prefs_ = system_state->prefs();
- omaha_request_params_ = system_state->request_params();
-
+ : processor_(new ActionProcessor()),
+ system_state_(system_state),
+ chrome_proxy_resolver_(dbus_iface),
+ update_completed_marker_(update_completed_marker) {
if (!update_completed_marker_.empty() &&
utils::FileExists(update_completed_marker_.c_str()))
status_ = UPDATE_STATUS_UPDATED_NEED_REBOOT;
@@ -168,6 +136,14 @@
status_ = UPDATE_STATUS_IDLE;
}
+void UpdateAttempter::Init() {
+ // Pulling from the SystemState can only be done after construction, since
+ // this is an aggregate of various objects (such as the UpdateAttempter),
+ // which requires them all to be constructed prior to it being used.
+ prefs_ = system_state_->prefs();
+ omaha_request_params_ = system_state_->request_params();
+}
+
UpdateAttempter::~UpdateAttempter() {
CleanupCpuSharesManagement();
}
diff --git a/update_attempter.h b/update_attempter.h
index 20b0173..311a918 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -63,6 +63,9 @@
DBusWrapperInterface* dbus_iface);
virtual ~UpdateAttempter();
+ // Further initialization to be done post construction.
+ void Init();
+
// Checks for update and, if a newer version is available, attempts to update
// the system. Non-empty |in_app_version| or |in_update_url| prevents
// automatic detection of the parameter. If |obey_proxies| is true, the
@@ -225,10 +228,6 @@
FRIEND_TEST(UpdateAttempterTest, ReportDailyMetrics);
FRIEND_TEST(UpdateAttempterTest, BootTimeInUpdateMarkerFile);
- // Ctor helper method.
- void Init(SystemState* system_state,
- const std::string& update_completed_marker);
-
// Checks if it's more than 24 hours since daily metrics were last
// reported and, if so, reports daily metrics. Returns |true| if
// metrics were reported, |false| otherwise.
@@ -368,7 +367,7 @@
// If non-null, this UpdateAttempter will send status updates over this
// dbus service.
- UpdateEngineService* dbus_service_;
+ UpdateEngineService* dbus_service_ = nullptr;
// Pointer to the OmahaResponseHandlerAction in the actions_ vector.
std::tr1::shared_ptr<OmahaResponseHandlerAction> response_handler_action_;
@@ -379,47 +378,47 @@
// Pointer to the preferences store interface. This is just a cached
// copy of system_state->prefs() because it's used in many methods and
// is convenient this way.
- PrefsInterface* prefs_;
+ PrefsInterface* prefs_ = nullptr;
// The current UpdateCheckScheduler to notify of state transitions.
- UpdateCheckScheduler* update_check_scheduler_;
+ UpdateCheckScheduler* update_check_scheduler_ = nullptr;
// Pending error event, if any.
scoped_ptr<OmahaEvent> error_event_;
// If we should request a reboot even tho we failed the update
- bool fake_update_success_;
+ bool fake_update_success_ = false;
// HTTP server response code from the last HTTP request action.
- int http_response_code_;
+ int http_response_code_ = 0;
// Current cpu shares.
- utils::CpuShares shares_;
+ utils::CpuShares shares_ = utils::kCpuSharesNormal;
// The cpu shares management timeout source.
- GSource* manage_shares_source_;
+ GSource* manage_shares_source_ = nullptr;
// Set to true if an update download is active (and BytesReceived
// will be called), set to false otherwise.
- bool download_active_;
+ bool download_active_ = false;
// For status:
UpdateStatus status_;
- double download_progress_;
- int64_t last_checked_time_;
+ double download_progress_ = 0.0;
+ int64_t last_checked_time_ = 0;
std::string prev_version_;
- std::string new_version_;
- int64_t new_payload_size_;
+ std::string new_version_ = "0.0.0.0";
+ int64_t new_payload_size_ = 0;
// Common parameters for all Omaha requests.
- OmahaRequestParams* omaha_request_params_;
+ OmahaRequestParams* omaha_request_params_ = nullptr;
// Number of consecutive manual update checks we've had where we obeyed
// Chrome's proxy settings.
- int proxy_manual_checks_;
+ int proxy_manual_checks_ = 0;
// If true, this update cycle we are obeying proxies
- bool obeying_proxies_;
+ bool obeying_proxies_ = true;
// Our two proxy resolvers
DirectProxyResolver direct_proxy_resolver_;
@@ -430,23 +429,26 @@
// completes its asynchronous run, |update_boot_flags_running_| is reset to
// false and |updated_boot_flags_| is set to true. From that point on there
// will be no more changes to these flags.
- bool updated_boot_flags_; // True if UpdateBootFlags has completed.
- bool update_boot_flags_running_; // True if UpdateBootFlags is running.
+ //
+ // True if UpdateBootFlags has completed.
+ bool updated_boot_flags_ = false;
+ // True if UpdateBootFlags is running.
+ bool update_boot_flags_running_ = false;
// True if the action processor needs to be started by the boot flag updater.
- bool start_action_processor_;
+ bool start_action_processor_ = false;
// Used for fetching information about the device policy.
scoped_ptr<policy::PolicyProvider> policy_provider_;
// A flag for indicating whether we are using a test server URL.
- bool is_using_test_url_;
+ bool is_using_test_url_ = false;
// If true, will induce a test mode update attempt.
- bool is_test_mode_;
+ bool is_test_mode_ = false;
// A flag indicating whether a test update cycle was already attempted.
- bool is_test_update_attempted_;
+ bool is_test_update_attempted_ = false;
// The current scatter factor as found in the policy setting.
base::TimeDelta scatter_factor_;
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 2148c06..82e96dd 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -47,14 +47,14 @@
// We always feed an explicit update completed marker name; however, unless
// explicitly specified, we feed an empty string, which causes the
// UpdateAttempter class to ignore / not write the marker file.
- UpdateAttempterUnderTest(MockSystemState* mock_system_state,
- MockDBusWrapper* dbus)
- : UpdateAttempter(mock_system_state, dbus, "") {}
+ UpdateAttempterUnderTest(SystemState* system_state,
+ DBusWrapperInterface* dbus_iface)
+ : UpdateAttempterUnderTest(system_state, dbus_iface, "") {}
- UpdateAttempterUnderTest(MockSystemState* mock_system_state,
- MockDBusWrapper* dbus,
- const string& update_completed_marker)
- : UpdateAttempter(mock_system_state, dbus, update_completed_marker) {}
+ UpdateAttempterUnderTest(SystemState* system_state,
+ DBusWrapperInterface* dbus_iface,
+ const std::string& update_completed_marker)
+ : UpdateAttempter(system_state, dbus_iface, update_completed_marker) {}
};
class UpdateAttempterTest : public ::testing::Test {
@@ -63,7 +63,12 @@
: attempter_(&mock_system_state_, &dbus_),
mock_connection_manager(&mock_system_state_),
loop_(NULL) {
+ // Override system state members.
mock_system_state_.set_connection_manager(&mock_connection_manager);
+ mock_system_state_.set_update_attempter(&attempter_);
+
+ // Finish initializing the attempter.
+ attempter_.Init();
}
virtual void SetUp() {
@@ -145,7 +150,7 @@
void P2PEnabledHousekeepingFailsStart();
static gboolean StaticP2PEnabledHousekeepingFails(gpointer data);
- NiceMock<MockSystemState> mock_system_state_;
+ MockSystemState mock_system_state_;
NiceMock<MockDBusWrapper> dbus_;
UpdateAttempterUnderTest attempter_;
NiceMock<ActionProcessorMock>* processor_;
@@ -464,14 +469,13 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
if (!valid_slot) {
// References bootable kernels in fake_hardware.h
string rollback_kernel = "/dev/sdz2";
LOG(INFO) << "Test Mark Unbootable: " << rollback_kernel;
- mock_system_state_.get_fake_hardware()->MarkKernelUnbootable(
+ mock_system_state_.fake_hardware()->MarkKernelUnbootable(
rollback_kernel);
}
@@ -629,8 +633,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetReleaseChannelDelegated(_)).WillRepeatedly(
DoAll(SetArgumentPointee<0>(bool(false)),
@@ -665,8 +668,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetUpdateDisabled(_))
.WillRepeatedly(DoAll(
@@ -858,8 +860,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetTargetVersionPrefix(_))
.WillRepeatedly(DoAll(
@@ -891,8 +892,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
.WillRepeatedly(DoAll(
@@ -920,7 +920,7 @@
Prefs prefs;
attempter_.prefs_ = &prefs;
- mock_system_state_.get_fake_hardware()->SetIsOOBEComplete(Time::UnixEpoch());
+ mock_system_state_.fake_hardware()->SetIsOOBEComplete(Time::UnixEpoch());
string prefs_dir;
EXPECT_TRUE(utils::MakeTempDirectory("ue_ut_prefs.XXXXXX",
@@ -937,8 +937,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
.WillRepeatedly(DoAll(
@@ -984,7 +983,7 @@
Prefs prefs;
attempter_.prefs_ = &prefs;
- mock_system_state_.get_fake_hardware()->SetIsOOBEComplete(Time::UnixEpoch());
+ mock_system_state_.fake_hardware()->SetIsOOBEComplete(Time::UnixEpoch());
string prefs_dir;
EXPECT_TRUE(utils::MakeTempDirectory("ue_ut_prefs.XXXXXX",
@@ -1004,8 +1003,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
.WillRepeatedly(DoAll(
diff --git a/update_check_scheduler_unittest.cc b/update_check_scheduler_unittest.cc
index 0eed080..d4a5a1e 100644
--- a/update_check_scheduler_unittest.cc
+++ b/update_check_scheduler_unittest.cc
@@ -171,7 +171,7 @@
TEST_F(UpdateCheckSchedulerTest, RunBootDeviceRemovableTest) {
scheduler_.enabled_ = true;
- mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(true);
+ mock_system_state_.fake_hardware()->SetIsOfficialBuild(true);
EXPECT_CALL(scheduler_, IsBootDeviceRemovable())
.Times(1)
.WillOnce(Return(true));
@@ -182,7 +182,7 @@
TEST_F(UpdateCheckSchedulerTest, RunNonOfficialBuildTest) {
scheduler_.enabled_ = true;
- mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(false);
+ mock_system_state_.fake_hardware()->SetIsOfficialBuild(false);
scheduler_.Run();
EXPECT_FALSE(scheduler_.enabled_);
EXPECT_EQ(NULL, attempter_.update_check_scheduler());
@@ -194,7 +194,7 @@
UpdateCheckScheduler::kTimeoutRegularFuzz,
&interval_min,
&interval_max);
- mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(true);
+ mock_system_state_.fake_hardware()->SetIsOfficialBuild(true);
EXPECT_CALL(scheduler_, IsBootDeviceRemovable())
.Times(1)
.WillOnce(Return(false));
@@ -281,7 +281,7 @@
TEST_F(UpdateCheckSchedulerTest, StaticCheckOOBECompleteTest) {
scheduler_.scheduled_ = true;
EXPECT_TRUE(scheduler_.mock_system_state_ != NULL);
- scheduler_.mock_system_state_->get_fake_hardware()->SetIsOOBEComplete(
+ scheduler_.mock_system_state_->fake_hardware()->SetIsOOBEComplete(
Time::UnixEpoch());
EXPECT_CALL(attempter_, Update("", "", false, false, false))
.Times(1)
@@ -293,7 +293,7 @@
TEST_F(UpdateCheckSchedulerTest, StaticCheckOOBENotCompleteTest) {
scheduler_.scheduled_ = true;
- scheduler_.mock_system_state_->get_fake_hardware()->UnsetIsOOBEComplete();
+ scheduler_.mock_system_state_->fake_hardware()->UnsetIsOOBEComplete();
EXPECT_CALL(attempter_, Update("", "", _, _, _)).Times(0);
int interval_min, interval_max;
FuzzRange(UpdateCheckScheduler::kTimeoutInitialInterval,
