dbus: perform D-Bus initialization in DBusControl

This moves the RPC specific initialization out of the Daemon class,
which makes it agnostic of the underlying RPC implementation.

Also move the ownership of Manager from Daemon to the ControlInterface
implementation. So the ControlInterface is now responsible for
creating Manager and exposing it through the underlying RPC interface.
This makes it easier to maintain the relevant objects (e.g. Manager,
ObjectManager), and avoids possible confusion during cleanup.

Bug: 25654681
TEST=Verify device setup works on Brillo device

Change-Id: Id2eb8d08346841e294f8286fe191708caa2cbc24

7 files changed, 90 insertions, 43 deletions

diff --git a/control_interface.h b/control_interface.h
index e5bbfd0..4661eca 100644
--- a/control_interface.h
+++ b/control_interface.h
@@ -31,6 +31,9 @@ class ControlInterface {
  public:
   virtual ~ControlInterface() {}
 
+  virtual void Init() = 0;
+  virtual void Shutdown() = 0;
+
   // Proxy creation APIs.
   virtual std::unique_ptr<FirewallProxyInterface> CreateFirewallProxy(
       const base::Closure& service_appeared_callback,
diff --git a/daemon.cc b/daemon.cc
index 696cea2..5980a77 100644
--- a/daemon.cc
+++ b/daemon.cc
@@ -19,18 +19,11 @@
 #include <sysexits.h>
 
 #include <base/logging.h>
-#include <base/message_loop/message_loop_proxy.h>
-#include <base/run_loop.h>
 
 #include "apmanager/dbus/dbus_control.h"
 
 namespace apmanager {
 
-namespace {
-const char kAPManagerServiceName[] = "org.chromium.apmanager";
-const char kAPMRootServicePath[] = "/org/chromium/apmanager";
-}  // namespace
-
 // static
 #if !defined(__ANDROID__)
 const char Daemon::kAPManagerGroupName[] = "apmanager";
@@ -41,40 +34,28 @@ const char Daemon::kAPManagerUserName[] = "system";
 #endif  // __ANDROID__
 
 Daemon::Daemon(const base::Closure& startup_callback)
-    : DBusServiceDaemon(kAPManagerServiceName, kAPMRootServicePath),
-      startup_callback_(startup_callback) {
+    : startup_callback_(startup_callback) {
 }
 
 int Daemon::OnInit() {
-  int return_code = brillo::DBusServiceDaemon::OnInit();
+  int return_code = brillo::Daemon::OnInit();
   if (return_code != EX_OK) {
     return return_code;
   }
 
+  // Setup control interface. The control interface will expose
+  // our service (Manager) through its RPC interface.
+  control_interface_.reset(new DBusControl());
+  control_interface_->Init();
+
   // Signal that we've acquired all resources.
   startup_callback_.Run();
 
-  // Start manager.
-  manager_->Start();
-
   return EX_OK;
 }
 
 void Daemon::OnShutdown(int* return_code) {
-  manager_.reset();
-  brillo::DBusServiceDaemon::OnShutdown(return_code);
-}
-
-void Daemon::RegisterDBusObjectsAsync(
-    brillo::dbus_utils::AsyncEventSequencer* sequencer) {
-  control_interface_.reset(new DBusControl(bus_));
-  manager_.reset(new apmanager::Manager());
-  // TODO(zqiu): remove object_manager_ and bus_ references once we moved over
-  // to use control interface for creating adaptors.
-  manager_->RegisterAsync(control_interface_.get(),
-                          object_manager_.get(),
-                          bus_,
-                          sequencer);
+  control_interface_->Shutdown();
 }
 
 }  // namespace apmanager
diff --git a/daemon.h b/daemon.h
index 38869fd..37e9b8d 100644
--- a/daemon.h
+++ b/daemon.h
@@ -18,14 +18,13 @@
 #define APMANAGER_DAEMON_H_
 
 #include <base/callback_forward.h>
-#include <brillo/daemons/dbus_daemon.h>
+#include <brillo/daemons/daemon.h>
 
 #include "apmanager/control_interface.h"
-#include "apmanager/manager.h"
 
 namespace apmanager {
 
-class Daemon : public brillo::DBusServiceDaemon {
+class Daemon : public brillo::Daemon {
  public:
   // User and group to run the apmanager process.
   static const char kAPManagerGroupName[];
@@ -37,14 +36,11 @@ class Daemon : public brillo::DBusServiceDaemon {
  protected:
   int OnInit() override;
   void OnShutdown(int* return_code) override;
-  void RegisterDBusObjectsAsync(
-      brillo::dbus_utils::AsyncEventSequencer* sequencer) override;
 
  private:
   friend class DaemonTest;
 
   std::unique_ptr<ControlInterface> control_interface_;
-  std::unique_ptr<Manager> manager_;
   base::Closure startup_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(Daemon);
diff --git a/dbus/dbus_control.cc b/dbus/dbus_control.cc
index 62e1d58..ed96661 100644
--- a/dbus/dbus_control.cc
+++ b/dbus/dbus_control.cc
@@ -17,6 +17,7 @@
 #include "apmanager/dbus/dbus_control.h"
 
 #include "apmanager/dbus/shill_dbus_proxy.h"
+#include "apmanager/manager.h"
 
 #if !defined(__ANDROID__)
 #include "apmanager/dbus/permission_broker_dbus_proxy.h"
@@ -24,13 +25,68 @@
 #include "apmanager/dbus/firewalld_dbus_proxy.h"
 #endif  //__ANDROID__
 
+using brillo::dbus_utils::AsyncEventSequencer;
+using brillo::dbus_utils::ExportedObjectManager;
+
 namespace apmanager {
 
-DBusControl::DBusControl(const scoped_refptr<dbus::Bus>& bus)
-    : bus_(bus) {}
+namespace {
+const char kServiceName[] = "org.chromium.apmanager";
+const char kServicePath[] = "/org/chromium/apmanager";
+}  // namespace
+
+DBusControl::DBusControl() {}
 
 DBusControl::~DBusControl() {}
 
+void DBusControl::Init() {
+  // Setup bus connection.
+  dbus::Bus::Options options;
+  options.bus_type = dbus::Bus::SYSTEM;
+  bus_ = new dbus::Bus(options);
+  CHECK(bus_->Connect());
+
+  // Create and register ObjectManager.
+  scoped_refptr<AsyncEventSequencer> sequencer(new AsyncEventSequencer());
+  object_manager_.reset(
+      new ExportedObjectManager(bus_, dbus::ObjectPath(kServicePath)));
+  object_manager_->RegisterAsync(
+      sequencer->GetHandler("ObjectManager.RegisterAsync() failed.", true));
+
+  // Create and register Manager.
+  manager_.reset(new Manager());
+  manager_->RegisterAsync(
+      this,
+      object_manager_.get(),
+      bus_,
+      sequencer->GetHandler("Manager.RegisterAsync() failed.", true));
+
+  // Take over the service ownership once the objects registration is completed.
+  sequencer->OnAllTasksCompletedCall({
+    base::Bind(&DBusControl::OnObjectRegistrationCompleted,
+               base::Unretained(this))
+  });
+}
+
+void DBusControl::Shutdown() {
+  manager_.reset();
+  object_manager_.reset();
+  if (bus_) {
+    bus_->ShutdownAndBlock();
+  }
+}
+
+void DBusControl::OnObjectRegistrationCompleted(bool registration_success) {
+  // Success should always be true since we've said that failures are fatal.
+  CHECK(registration_success) << "Init of one or more objects has failed.";
+  CHECK(bus_->RequestOwnershipAndBlock(kServiceName,
+                                       dbus::Bus::REQUIRE_PRIMARY))
+      << "Unable to take ownership of " << kServiceName;
+
+  // D-Bus service is ready, now we can start the Manager.
+  manager_->Start();
+}
+
 std::unique_ptr<FirewallProxyInterface> DBusControl::CreateFirewallProxy(
     const base::Closure& service_appeared_callback,
     const base::Closure& service_vanished_callback) {
diff --git a/dbus/dbus_control.h b/dbus/dbus_control.h
index e1e775e..0efdc08 100644
--- a/dbus/dbus_control.h
+++ b/dbus/dbus_control.h
@@ -18,19 +18,23 @@
 #define APMANAGER_DBUS_DBUS_CONTROL_H_
 
 #include <base/macros.h>
+#include <brillo/dbus/exported_object_manager.h>
 #include <dbus/bus.h>
 
 #include "apmanager/control_interface.h"
+#include "apmanager/manager.h"
 
 namespace apmanager {
 
 // D-Bus control interface for IPC through D-Bus.
 class DBusControl : public ControlInterface {
  public:
-  DBusControl(const scoped_refptr<dbus::Bus>& bus);
+  DBusControl();
   ~DBusControl() override;
 
   // Inheritted from ControlInterface.
+  void Init() override;
+  void Shutdown() override;
   std::unique_ptr<FirewallProxyInterface> CreateFirewallProxy(
       const base::Closure& service_appeared_callback,
       const base::Closure& service_vanished_callback) override;
@@ -39,11 +43,18 @@ class DBusControl : public ControlInterface {
       const base::Closure& service_vanished_callback) override;
 
  private:
+  // Invoked when D-Bus objects for both ObjectManager and Manager
+  // are registered to the bus.
+  void OnObjectRegistrationCompleted(bool registration_success);
+
   // NOTE: No dedicated bus is needed for the proxies, since the proxies
   // being created here doesn't listen for any broadcast signals.
   // Use a dedicated bus for the proxies if this condition is not true
   // anymore.
   scoped_refptr<dbus::Bus> bus_;
+  std::unique_ptr<brillo::dbus_utils::ExportedObjectManager> object_manager_;
+  std::unique_ptr<Manager> manager_;
+
   DISALLOW_COPY_AND_ASSIGN(DBusControl);
 };
 
diff --git a/manager.cc b/manager.cc
index 2cebb74..a288141 100644
--- a/manager.cc
+++ b/manager.cc
@@ -44,10 +44,11 @@ Manager::~Manager() {
   }
 }
 
-void Manager::RegisterAsync(ControlInterface* control_interface,
-                            ExportedObjectManager* object_manager,
-                            const scoped_refptr<dbus::Bus>& bus,
-                            AsyncEventSequencer* sequencer) {
+void Manager::RegisterAsync(
+    ControlInterface* control_interface,
+    ExportedObjectManager* object_manager,
+    const scoped_refptr<dbus::Bus>& bus,
+    const base::Callback<void(bool)>& completion_callback) {
   CHECK(!dbus_object_) << "Already registered";
   dbus_object_.reset(
       new brillo::dbus_utils::DBusObject(
@@ -55,8 +56,7 @@ void Manager::RegisterAsync(ControlInterface* control_interface,
           bus,
           org::chromium::apmanager::ManagerAdaptor::GetObjectPath()));
   RegisterWithDBusObject(dbus_object_.get());
-  dbus_object_->RegisterAsync(
-      sequencer->GetHandler("Manager.RegisterAsync() failed.", true));
+  dbus_object_->RegisterAsync(completion_callback);
   bus_ = bus;
 
   shill_manager_.Init(control_interface);
diff --git a/manager.h b/manager.h
index c30e60b..ad38a07 100644
--- a/manager.h
+++ b/manager.h
@@ -60,7 +60,7 @@ class Manager : public org::chromium::apmanager::ManagerAdaptor,
       ControlInterface* control_interface,
       brillo::dbus_utils::ExportedObjectManager* object_manager,
       const scoped_refptr<dbus::Bus>& bus,
-      brillo::dbus_utils::AsyncEventSequencer* sequencer);
+      const base::Callback<void(bool)>& completion_callback);
 
   virtual void Start();
   virtual void Stop();