Multi process notifications on linux (dbus)

chrome/browser/multi_process_notification_linux.cc

Index: chrome/browser/multi_process_notification_linux.cc
diff --git a/chrome/browser/multi_process_notification_linux.cc b/chrome/browser/multi_process_notification_linux.cc
index 940556f4cbd34996a32af2f1587a1fef1dbbcdc3..5bcbe7154f764a72a028b1e07be1773088d6dc9c 100644
--- a/chrome/browser/multi_process_notification_linux.cc
+++ b/chrome/browser/multi_process_notification_linux.cc
@@ -4,45 +4,330 @@
 
 #include "chrome/browser/multi_process_notification.h"
 
+#include <dbus/dbus.h>
+#include <set>
+
+#include "base/basictypes.h"
+#include "base/file_path.h"
 #include "base/logging.h"
+#include "base/message_loop_proxy.h"
+#include "base/path_service.h"
+#include "base/stringprintf.h"
+#include "base/threading/simple_thread.h"
+#include "chrome/common/chrome_paths.h"
 
 namespace multi_process_notification {
 
-bool Post(const std::string& name, Domain domain) {
-  // TODO(dmaclach): Implement
-  NOTIMPLEMENTED();
-  return false;
-}
+const char* kNotifyPath = "/modules/notify";
+const char* kNotifyInterface = "org.chromium.Notify";
+const char* kNotifySignalName = "Notify";
+
+// A simple thread to run the DBus main loop.
+class ListenerThread : public base::SimpleThread {
+ public:
+  ListenerThread();
+
+  bool Init();
+
+  bool AddListener(ListenerImpl* listner);
+  bool RemoveListener(ListenerImpl* listner);
+
+  // SimpleThread overrides
+  virtual void Run();
 
+ private:
+  DBusConnection* connection_;
+
+  Lock listeners_lock_;
+  std::set<ListenerImpl*> listeners_;
+};
+
+// This does all the heavy lifting for Listener class.
 class ListenerImpl {
  public:
   ListenerImpl(const std::string& name,
                Domain domain,
                Listener::Delegate* delegate);
+  virtual ~ListenerImpl();

[dmac, 2011/01/21 22:28:33]

no need for this to be virtual. Nothing else is here, and this class doesn't
inherit.

[garykac, 2011/01/22 00:09:05]

Done.

 
   bool Start(MessageLoop* io_loop_to_listen_on);
+  void OnListen(const std::string& name);

[dmac, 2011/01/21 22:28:33]

can OnListen be private?

[garykac, 2011/01/22 00:09:05]

ListenerThread calls it.

 
   std::string name() const { return name_; }
   Domain domain() const { return domain_; }
 
  private:
+  void StartListener();
+
   std::string name_;
   Domain domain_;
   Listener::Delegate* delegate_;
 
+  // Full name of signal (= domain-prefix + user-specified-name).
+  std::string fullname_;
+
+  DBusError error_;
+  DBusConnection* connection_;
+
+  Lock thread_lock_;
+  static ListenerThread* g_thread_;
+
+  scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
+
   DISALLOW_COPY_AND_ASSIGN(ListenerImpl);
 };
 
+ListenerThread* ListenerImpl::g_thread_ = NULL;
+
+// Return the fullname for this signal by taking the user-specified name and
+// adding a prefix based on the domain.
+std::string AddDomainPrefixToNotification(const std::string& name,
+                                          Domain domain) {
+  std::string prefix;
+  switch (domain) {
+    case multi_process_notification::ProfileDomain: {
+      FilePath user_data_dir;
+      if (!PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
+        NOTREACHED();
+      }
+      prefix = StringPrintf("user.%u.%s.", getuid(),
+                            user_data_dir.value().c_str());
+      break;
+    }
+    case multi_process_notification::UserDomain:
+      prefix = StringPrintf("user.%u.", getuid());
+      break;
+    case multi_process_notification::SystemDomain:
+      prefix = "";
+      break;
+  }
+  return prefix + name;
+}
+
+bool Post(const std::string& name, Domain domain) {
+  DBusError error;
+  dbus_error_init(&error);
+
+  // Get a connection to the DBus.
+  DBusConnection* connection = dbus_bus_get(DBUS_BUS_SESSION, &error);
+  if (dbus_error_is_set(&error)) {
+    LOG(ERROR) << "Failed to create initial dbus connection: " << error.message;
+    dbus_error_free(&error);
+    return false;
+  }
+  if (!connection) {
+    LOG(ERROR) << "Failed to create initial dbus connection";
+    return false;
+  }
+
+  std::string fullname = AddDomainPrefixToNotification(name, domain);
+
+  // Create the Notify signal.
+  bool success = true;
+  DBusMessage* message = dbus_message_new_signal(kNotifyPath, kNotifyInterface,
+                                                 kNotifySignalName);
+  if (!message) {
+    LOG(ERROR) << "Failed to create dbus message for signal: "
+               << kNotifySignalName << " (" << kNotifyInterface << ")";
+    success = false;
+  }
+
+  // Add the full signal name as an argument to the Notify signal.
+  if (success) {
+    DBusMessageIter args;
+    const char* cname = fullname.c_str();
+    dbus_message_iter_init_append(message, &args);
+    if (!dbus_message_iter_append_basic(&args,
+                                        DBUS_TYPE_STRING, &cname)) {
+      LOG(ERROR) << "Failed to set signal name: " << fullname;
+      success = false;
+    }
+  }
+
+  // Actually send the signal.
+  if (success) {
+    dbus_uint32_t serial = 0;
+    if (!dbus_connection_send(connection, message, &serial)) {
+      LOG(ERROR) << "Unable to send dbus message for " << fullname;
+      success = false;
+    }
+  }
+
+  if (success) {
+    dbus_connection_flush(connection);
+  }
+  dbus_message_unref(message);
+
+  return success;
+}
+
+ListenerThread::ListenerThread()
+    : base::SimpleThread("ListenerThread"),
+      connection_(NULL) {
+}
+
+bool ListenerThread::Init() {
+  DBusError error;
+  dbus_error_init(&error);
+
+  // Get a connection to the DBus.
+  connection_ = dbus_bus_get(DBUS_BUS_SESSION, &error);
+  if (dbus_error_is_set(&error)) {
+    LOG(ERROR) << "Failed to create initial dbus connection: " << error.message;
+    dbus_error_free(&error);
+    return false;
+  }
+  if (!connection_) {
+    LOG(ERROR) << "Failed to create initial dbus connection";
+    return false;
+  }
+
+  // Create matching rule for our signal type.
+  std::string match_rule = StringPrintf("type='signal',interface='%s'",
+                                        kNotifyInterface);
+  dbus_bus_add_match(connection_, match_rule.c_str(), &error);
+  dbus_connection_flush(connection_);
+  if (dbus_error_is_set(&error)) {
+    LOG(ERROR) << "Failed to add dbus match rule for "
+               << kNotifyInterface << ": "
+               << error.message;
+    dbus_error_free(&error);
+    return false;
+  }
+
+  return true;
+}
+
+bool ListenerThread::AddListener(ListenerImpl* listener) {
+  base::AutoLock autolock(listeners_lock_);
+  listeners_.insert(listener);
+  return true;
+}
+
+bool ListenerThread::RemoveListener(ListenerImpl* listener) {
+  base::AutoLock autolock(listeners_lock_);
+  listeners_.erase(listener);
+  return true;
+}
+
+void ListenerThread::Run() {
+  bool done = false;
+
+  while (!done) {
+    // Get next available message.
+    // Using -1 for a timeout to make this a blocking call - but it blocks all
+    // dbus calls on this connection. Thus, we use a timeout of 0 and a sleep.
+    // TODO(garykac): Ugh. Fix this to use file descriptors if possible.
+    dbus_connection_read_write(connection_, 0);
+
+    DBusMessage* message = dbus_connection_pop_message(connection_);
+    if (!message) {
+      struct timespec delay = {0};
+      delay.tv_sec = 0;
+      delay.tv_nsec = 500 * 1000 * 1000;
+      nanosleep(&delay, NULL);  // nanoYuck!
+      continue;
+    }
+
+    // Process all queued up messages.
+    while (message) {
+      if (dbus_message_is_signal(message,
+                                 kNotifyInterface, kNotifySignalName)) {
+        // Get user-defined name from the signal arguments.
+        DBusMessageIter args;
+        if (!dbus_message_iter_init(message, &args)) {
+          LOG(ERROR) << "Params missing from dbus signal";
+        } else if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_STRING) {
+          LOG(ERROR) << "Dbus signal param is not string type";
+        } else {
+          char* name;
+          dbus_message_iter_get_basic(&args, &name);
+          {  // Scope for lock
+            base::AutoLock autolock(listeners_lock_);
+            std::set<ListenerImpl*>::iterator it;
+            // Note that this doesn't scale well to a large number of listeners.
+            // But we should only have a couple active at a time.
+            for (it=listeners_.begin(); it!=listeners_.end(); it++) {
+              (*it)->OnListen(name);
+            }
+          }
+        }
+      }
+
+      dbus_message_unref(message);
+      message = dbus_connection_pop_message(connection_);
+    }
+  }
+}
+
 ListenerImpl::ListenerImpl(const std::string& name,
                            Domain domain,
                            Listener::Delegate* delegate)
