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());
 }