dbus: verify object path of incoming signals

The existing behavior, while convenient for debugging, is wrong.

D-Bus will not call any further filter functions once one returns
DBUS_HANDLER_RESULT_HANDLED, in order for the next to be called a
filter must return DBUS_HANDLER_RESULT_NOT_YET_HANDLED if it does
not handle the incoming signal.

We also can't defer this to the signal function since we have to
post that to a different thread, and return values get hard.

Since object proxies are constructed per-path, and match common
interfaces and members, this means signals must be matched on an
object otherwise only the first registered object proxy for any
client will be called, and will be called for all signals.

BUG=chromium-os:27113
TEST=ran unit tests, and manually verified existing code that uses ConnectToSignal

Change-Id: Ia4cbc064dff0421a37fe4c4b7c719acf25eb630c


Review URL: http://codereview.chromium.org/9508005

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

1 files changed, 38 insertions, 5 deletions

diff --git a/dbus/end_to_end_async_unittest.cc b/dbus/end_to_end_async_unittest.cc
index 9a88b42..302b7b4 100644
--- a/dbus/end_to_end_async_unittest.cc
+++ b/dbus/end_to_end_async_unittest.cc
@@ -83,6 +83,24 @@ class EndToEndAsyncTest : public testing::Test {
                    base::Unretained(this)));
     // Wait until the object proxy is connected to the signal.
     message_loop_.Run();
+
+    // Create a second object proxy for the root object.
+    root_object_proxy_ = bus_->GetObjectProxy(
+        "org.chromium.TestService",
+        dbus::ObjectPath("/"));
+    ASSERT_TRUE(bus_->HasDBusThread());
+
+    // Connect to the "Test" signal of "org.chromium.TestInterface" from
+    // the root remote object too.
+    root_object_proxy_->ConnectToSignal(
+        "org.chromium.TestInterface",
+        "Test",
+        base::Bind(&EndToEndAsyncTest::OnRootTestSignal,
+                   base::Unretained(this)),
+        base::Bind(&EndToEndAsyncTest::OnConnected,
+                   base::Unretained(this)));
+    // Wait until the root object proxy is connected to the signal.
+    message_loop_.Run();
   }
 
   virtual void TearDown() {
@@ -140,6 +158,15 @@ class EndToEndAsyncTest : public testing::Test {
     message_loop_.Quit();
   }
 
+  // Called when the "Test" signal is received, in the main thread, by
+  // the root object proxy. Copy the string payload to
+  // |root_test_signal_string_|.
+  void OnRootTestSignal(dbus::Signal* signal) {
+    dbus::MessageReader reader(signal);
+    ASSERT_TRUE(reader.PopString(&root_test_signal_string_));
+    message_loop_.Quit();
+  }
+
   // Called when the "Test2" signal is received, in the main thread.
   void OnTest2Signal(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
@@ -165,9 +192,12 @@ class EndToEndAsyncTest : public testing::Test {
   scoped_ptr<base::Thread> dbus_thread_;
   scoped_refptr<dbus::Bus> bus_;
   dbus::ObjectProxy* object_proxy_;
+  dbus::ObjectProxy* root_object_proxy_;
   scoped_ptr<dbus::TestService> test_service_;
   // Text message from "Test" signal.
   std::string test_signal_string_;
+  // Text message from "Test" signal delivered to root.
+  std::string root_test_signal_string_;
 };
 
 TEST_F(EndToEndAsyncTest, Echo) {
@@ -302,11 +332,14 @@ TEST_F(EndToEndAsyncTest, TestSignal) {
 
 TEST_F(EndToEndAsyncTest, TestSignalFromRoot) {
   const char kMessage[] = "hello, world";
-  // Send the test signal from the root object path, to see if we can
-  // handle signals sent from "/", like dbus-send does.
+  // Object proxies are tied to a particular object path, if a signal
+  // arrives from a different object path like "/" the first object proxy
+  // |object_proxy_| should not handle it, and should leave it for the root
+  // object proxy |root_object_proxy_|.
   test_service_->SendTestSignalFromRoot(kMessage);
-  // Receive the signal with the object proxy. The signal is handled in
-  // EndToEndAsyncTest::OnTestSignal() in the main thread.
   WaitForTestSignal();
-  ASSERT_EQ(kMessage, test_signal_string_);
+  // Verify the signal was not received by the specific proxy.
+  ASSERT_TRUE(test_signal_string_.empty());
+  // Verify the string WAS received by the root proxy.
+  ASSERT_EQ(kMessage, root_test_signal_string_);
 }