-    : name_(name), domain_(domain), delegate_(delegate) {
+    : name_(name),
+      domain_(domain),
+      delegate_(delegate),
+      fullname_(name),

[dmac, 2011/01/21 22:28:33]

why bother with fullname_ you use it in two places rarely. I would recommend
just generating it when you need it. It's being initialized incorrectly here
anyways.

[garykac, 2011/01/22 00:09:05]

Done.

+      connection_(NULL) {
+}
+
+ListenerImpl::~ListenerImpl() {
+  if (g_thread_) {

[dmac, 2011/01/21 22:28:33]

do you need to lock around this reference to g_thread_ as well? appears there is
an error in the Mac version as well regarding this. Would you be so kind as to
add a todo for me to the Mac code?

[garykac, 2011/01/22 00:09:05]

Done.  I checked the Mac code and it looks correct (ti has a lock in the
destructor).

+    g_thread_->RemoveListener(this);
+  }
 }
 
 bool ListenerImpl::Start(MessageLoop* io_loop_to_listen_on) {
-  // TODO(dmaclach): Implement
-  NOTIMPLEMENTED();
-  return false;
+  if (io_loop_to_listen_on->type() != MessageLoop::TYPE_IO) {
+    DLOG(ERROR) << "io_loop_to_listen_on must be TYPE_IO";
+    return false;
+  }
+
+  // Record the fullname for this signal.
+  fullname_ = AddDomainPrefixToNotification(name_, domain_);
+
+  // Start the listener on the IO thread.
+  message_loop_proxy_ = base::MessageLoopProxy::CreateForCurrentThread();
+  Task* task = NewRunnableMethod(this, &ListenerImpl::StartListener);
+  io_loop_to_listen_on->PostTask(FROM_HERE, task);
+  return true;
+}
+
+void ListenerImpl::StartListener() {
+  DCHECK_EQ(MessageLoop::TYPE_IO, MessageLoop::current()->type());
+  bool success = true;
+
+  if (!g_thread_) {

[dmac, 2011/01/21 22:28:33]

double checked lock... not going to work unfortunately

http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf

[garykac, 2011/01/22 00:09:05]

Damn.

+    // No listener thread. Get the lock and check again.
+    base::AutoLock autolock(thread_create_lock_);
+
+    // Start the main dbus loop if needed.
+    if (!g_thread_) {
+      g_thread_ = new ListenerThread();
+      success = g_thread_->Init();
+      if (success) {
+        g_thread_->Start();
+      }
+    }
+  }
+
+  // Register ourselves as a listener of signals.
+  if (success) {
+    g_thread_->AddListener(this);
+  }
+
+  // Send initialization success/fail status to delegate.
+  Task* task = new Listener::ListenerStartedTask(name_, domain_, delegate_,
+                                                 success);
+  CHECK(message_loop_proxy_->PostTask(FROM_HERE, task));
+}
+
+void ListenerImpl::OnListen(const std::string& name) {
+  // Ignore the signal unless it matches our name.
+  if (name == fullname_) {
+    Task* task =
+        new Listener::NotificationReceivedTask(name_, domain_, delegate_);
+    CHECK(message_loop_proxy_->PostTask(FROM_HERE, task));
+  }
 }
 
 Listener::Listener(const std::string& name,
@@ -67,3 +352,5 @@ Domain Listener::domain() const {
 }
 
 }  // namespace multi_process_notification
+
+DISABLE_RUNNABLE_METHOD_REFCOUNT(multi_process_notification::ListenerImpl);