MonitorSignal low-level wrapper introduced

chromeos/dbus/dbus.h

 // Copyright (c) 2009 The Chromium OS Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROMEOS_DBUS_H_
 #define CHROMEOS_DBUS_H_
 
+#include <dbus/dbus-glib-lowlevel.h>

[Daniel Erat, 2011/02/10 23:07:24]

do you need this in the header, or can you forward-declare DBusMessage and
DBusConnection?  (i'm not sure whether they're regular structs or typedef-ed
anonymous structs)

[glotov, 2011/02/11 16:36:52]

Done.

 #include <dbus/dbus-glib.h>
 #include <glib-object.h>
 
 #include <algorithm>
 #include <string>
 
 #include "base/logging.h"
 #include "chromeos/glib/object.h"
 
 namespace chromeos {
@@ skipping 391 matching lines @@
 
 BusConnection GetPrivateBusConnection(const char* address);
 
 // \brief Sends a signal named |signal_name| with no arguments to the
 // system bus per the given |path| and |interface_name|.
 
 void SendSignalWithNoArgumentsToSystemBus(const char* path,
                                           const char* interface_name,
                                           const char* signal_name);
 
+// \brief Low-level signal monitor base class.
+//
+// Used when there is no definite named signal sender (that Proxy
+// could be used for).
+
+class MonitorSignal {

[Daniel Erat, 2011/02/10 23:13:36]

ah, just looked at the change where you're using it.  please rename the class to
something like "SignalWatcher" that makes it clear what it does.

[glotov, 2011/02/11 16:36:52]

Done.

+ public:
+  MonitorSignal() : interface_(NULL), signal_(NULL) {}
+  ~MonitorSignal();
+  void StartMonitoring(const char* interface, const char* signal);

[Daniel Erat, 2011/02/10 23:07:24]

it might be a bit cleaner to pass |interface| and |signal| in through the c'tor
instead of including them here, since they're really tied to the lifetime of the
object, or maybe you should just start the monitoring from the c'tor as well.

[glotov, 2011/02/11 16:36:52]

Start monitoring in ctor is not good because derived class might not have
completed its initialization by the time our constructor is done and monitoring
started.

About putting |interface| and |signal| in ctor. The only disadvantage is that
StartMonitoring() would not be able to check if it had already been called (or
additional flag would be needed). So I preferred to pass them here.

+ private:
+  // Callback invoked on the given signal arrival.
+  virtual void OnSignal(DBusMessage* message) = 0;

[Daniel Erat, 2011/02/10 23:07:24]

nit: add a blank line between each method signature and the next method's
comment so this is easier to read

[glotov, 2011/02/11 16:36:52]

Done.

+  // Returns a string matching the D-Bus messages that we want to listen for.
+  std::string GetDBusMatchString() const;
+  // A D-Bus message filter to receive signals.
+  static DBusHandlerResult FilterDBusMessage(DBusConnection* dbus_conn,
+                                             DBusMessage* message,
+                                             void* data);
+  const char* interface_;
+  const char* signal_;
+};
+
 }  // namespace dbus
 }  // namespace chromeos
 
 #endif  // CHROMEOS_DBUS_H_