Enhance the DBus interface for peerd

chromeos/dbus/peer_daemon_manager_client.cc

Index: chromeos/dbus/peer_daemon_manager_client.cc
diff --git a/chromeos/dbus/peer_daemon_manager_client.cc b/chromeos/dbus/peer_daemon_manager_client.cc
index b2ba214a23630632f274b48bac508c7769677022..0a12d563969ed2c26c8c61ac21e9d4991fbfdf2f 100644
--- a/chromeos/dbus/peer_daemon_manager_client.cc
+++ b/chromeos/dbus/peer_daemon_manager_client.cc
@@ -8,8 +8,11 @@
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/observer_list.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
+#include "dbus/object_manager.h"
 #include "dbus/object_proxy.h"
 #include "dbus/values_util.h"
 
@@ -19,8 +22,11 @@ namespace {
 // TODO(benchan): Move these constants to system_api.
 namespace peerd {
 const char kPeerdServiceName[] = "org.chromium.peerd";
+const char kPeerdObjectManagerServicePath[] = "/org/chromium/peerd";
 const char kPeerdManagerPath[] = "/org/chromium/peerd/Manager";
 const char kManagerInterface[] = "org.chromium.peerd.Manager";
+const char kServiceInterface[] = "org.chromium.peerd.Service";
+const char kPeerInterface[] = "org.chromium.peerd.Peer";
 const char kStartMonitoringMethod[] = "StartMonitoring";
 const char kStopMonitoringMethod[] = "StopMonitoring";
 const char kExposeServiceMethod[] = "ExposeService";
@@ -29,15 +35,24 @@ const char kPingMethod[] = "Ping";
 }  // namespace peerd
 
 // The PeerDaemonManagerClient implementation used in production.
-class PeerDaemonManagerClientImpl : public PeerDaemonManagerClient {
+class PeerDaemonManagerClientImpl : public PeerDaemonManagerClient,
+                                    public dbus::ObjectManager::Interface {
  public:
   PeerDaemonManagerClientImpl();
   ~PeerDaemonManagerClientImpl() override;
 
-  // DBusClient overrides:
+  // DBusClient overrides.
   void Init(dbus::Bus* bus) override;
 
-  // PeerDaemonManagerClient overrides:
+  // PeerDaemonManagerClient overrides.
+  void AddObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
+  std::vector<dbus::ObjectPath> GetPeers() override;
+  std::vector<dbus::ObjectPath> GetServices() override;
+  PeerProperties* GetPeerProperties(
+      const dbus::ObjectPath& object_path) override;
+  ServiceProperties* GetServiceProperties(
+      const dbus::ObjectPath& object_path) override;

[hashimoto, 2015/02/06 07:45:11]

nit: Please put these overrides in the same order as declared in the header.

[dtapuska, 2015/02/06 15:56:00]

Done... Manager, Service, Peer should be the order everywhere now.

   void StartMonitoring(
       const std::vector<std::string>& requested_technologies,
       const base::DictionaryValue& options,
@@ -53,55 +68,126 @@ class PeerDaemonManagerClientImpl : public PeerDaemonManagerClient {
                             const VoidDBusMethodCallback& callback) override;
   void Ping(const StringDBusMethodCallback& callback) override;
 
+  // dbus::ObjectManager::Interface overrides.
+  dbus::PropertySet* CreateProperties(
+      dbus::ObjectProxy* object_proxy,
+      const dbus::ObjectPath& object_path,
+      const std::string& interface_name) override;
+  void ObjectAdded(const dbus::ObjectPath& object_path,
+                   const std::string& interface_name) override;
+  void ObjectRemoved(const dbus::ObjectPath& object_path,
+                     const std::string& interface_name) override;
+
  private:
   void OnStringDBusMethod(const StringDBusMethodCallback& callback,
                           dbus::Response* response);
   void OnVoidDBusMethod(const VoidDBusMethodCallback& callback,
                         dbus::Response* response);
+  void OnManagerPropertyChanged(const std::string& property_name);
+  void OnPeerPropertyChanged(const dbus::ObjectPath& object_path,

[hashimoto, 2015/02/06 07:45:11]

nit: Please stick exclusively to one ordering, Manager->Peer->Service, or
Manager->Service->Peer to make the code intuitively understandable.
The same goes for everywhere.

[dtapuska, 2015/02/06 15:56:00]

Done.

+                             const std::string& property_name);
+  void OnServicePropertyChanged(const dbus::ObjectPath& object_path,
+                                const std::string& property_name);
+
+  // List of observers interested in event notifications from us.
+  ObserverList<Observer> observers_;
+  dbus::ObjectManager* object_manager_;
 
-  dbus::ObjectProxy* peer_daemon_proxy_;
   base::WeakPtrFactory<PeerDaemonManagerClientImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PeerDaemonManagerClientImpl);
 };
 
 PeerDaemonManagerClientImpl::PeerDaemonManagerClientImpl()
-    : peer_daemon_proxy_(nullptr), weak_ptr_factory_(this) {
+    : object_manager_(nullptr), weak_ptr_factory_(this) {
 }
 
 PeerDaemonManagerClientImpl::~PeerDaemonManagerClientImpl() {
+  if (object_manager_) {
+    object_manager_->UnregisterInterface(peerd::kManagerInterface);
+    object_manager_->UnregisterInterface(peerd::kPeerInterface);
+    object_manager_->UnregisterInterface(peerd::kServiceInterface);
+  }
+}
+
+void PeerDaemonManagerClientImpl::AddObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.AddObserver(observer);
+}
+
+void PeerDaemonManagerClientImpl::RemoveObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.RemoveObserver(observer);
+}
+
+std::vector<dbus::ObjectPath> PeerDaemonManagerClientImpl::GetPeers() {
+  return object_manager_->GetObjectsWithInterface(peerd::kPeerInterface);
+}
+
+std::vector<dbus::ObjectPath> PeerDaemonManagerClientImpl::GetServices() {
+  return object_manager_->GetObjectsWithInterface(peerd::kServiceInterface);
+}
+
+PeerDaemonManagerClient::PeerProperties*
+PeerDaemonManagerClientImpl::GetPeerProperties(
+    const dbus::ObjectPath& object_path) {
+  return static_cast<PeerProperties*>(
+      object_manager_->GetProperties(object_path, peerd::kPeerInterface));
+}
+
+PeerDaemonManagerClient::ServiceProperties*
+PeerDaemonManagerClientImpl::GetServiceProperties(
+    const dbus::ObjectPath& object_path) {
+  return static_cast<ServiceProperties*>(
+      object_manager_->GetProperties(object_path, peerd::kServiceInterface));
 }
 
 void PeerDaemonManagerClientImpl::StartMonitoring(
     const std::vector<std::string>& requested_technologies,
     const base::DictionaryValue& options,
     const StringDBusMethodCallback& callback) {
+  dbus::ObjectProxy* object_proxy = object_manager_->GetObjectProxy(
+      dbus::ObjectPath(peerd::kPeerdManagerPath));
+  if (!object_proxy) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&PeerDaemonManagerClientImpl::OnStringDBusMethod,
+                   weak_ptr_factory_.GetWeakPtr(), callback, nullptr));
+    return;
+  }
+
   dbus::MethodCall method_call(peerd::kManagerInterface,
                                peerd::kStartMonitoringMethod);
   dbus::MessageWriter writer(&method_call);
   writer.AppendArrayOfStrings(requested_technologies);
   dbus::AppendValueData(&writer, options);
