D-Bus: allow multiple signal handlers for a signal

For the org.freedesktop.DBus.Properties.PropertyChanged signal, all
relevant clients will share a single ObjectProxy since they share
the same path and interface; the actual destination client for the
signal is determined by its arguments.

This means that we must support multiple signal handlers for a single
object proxy, the previous fix of replacing with the latest was not
sufficient for this case.

Due to issue 223483, this is not a complete fix. Property objects
coming and going will leave signal handlers in the ObjectProxy with
NULLd weak pointer references - they will be harmless, but use up
memory.

BUG=234380
TEST=dbus_unittests
R=satorux@chromium.org

Review URL: https://codereview.chromium.org/14333009

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

3 files changed, 33 insertions, 27 deletions

diff --git a/dbus/end_to_end_async_unittest.cc b/dbus/end_to_end_async_unittest.cc
index 6943494..7ef1725 100644
--- a/dbus/end_to_end_async_unittest.cc
+++ b/dbus/end_to_end_async_unittest.cc
@@ -587,24 +587,24 @@ TEST_F(EndToEndAsyncTest, DisconnectedSignal) {
   EXPECT_EQ(1, on_disconnected_call_count_);
 }
 
-class SignalReplacementTest : public EndToEndAsyncTest {
+class SignalMultipleHandlerTest : public EndToEndAsyncTest {
  public:
-  SignalReplacementTest() {
+  SignalMultipleHandlerTest() {
   }
 
   virtual void SetUp() {
     // Set up base class.
     EndToEndAsyncTest::SetUp();
 
-    // Reconnect the root object proxy's signal handler to a new handler
+    // Connect the root object proxy's signal handler to a new handler
     // so that we can verify that a second call to ConnectSignal() delivers
-    // to our new handler and not the old.
+    // to both our new handler and the old.
     object_proxy_->ConnectToSignal(
         "org.chromium.TestInterface",
         "Test",
-        base::Bind(&SignalReplacementTest::OnReplacementTestSignal,
+        base::Bind(&SignalMultipleHandlerTest::OnAdditionalTestSignal,
                    base::Unretained(this)),
-        base::Bind(&SignalReplacementTest::OnReplacementConnected,
+        base::Bind(&SignalMultipleHandlerTest::OnAdditionalConnected,
                    base::Unretained(this)));
     // Wait until the object proxy is connected to the signal.
     message_loop_.Run();
@@ -612,33 +612,33 @@ class SignalReplacementTest : public EndToEndAsyncTest {
 
  protected:
   // Called when the "Test" signal is received, in the main thread.
-  // Copy the string payload to |replacement_test_signal_string_|.
-  void OnReplacementTestSignal(dbus::Signal* signal) {
+  // Copy the string payload to |additional_test_signal_string_|.
+  void OnAdditionalTestSignal(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
-    ASSERT_TRUE(reader.PopString(&replacement_test_signal_string_));
+    ASSERT_TRUE(reader.PopString(&additional_test_signal_string_));
     message_loop_.Quit();
   }
 
   // Called when connected to the signal.
-  void OnReplacementConnected(const std::string& interface_name,
-                              const std::string& signal_name,
-                              bool success) {
+  void OnAdditionalConnected(const std::string& interface_name,
+                             const std::string& signal_name,
+                             bool success) {
     ASSERT_TRUE(success);
     message_loop_.Quit();
   }
 
-  // Text message from "Test" signal delivered to replacement handler.
-  std::string replacement_test_signal_string_;
+  // Text message from "Test" signal delivered to additional handler.
+  std::string additional_test_signal_string_;
 };
 
-TEST_F(SignalReplacementTest, TestSignalReplacement) {
+TEST_F(SignalMultipleHandlerTest, TestMultipleHandlers) {
   const char kMessage[] = "hello, world";
   // Send the test signal from the exported object.
   test_service_->SendTestSignal(kMessage);
   // Receive the signal with the object proxy.
   WaitForTestSignal();
-  // Verify the string WAS NOT received by the original handler.
-  ASSERT_TRUE(test_signal_string_.empty());
-  // Verify the signal WAS received by the replacement handler.
-  ASSERT_EQ(kMessage, replacement_test_signal_string_);
+  // Verify the string WAS received by the original handler.
+  ASSERT_EQ(kMessage, test_signal_string_);
+  // Verify the signal WAS ALSO received by the additional handler.
+  ASSERT_EQ(kMessage, additional_test_signal_string_);
 }
diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index f6b2952..0e56c7a 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -495,11 +495,14 @@ DBusHandlerResult ObjectProxy::HandleMessage(
 }
 
 void ObjectProxy::RunMethod(base::TimeTicks start_time,
-                            SignalCallback signal_callback,
+                            std::vector<SignalCallback> signal_callbacks,
                             Signal* signal) {
   bus_->AssertOnOriginThread();
 
-  signal_callback.Run(signal);
+  for (std::vector<SignalCallback>::iterator iter = signal_callbacks.begin();
+       iter != signal_callbacks.end(); ++iter)
+    iter->Run(signal);
+
   // Delete the message on the D-Bus thread. See comments in
   // RunResponseCallback().
   bus_->PostTaskToDBusThread(
@@ -568,12 +571,12 @@ bool ObjectProxy::AddMatchRuleWithCallback(
       // Store the match rule, so that we can remove this in Detach().
       match_rules_.insert(match_rule);
       // Add the signal callback to the method table.
-      method_table_[absolute_signal_name] = signal_callback;
+      method_table_[absolute_signal_name].push_back(signal_callback);
       return true;
     }
   } else {
     // We already have the match rule.
-    method_table_[absolute_signal_name] = signal_callback;
+    method_table_[absolute_signal_name].push_back(signal_callback);
     return true;
   }
 }
@@ -654,11 +657,13 @@ DBusHandlerResult ObjectProxy::HandleNameOwnerChanged(
       if (!name_owner_changed_callback_.is_null()) {
         const base::TimeTicks start_time = base::TimeTicks::Now();
         Signal* released_signal = signal.release();
+        std::vector<SignalCallback> callbacks;
+        callbacks.push_back(name_owner_changed_callback_);
         bus_->PostTaskToOriginThread(FROM_HERE,
                                      base::Bind(&ObjectProxy::RunMethod,
                                                 this,
                                                 start_time,
-                                                name_owner_changed_callback_,
+                                                callbacks,
                                                 released_signal));
       }
     }
diff --git a/dbus/object_proxy.h b/dbus/object_proxy.h
index 23c0f4c..44652d6 100644
--- a/dbus/object_proxy.h
+++ b/dbus/object_proxy.h
@@ -10,6 +10,7 @@
 #include <map>
 #include <set>
 #include <string>
+#include <vector>
 
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
@@ -222,7 +223,7 @@ class CHROME_DBUS_EXPORT ObjectProxy
 
   // Runs the method. Helper function for HandleMessage().
   void RunMethod(base::TimeTicks start_time,
-                 SignalCallback signal_callback,
+                 std::vector<SignalCallback> signal_callbacks,
                  Signal* signal);
 
   // Redirects the function call to HandleMessage().
@@ -268,8 +269,8 @@ class CHROME_DBUS_EXPORT ObjectProxy
   bool filter_added_;
 
   // The method table where keys are absolute signal names (i.e. interface
-  // name + signal name), and values are the corresponding callbacks.
-  typedef std::map<std::string, SignalCallback> MethodTable;
+  // name + signal name), and values are lists of the corresponding callbacks.
+  typedef std::map<std::string, std::vector<SignalCallback> > MethodTable;
   MethodTable method_table_;
 
   // The callback called when NameOwnerChanged signal is received.