chrome: bluetooth: hook up DeviceCreated/Removed signals

chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc

Index: chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
diff --git a/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc b/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
index 31ef1dca981ffb8abfe9c0b3f930d6a513faa860..3f88d2e4b8351609cf7fc378cc5a21283cb3ac6c 100644
--- a/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
+++ b/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
@@ -262,6 +262,22 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient {
 
     adapter_proxy->ConnectToSignal(
         bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kDeviceCreatedSignal,
+        base::Bind(&BluetoothAdapterClientImpl::DeviceCreatedReceived,
+                   weak_ptr_factory_.GetWeakPtr(), object_path),
+        base::Bind(&BluetoothAdapterClientImpl::DeviceCreatedConnected,
+                   weak_ptr_factory_.GetWeakPtr(), object_path));
+
+    adapter_proxy->ConnectToSignal(
+        bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kDeviceRemovedSignal,
+        base::Bind(&BluetoothAdapterClientImpl::DeviceRemovedReceived,
+                   weak_ptr_factory_.GetWeakPtr(), object_path),
+        base::Bind(&BluetoothAdapterClientImpl::DeviceRemovedConnected,
+                   weak_ptr_factory_.GetWeakPtr(), object_path));
+
+    adapter_proxy->ConnectToSignal(
+        bluetooth_adapter::kBluetoothAdapterInterface,
         bluetooth_adapter::kPropertyChangedSignal,
         base::Bind(&BluetoothAdapterClientImpl::PropertyChangedReceived,
                    weak_ptr_factory_.GetWeakPtr(), object_path),
@@ -292,6 +308,60 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient {
     proxy_map_.erase(object_path);
   }
 
+  // Called by dbus:: when a DeviceCreated signal is received.

[mukesh agrawal, 2012/02/01 00:48:42]

why "dbus::" (trailing colons)?
(same for other functions)

[keybuk, 2012/02/04 00:35:47]

referring to "code in the dbus namespace", ie the generic dbus::ObjectProxy,
dbus::Bus, etc. code

+  void DeviceCreatedReceived(const std::string& object_path,

[mukesh agrawal, 2012/02/01 00:48:42]

you might consider "using std::string" and shortening the parameter types.
(OTOH, there's a bunch of existing code that would need to be change. so i'm
also ok leaving as-is.)

[satorux1, 2012/02/01 17:45:18]

Please don't do this in Chrome's code for consistency. std::whatever is not
shortened in Chrome's code.

+                             dbus::Signal* signal) {
+    DCHECK(signal);
+    dbus::MessageReader reader(signal);
+    std::string device_path;
+    if (!reader.PopString(&device_path)) {
+      LOG(ERROR) << object_path
+          << ": DeviceCreated signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+    VLOG(1) << object_path << ": Device created: " << device_path;
+
+    FOR_EACH_OBSERVER(BluetoothAdapterClient::Observer, observers_,
+                      DeviceCreated(object_path, device_path));

[mukesh agrawal, 2012/02/01 00:48:42]

are the arguments in the correct order? prototype is

  void DeviceCreated(const std::string& adapter_path,
                     const std::string& object_path)

[keybuk, 2012/02/04 00:35:47]

yes, the "object_path" here refers to the adapter (this is adapter code), but
from the observer point of view, the object_path is the device being created so
that means we put our object_path into the adapter_path argument and device_path
into the object_path argument

I'm slowly converting the code to use adapter_path, device_path, etc. rather
than the generic object_path, but have to do it one step at a time

+  }
+
+  // Called by dbus:: when the DeviceCreated signal is initially connected.
+  void DeviceCreatedConnected(const std::string& object_path,
+                              const std::string& interface_name,

[mukesh agrawal, 2012/02/01 00:48:42]

google style comments out unused parameters. |interface_name| and |signal_name|
should be commented out here.

[keybuk, 2012/02/04 00:35:47]

Chrome style seems to not do that afaict?

+                              const std::string& signal_name,
+                              bool success) {
+    LOG_IF(WARNING, !success) << object_path
+        << ": Failed to connect to DeviceCreated signal.";
+  }
+
+  // Called by dbus:: when a DeviceRemoved signal is received.
+  void DeviceRemovedReceived(const std::string& object_path,
+                             dbus::Signal* signal) {
+    DCHECK(signal);
+    dbus::MessageReader reader(signal);
+    std::string device_path;
+    if (!reader.PopString(&device_path)) {
+      LOG(ERROR) << object_path
+          << ": DeviceRemoved signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+    VLOG(1) << object_path << ": Device created: " << device_path;
+
+    FOR_EACH_OBSERVER(BluetoothAdapterClient::Observer, observers_,
+                      DeviceRemoved(object_path, device_path));

[mukesh agrawal, 2012/02/01 00:48:42]

as in DeviceAddedReceived: check callback parameter order

+  }
+
+  // Called by dbus:: when the DeviceRemoved signal is initially connected.
+  void DeviceRemovedConnected(const std::string& object_path,
+                              const std::string& interface_name,

[mukesh agrawal, 2012/02/01 00:48:42]

as above (comment out unused parameters).

+                              const std::string& signal_name,
+                              bool success) {
+    LOG_IF(WARNING, !success) << object_path
+        << ": Failed to connect to DeviceRemoved signal.";
+  }
+
   // Called by dbus:: when a PropertyChanged signal is received.
   void PropertyChangedReceived(const std::string& object_path,
                                dbus::Signal* signal) {