-  peer_daemon_proxy_->CallMethod(
-      &method_call,
-      dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+  object_proxy->CallMethod(
+      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
       base::Bind(&PeerDaemonManagerClientImpl::OnStringDBusMethod,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 callback));
+                 weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
 void PeerDaemonManagerClientImpl::StopMonitoring(
     const std::string& monitoring_token,
     const VoidDBusMethodCallback& callback) {
+  dbus::ObjectProxy* object_proxy = object_manager_->GetObjectProxy(
+      dbus::ObjectPath(peerd::kPeerdManagerPath));
+  if (!object_proxy) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&PeerDaemonManagerClientImpl::OnVoidDBusMethod,
+                   weak_ptr_factory_.GetWeakPtr(), callback, nullptr));
+    return;
+  }
+
   dbus::MethodCall method_call(peerd::kManagerInterface,
                                peerd::kStopMonitoringMethod);
   dbus::MessageWriter writer(&method_call);
   writer.AppendString(monitoring_token);
-  peer_daemon_proxy_->CallMethod(
-      &method_call,
-      dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+  object_proxy->CallMethod(
+      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
       base::Bind(&PeerDaemonManagerClientImpl::OnVoidDBusMethod,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 callback));
+                 weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
 void PeerDaemonManagerClientImpl::ExposeService(
@@ -109,6 +195,13 @@ void PeerDaemonManagerClientImpl::ExposeService(
     const std::map<std::string, std::string>& service_info,
     const base::DictionaryValue& options,
     const StringDBusMethodCallback& callback) {
+  dbus::ObjectProxy* object_proxy = object_manager_->GetObjectProxy(
+      dbus::ObjectPath(peerd::kPeerdManagerPath));
+  if (!object_proxy) {
+    callback.Run(DBUS_METHOD_CALL_FAILURE, "");

[hashimoto, 2015/02/06 07:45:12]

Please PostTask() here and use std::string().
The same goes for other methods.

[dtapuska, 2015/02/06 15:56:00]

Done.

+    return;
+  }
+
   dbus::MethodCall method_call(peerd::kManagerInterface,
                                peerd::kExposeServiceMethod);
   dbus::MessageWriter writer(&method_call);
@@ -126,12 +219,10 @@ void PeerDaemonManagerClientImpl::ExposeService(
   writer.CloseContainer(&array_writer);
 
   dbus::AppendValueData(&writer, options);
-  peer_daemon_proxy_->CallMethod(
-      &method_call,
-      dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+  object_proxy->CallMethod(
+      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
       base::Bind(&PeerDaemonManagerClientImpl::OnStringDBusMethod,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 callback));
+                 weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
 void PeerDaemonManagerClientImpl::RemoveExposedService(
@@ -139,27 +230,89 @@ void PeerDaemonManagerClientImpl::RemoveExposedService(
     const VoidDBusMethodCallback& callback) {
   dbus::MethodCall method_call(peerd::kManagerInterface,
                                peerd::kRemoveExposedServiceMethod);
+  dbus::ObjectProxy* object_proxy = object_manager_->GetObjectProxy(
+      dbus::ObjectPath(peerd::kPeerdManagerPath));
+  if (!object_proxy) {
+    callback.Run(DBUS_METHOD_CALL_FAILURE);
+    return;
+  }
   dbus::MessageWriter writer(&method_call);
   writer.AppendString(service_token);
-  peer_daemon_proxy_->CallMethod(
-      &method_call,
-      dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+  object_proxy->CallMethod(
+      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
       base::Bind(&PeerDaemonManagerClientImpl::OnVoidDBusMethod,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 callback));
+                 weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
 void PeerDaemonManagerClientImpl::Ping(
     const StringDBusMethodCallback& callback) {
   dbus::MethodCall method_call(peerd::kManagerInterface,
                                peerd::kPingMethod);
+  dbus::ObjectProxy* object_proxy = object_manager_->GetObjectProxy(
+      dbus::ObjectPath(peerd::kPeerdManagerPath));
+  if (!object_proxy) {
+    callback.Run(DBUS_METHOD_CALL_FAILURE, "");
+    return;
+  }
   dbus::MessageWriter writer(&method_call);
-  peer_daemon_proxy_->CallMethod(
-      &method_call,
-      dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+  object_proxy->CallMethod(
+      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
       base::Bind(&PeerDaemonManagerClientImpl::OnStringDBusMethod,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 callback));
+                 weak_ptr_factory_.GetWeakPtr(), callback));
+}
+
+dbus::PropertySet* PeerDaemonManagerClientImpl::CreateProperties(
+    dbus::ObjectProxy* object_proxy,
+    const dbus::ObjectPath& object_path,
+    const std::string& interface_name) {
+  dbus::PropertySet* properties = nullptr;
+  if (interface_name == peerd::kManagerInterface) {
+    properties = new ManagerProperties(
+        object_proxy,
+        base::Bind(&PeerDaemonManagerClientImpl::OnManagerPropertyChanged,
+                   weak_ptr_factory_.GetWeakPtr()));
+  } else if (interface_name == peerd::kServiceInterface) {
+    properties = new ServiceProperties(
+        object_proxy,
+        base::Bind(&PeerDaemonManagerClientImpl::OnServicePropertyChanged,
+                   weak_ptr_factory_.GetWeakPtr(), object_path));
+  } else if (interface_name == peerd::kPeerInterface) {
+    properties = new PeerProperties(
+        object_proxy,
+        base::Bind(&PeerDaemonManagerClientImpl::OnPeerPropertyChanged,
+                   weak_ptr_factory_.GetWeakPtr(), object_path));
+  } else {
+    NOTREACHED() << "Unhandled interface name " << interface_name;
+  }
+  return properties;
+}
+
+void PeerDaemonManagerClientImpl::ObjectAdded(
+    const dbus::ObjectPath& object_path,
+    const std::string& interface_name) {
+  if (interface_name == peerd::kManagerInterface) {
+    FOR_EACH_OBSERVER(Observer, observers_, ManagerAdded());
+  } else if (interface_name == peerd::kServiceInterface) {
+    FOR_EACH_OBSERVER(Observer, observers_, ServiceAdded(object_path));
+  } else if (interface_name == peerd::kPeerInterface) {
+    FOR_EACH_OBSERVER(Observer, observers_, PeerAdded(object_path));
+  } else {
+    NOTREACHED() << "Unhandled interface name " << interface_name;
+  }
+}
+
+void PeerDaemonManagerClientImpl::ObjectRemoved(
+    const dbus::ObjectPath& object_path,
+    const std::string& interface_name) {
+  if (interface_name == peerd::kManagerInterface) {
+    FOR_EACH_OBSERVER(Observer, observers_, ManagerRemoved());
+  } else if (interface_name == peerd::kServiceInterface) {
+    FOR_EACH_OBSERVER(Observer, observers_, ServiceRemoved(object_path));
+  } else if (interface_name == peerd::kPeerInterface) {
+    FOR_EACH_OBSERVER(Observer, observers_, PeerRemoved(object_path));
+  } else {
+    NOTREACHED() << "Unhandled interface name " << interface_name;
+  }
 }
 
 void PeerDaemonManagerClientImpl::OnStringDBusMethod(
@@ -187,13 +340,68 @@ void PeerDaemonManagerClientImpl::OnVoidDBusMethod(
 }
 
 void PeerDaemonManagerClientImpl::Init(dbus::Bus* bus) {
-  peer_daemon_proxy_ =
-      bus->GetObjectProxy(peerd::kPeerdServiceName,
-                          dbus::ObjectPath(peerd::kPeerdManagerPath));
+  object_manager_ = bus->GetObjectManager(
+      peerd::kPeerdServiceName,
+      dbus::ObjectPath(peerd::kPeerdObjectManagerServicePath));
+  object_manager_->RegisterInterface(peerd::kManagerInterface, this);
+  object_manager_->RegisterInterface(peerd::kPeerInterface, this);
+  object_manager_->RegisterInterface(peerd::kServiceInterface, this);
+}
+
+void PeerDaemonManagerClientImpl::OnManagerPropertyChanged(
+    const std::string& property_name) {
+  FOR_EACH_OBSERVER(Observer, observers_,
+                    ManagerPropertyChanged(property_name));
+}
+
+void PeerDaemonManagerClientImpl::OnPeerPropertyChanged(
+    const dbus::ObjectPath& object_path,
+    const std::string& property_name) {
+  FOR_EACH_OBSERVER(Observer, observers_,
+                    PeerPropertyChanged(object_path, property_name));
+}
+
+void PeerDaemonManagerClientImpl::OnServicePropertyChanged(
+    const dbus::ObjectPath& object_path,
+    const std::string& property_name) {
+  FOR_EACH_OBSERVER(Observer, observers_,
+                    ServicePropertyChanged(object_path, property_name));
 }
 
 }  // namespace
 
+PeerDaemonManagerClient::ManagerProperties::ManagerProperties(
+    dbus::ObjectProxy* object_proxy,
+    const PropertyChangedCallback& callback)
+    : dbus::PropertySet{object_proxy, peerd::kManagerInterface, callback} {
+  RegisterProperty("MonitoredTechnologies", &monitored_technologies);
+}
+
+PeerDaemonManagerClient::ManagerProperties::~ManagerProperties() {
+}
+
+PeerDaemonManagerClient::PeerProperties::PeerProperties(

[hashimoto, 2015/02/06 07:45:11]

Please put methods in the same order as declared in the header.

[dtapuska, 2015/02/06 15:56:00]

Done.

+    dbus::ObjectProxy* object_proxy,
+    const PropertyChangedCallback& callback)
+    : dbus::PropertySet{object_proxy, peerd::kPeerInterface, callback} {
+  RegisterProperty("UUID", &uuid);
+  RegisterProperty("LastSeen", &last_seen);
+}
+
+PeerDaemonManagerClient::PeerProperties::~PeerProperties() {
+}
+
+PeerDaemonManagerClient::ServiceProperties::ServiceProperties(
+    dbus::ObjectProxy* object_proxy,
+    const PropertyChangedCallback& callback)
+    : dbus::PropertySet{object_proxy, peerd::kServiceInterface, callback} {
+  RegisterProperty("ServiceId", &service_id);
+  RegisterProperty("ServiceInfo", &service_info);
+  RegisterProperty("IpInfos", &ip_infos);
+}
+
+PeerDaemonManagerClient::ServiceProperties::~ServiceProperties() {
+}
 
 PeerDaemonManagerClient::PeerDaemonManagerClient() {
 }