Call get_dispatch_status function to handle Disconnected signal.

It turns out that dbus_connection_get_dispatch_status should be called even if
the connection is lost, otherwise we miss "Disconnected" signal.

BUG=174431
TEST=Manually done on alex and ran dbus_unittests.


Review URL: https://chromiumcodereview.appspot.com/12211022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181479 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 64 insertions, 7 deletions

diff --git a/dbus/bus.cc b/dbus/bus.cc
index 4096049..6bd404c 100644
--- a/dbus/bus.cc
+++ b/dbus/bus.cc
@@ -1,9 +1,6 @@
 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-//
-// TODO(satorux):
-// - Handle "disconnected" signal.
 
 #include "dbus/bus.h"
 
@@ -24,6 +21,11 @@ namespace dbus {
 
 namespace {
 
+const char kDisconnectedSignal[] = "Disconnected";
+const char kDisconnectedMatchRule[] =
+    "type='signal', path='/org/freedesktop/DBus/Local',"
+    "interface='org.freedesktop.DBus.Local', member='Disconnected'";
+
 // The class is used for watching the file descriptor used for D-Bus
 // communication.
 class Watch : public base::MessagePumpLibevent::Watcher {
@@ -364,12 +366,19 @@ bool Bus::Connect() {
   // We shouldn't exit on the disconnected signal.
   dbus_connection_set_exit_on_disconnect(connection_, false);
 
+  // Watch Disconnected signal.
+  AddFilterFunction(Bus::OnConnectionDisconnectedFilter, this);
+  AddMatch(kDisconnectedMatchRule, error.get());
+
   return true;
 }
 
 void Bus::ShutdownAndBlock() {
   AssertOnDBusThread();
 
+  if (shutdown_completed_)
+    return;  // Already shutdowned, just return.
+
   // Unregister the exported objects.
   for (ExportedObjectTable::iterator iter = exported_object_table_.begin();
        iter != exported_object_table_.end(); ++iter) {
@@ -403,6 +412,11 @@ void Bus::ShutdownAndBlock() {
 
   // Private connection should be closed.
   if (connection_) {
+    // Remove Disconnected watcher.
+    ScopedDBusError error;
+    RemoveFilterFunction(Bus::OnConnectionDisconnectedFilter, this);
+    RemoveMatch(kDisconnectedMatchRule, error.get());
+
     if (connection_type_ == PRIVATE)
       dbus_connection_close(connection_);
     // dbus_connection_close() won't unref.
@@ -704,9 +718,12 @@ void Bus::ProcessAllIncomingDataIfAny() {
   AssertOnDBusThread();
 
   // As mentioned at the class comment in .h file, connection_ can be NULL.
-  if (!connection_ || !dbus_connection_get_is_connected(connection_))
+  if (!connection_)
     return;
 
+  // It is safe and necessary to call dbus_connection_get_dispatch_status even
+  // if the connection is lost. Otherwise we will miss "Disconnected" signal.
+  // (crbug.com/174431)
   if (dbus_connection_get_dispatch_status(connection_) ==
       DBUS_DISPATCH_DATA_REMAINS) {
     while (dbus_connection_dispatch(connection_) ==
@@ -842,9 +859,6 @@ void Bus::OnDispatchStatusChanged(DBusConnection* connection,
   DCHECK_EQ(connection, connection_);
   AssertOnDBusThread();
 
-  if (!dbus_connection_get_is_connected(connection))
-    return;
-
   // We cannot call ProcessAllIncomingDataIfAny() here, as calling
   // dbus_connection_dispatch() inside DBusDispatchStatusFunction is
   // prohibited by the D-Bus library. Hence, we post a task here instead.
@@ -854,6 +868,21 @@ void Bus::OnDispatchStatusChanged(DBusConnection* connection,
                                   this));
 }
 
+void Bus::OnConnectionDisconnected(DBusConnection* connection) {
+  AssertOnDBusThread();
+
+  if (!connection)
+    return;
+  DCHECK(!dbus_connection_get_is_connected(connection));
+
+  if (shutdown_completed_)
+    return;  // Do nothing if the shutdown is already completed.
+
+  // Unexpected disconnection, maybe the peer closes the connection.
+  DCHECK_EQ(connection, connection_);
+  ShutdownAndBlock();
+}
+
 dbus_bool_t Bus::OnAddWatchThunk(DBusWatch* raw_watch, void* data) {
   Bus* self = static_cast<Bus*>(data);
   return self->OnAddWatch(raw_watch);
@@ -891,4 +920,19 @@ void Bus::OnDispatchStatusChangedThunk(DBusConnection* connection,
   self->OnDispatchStatusChanged(connection, status);
 }
 
+DBusHandlerResult Bus::OnConnectionDisconnectedFilter(
+    DBusConnection *connection,
+    DBusMessage *message,
+    void *data) {
+  if (dbus_message_is_signal(message,
+                             DBUS_INTERFACE_LOCAL,
+                             kDisconnectedSignal)) {
+    Bus* self = static_cast<Bus*>(data);
+    self->AssertOnDBusThread();
+    self->OnConnectionDisconnected(connection);
+    return DBUS_HANDLER_RESULT_HANDLED;
+  }
+  return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+}
+
 }  // namespace dbus
diff --git a/dbus/bus.h b/dbus/bus.h
index e6a9b20..1bf07d9 100644
--- a/dbus/bus.h
+++ b/dbus/bus.h
@@ -300,6 +300,9 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   // - Releases the service names
   // - Closes the connection to dbus-daemon.
   //
+  // This function can be called multiple times and it is no-op for the 2nd time
+  // calling.
+  //
   // BLOCKING CALL.
   virtual void ShutdownAndBlock();
 
@@ -534,6 +537,9 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   void OnDispatchStatusChanged(DBusConnection* connection,
                                DBusDispatchStatus status);
 
+  // Called when the connection is diconnected.
+  void OnConnectionDisconnected(DBusConnection* connection);
+
   // Callback helper functions. Redirects to the corresponding member function.
   static dbus_bool_t OnAddWatchThunk(DBusWatch* raw_watch, void* data);
   static void OnRemoveWatchThunk(DBusWatch* raw_watch, void* data);
@@ -544,6 +550,13 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   static void OnDispatchStatusChangedThunk(DBusConnection* connection,
                                            DBusDispatchStatus status,
                                            void* data);
+
+  // Calls OnConnectionDisconnected if the Diconnected signal is received.
+  static DBusHandlerResult OnConnectionDisconnectedFilter(
+      DBusConnection *connection,
+      DBusMessage *message,
+      void *user_data);
+
   const BusType bus_type_;
   const ConnectionType connection_type_;
   scoped_refptr<base::MessageLoopProxy> dbus_thread_message_loop_proxy_;