Make UpdateAttempter own debugd_proxy.
It's the only class using this proxy.
Test: mma
Bug: 28800946
Change-Id: I1a98b417f213db5d47de451390367ca4975db4b0
diff --git a/fake_system_state.cc b/fake_system_state.cc
index 49ba058..d51f775 100644
--- a/fake_system_state.cc
+++ b/fake_system_state.cc
@@ -21,7 +21,7 @@
// Mock the SystemStateInterface so that we could lie that
// OOBE is completed even when there's no such marker file, etc.
FakeSystemState::FakeSystemState()
- : mock_update_attempter_(this, nullptr, nullptr, nullptr),
+ : mock_update_attempter_(this, nullptr, nullptr),
mock_request_params_(this),
fake_update_manager_(&fake_clock_),
clock_(&fake_clock_),
diff --git a/real_system_state.cc b/real_system_state.cc
index 16d8876..70bef8c 100644
--- a/real_system_state.cc
+++ b/real_system_state.cc
@@ -37,9 +37,6 @@
namespace chromeos_update_engine {
-RealSystemState::RealSystemState(const scoped_refptr<dbus::Bus>& bus)
- : debugd_proxy_(bus) {}
-
RealSystemState::~RealSystemState() {
// Prevent any DBus communication from UpdateAttempter when shutting down the
// daemon.
@@ -134,8 +131,7 @@
// Initialize the UpdateAttempter before the UpdateManager.
update_attempter_.reset(
- new UpdateAttempter(this, certificate_checker_.get(), &libcros_proxy_,
- &debugd_proxy_));
+ new UpdateAttempter(this, certificate_checker_.get(), &libcros_proxy_));
update_attempter_->Init();
weave_service_ = ConstructWeaveService(update_attempter_.get());
diff --git a/real_system_state.h b/real_system_state.h
index 65175ae..18a0d98 100644
--- a/real_system_state.h
+++ b/real_system_state.h
@@ -21,7 +21,6 @@
#include <memory>
-#include <debugd/dbus-proxies.h>
#include <metrics/metrics_library.h>
#include <policy/device_policy.h>
@@ -32,7 +31,6 @@
#include "update_engine/common/prefs.h"
#include "update_engine/connection_manager_interface.h"
#include "update_engine/daemon_state_interface.h"
-#include "update_engine/dbus_connection.h"
#include "update_engine/p2p_manager.h"
#include "update_engine/payload_state.h"
#include "update_engine/power_manager_interface.h"
@@ -48,9 +46,7 @@
public:
// Constructs all system objects that do not require separate initialization;
// see Initialize() below for the remaining ones.
- // TODO(senj): Remove this constructor once all proxies get DBus themselves.
- explicit RealSystemState(const scoped_refptr<dbus::Bus>& bus);
- RealSystemState() : RealSystemState(DBusConnection::Get()->GetDBus()){};
+ RealSystemState() = default;
~RealSystemState() override;
// Initializes and sets systems objects that require an initialization
@@ -127,7 +123,6 @@
private:
// Real DBus proxies using the DBus connection.
- org::chromium::debugdProxy debugd_proxy_;
LibCrosProxy libcros_proxy_;
// Interface for the power manager.
diff --git a/update_attempter.cc b/update_attempter.cc
index 57a9074..11c736e 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -50,7 +50,7 @@
#include "update_engine/common/prefs_interface.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/utils.h"
-#include "update_engine/dbus_service.h"
+#include "update_engine/dbus_connection.h"
#include "update_engine/metrics.h"
#include "update_engine/omaha_request_action.h"
#include "update_engine/omaha_request_params.h"
@@ -120,18 +120,17 @@
return code;
}
-UpdateAttempter::UpdateAttempter(
- SystemState* system_state,
- CertificateChecker* cert_checker,
- LibCrosProxy* libcros_proxy,
- org::chromium::debugdProxyInterface* debugd_proxy)
+UpdateAttempter::UpdateAttempter(SystemState* system_state,
+ CertificateChecker* cert_checker,
+ LibCrosProxy* libcros_proxy)
: processor_(new ActionProcessor()),
system_state_(system_state),
- cert_checker_(cert_checker),
#if USE_LIBCROS
- chrome_proxy_resolver_(libcros_proxy),
+ cert_checker_(cert_checker),
+ chrome_proxy_resolver_(libcros_proxy) {
+#else
+ cert_checker_(cert_checker) {
#endif // USE_LIBCROS
- debugd_proxy_(debugd_proxy) {
}
UpdateAttempter::~UpdateAttempter() {
@@ -163,6 +162,11 @@
#if USE_LIBCROS
chrome_proxy_resolver_.Init();
#endif // USE_LIBCROS
+
+ // unittest can set this to a mock before calling Init().
+ if (!debugd_proxy_)
+ debugd_proxy_.reset(
+ new org::chromium::debugdProxy(DBusConnection::Get()->GetDBus()));
}
void UpdateAttempter::ScheduleUpdates() {
diff --git a/update_attempter.h b/update_attempter.h
index 58a41d3..5dbe3dd 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -68,8 +68,7 @@
UpdateAttempter(SystemState* system_state,
CertificateChecker* cert_checker,
- LibCrosProxy* libcros_proxy,
- org::chromium::debugdProxyInterface* debugd_proxy);
+ LibCrosProxy* libcros_proxy);
~UpdateAttempter() override;
// Further initialization to be done post construction.
@@ -507,7 +506,7 @@
std::string forced_app_version_;
std::string forced_omaha_url_;
- org::chromium::debugdProxyInterface* debugd_proxy_;
+ std::unique_ptr<org::chromium::debugdProxyInterface> debugd_proxy_;
DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
};
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index b9a60b3..93ca81f 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -80,9 +80,8 @@
class UpdateAttempterUnderTest : public UpdateAttempter {
public:
UpdateAttempterUnderTest(SystemState* system_state,
- LibCrosProxy* libcros_proxy,
- org::chromium::debugdProxyInterface* debugd_proxy)
- : UpdateAttempter(system_state, nullptr, libcros_proxy, debugd_proxy) {}
+ LibCrosProxy* libcros_proxy)
+ : UpdateAttempter(system_state, nullptr, libcros_proxy) {}
// Wrap the update scheduling method, allowing us to opt out of scheduled
// updates for testing purposes.
@@ -111,7 +110,8 @@
class UpdateAttempterTest : public ::testing::Test {
protected:
UpdateAttempterTest()
- : service_interface_mock_(new LibCrosServiceInterfaceProxyMock()),
+ : debugd_proxy_mock_(new org::chromium::debugdProxyMock()),
+ service_interface_mock_(new LibCrosServiceInterfaceProxyMock()),
ue_proxy_resolved_interface_mock_(
new NiceMock<UpdateEngineLibcrosProxyResolvedInterfaceProxyMock>()),
libcros_proxy_(
@@ -127,6 +127,7 @@
certificate_checker_.Init();
// Finish initializing the attempter.
+ attempter_.debugd_proxy_.reset(debugd_proxy_mock_);
attempter_.Init();
}
@@ -188,16 +189,14 @@
brillo::BaseMessageLoop loop_{&base_loop_};
FakeSystemState fake_system_state_;
- org::chromium::debugdProxyMock debugd_proxy_mock_;
+ org::chromium::debugdProxyMock* debugd_proxy_mock_;
LibCrosServiceInterfaceProxyMock* service_interface_mock_;
UpdateEngineLibcrosProxyResolvedInterfaceProxyMock*
ue_proxy_resolved_interface_mock_;
LibCrosProxy libcros_proxy_;
OpenSSLWrapper openssl_wrapper_;
CertificateChecker certificate_checker_;
- UpdateAttempterUnderTest attempter_{&fake_system_state_,
- &libcros_proxy_,
- &debugd_proxy_mock_};
+ UpdateAttempterUnderTest attempter_{&fake_system_state_, &libcros_proxy_};
NiceMock<MockActionProcessor>* processor_;
NiceMock<MockPrefs>* prefs_; // Shortcut to fake_system_state_->mock_prefs().
@@ -256,10 +255,8 @@
EXPECT_TRUE(utils::GetBootId(&boot_id));
fake_prefs.SetString(kPrefsUpdateCompletedOnBootId, boot_id);
fake_system_state_.set_prefs(&fake_prefs);
- UpdateAttempterUnderTest attempter(&fake_system_state_, &libcros_proxy_,
- &debugd_proxy_mock_);
- attempter.Init();
- EXPECT_EQ(UpdateStatus::UPDATED_NEED_REBOOT, attempter.status());
+ attempter_.Init();
+ EXPECT_EQ(UpdateStatus::UPDATED_NEED_REBOOT, attempter_.status());
}
TEST_F(UpdateAttempterTest, GetErrorCodeForActionTest) {
@@ -925,22 +922,19 @@
}
TEST_F(UpdateAttempterTest, BootTimeInUpdateMarkerFile) {
- UpdateAttempterUnderTest attempter{&fake_system_state_,
- &libcros_proxy_,
- &debugd_proxy_mock_};
FakeClock fake_clock;
fake_clock.SetBootTime(Time::FromTimeT(42));
fake_system_state_.set_clock(&fake_clock);
FakePrefs fake_prefs;
fake_system_state_.set_prefs(&fake_prefs);
- attempter.Init();
+ attempter_.Init();
Time boot_time;
- EXPECT_FALSE(attempter.GetBootTimeAtUpdate(&boot_time));
+ EXPECT_FALSE(attempter_.GetBootTimeAtUpdate(&boot_time));
- attempter.WriteUpdateCompletedMarker();
+ attempter_.WriteUpdateCompletedMarker();
- EXPECT_TRUE(attempter.GetBootTimeAtUpdate(&boot_time));
+ EXPECT_TRUE(attempter_.GetBootTimeAtUpdate(&boot_time));
EXPECT_EQ(boot_time.ToTimeT(), 42);
}
@@ -952,7 +946,7 @@
TEST_F(UpdateAttempterTest, AnyUpdateSourceAllowedOfficialDevmode) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
- EXPECT_CALL(debugd_proxy_mock_, QueryDevFeatures(_, _, _))
+ EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _))
.WillRepeatedly(DoAll(SetArgumentPointee<0>(0), Return(true)));
EXPECT_TRUE(attempter_.IsAnyUpdateSourceAllowed());
}
@@ -961,7 +955,7 @@
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetIsNormalBootMode(true);
// debugd should not be queried in this case.
- EXPECT_CALL(debugd_proxy_mock_, QueryDevFeatures(_, _, _)).Times(0);
+ EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _)).Times(0);
EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
}
@@ -969,7 +963,7 @@
using debugd::DEV_FEATURES_DISABLED;
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
- EXPECT_CALL(debugd_proxy_mock_, QueryDevFeatures(_, _, _))
+ EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _))
.WillRepeatedly(
DoAll(SetArgumentPointee<0>(DEV_FEATURES_DISABLED), Return(true)));
EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
@@ -978,7 +972,7 @@
TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedDebugdFailure) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
- EXPECT_CALL(debugd_proxy_mock_, QueryDevFeatures(_, _, _))
+ EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _))
.WillRepeatedly(Return(false));
EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
}