dbus: don't fail when reconnecting object signals
Since dbus::ObjectProxy is silently cached, with no way to invalidate,
it's possible that individual instances of objects will come and go
using the same underlying object proxy. i.e. dbus::PropertySet
These will need to change the signal callbacks to be bound to their
own instance, so the current behaviour of failing in this case with
a log message is pessimal.
Change dbus::ObjectProxy to overwrite the existing signal callbacks
with the new ones on repeated calls, rather than preserve the first.
BUG=chromium-os:28064
TEST=unit test included, and we receive property notifications on devices after connection now
Change-Id: Ic4ae092163a364c53bdfcf88f4ce8f74b110b5cb
R=satorux@chromium.org
Review URL: http://codereview.chromium.org/9808001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128100 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: 3b6205cfb6b12f5e1368b6cc57e24debec56e434
diff --git a/dbus/end_to_end_async_unittest.cc b/dbus/end_to_end_async_unittest.cc
index 600c83a..9be24e0 100644
--- a/dbus/end_to_end_async_unittest.cc
+++ b/dbus/end_to_end_async_unittest.cc
@@ -343,3 +343,59 @@
// Verify the string WAS received by the root proxy.
ASSERT_EQ(kMessage, root_test_signal_string_);
}
+
+class SignalReplacementTest : public EndToEndAsyncTest {
+ public:
+ SignalReplacementTest() {
+ }
+
+ virtual void SetUp() {
+ // Set up base class.
+ EndToEndAsyncTest::SetUp();
+
+ // Reconnect 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.
+ object_proxy_->ConnectToSignal(
+ "org.chromium.TestInterface",
+ "Test",
+ base::Bind(&SignalReplacementTest::OnReplacementTestSignal,
+ base::Unretained(this)),
+ base::Bind(&SignalReplacementTest::OnReplacementConnected,
+ base::Unretained(this)));
+ // Wait until the object proxy is connected to the signal.
+ message_loop_.Run();
+ }
+
+ 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) {
+ dbus::MessageReader reader(signal);
+ ASSERT_TRUE(reader.PopString(&replacement_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) {
+ ASSERT_TRUE(success);
+ message_loop_.Quit();
+ }
+
+ // Text message from "Test" signal delivered to replacement handler.
+ std::string replacement_test_signal_string_;
+};
+
+TEST_F(SignalReplacementTest, TestSignalReplacement) {
+ 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_);
+}
diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index 0784ea7..0cbd93e 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -281,14 +281,8 @@
OnConnectedCallback on_connected_callback) {
bus_->AssertOnDBusThread();
- // Check if the object is already connected to the signal.
const std::string absolute_signal_name =
GetAbsoluteSignalName(interface_name, signal_name);
- if (method_table_.find(absolute_signal_name) != method_table_.end()) {
- LOG(ERROR) << "The object proxy is already connected to "
- << absolute_signal_name;
- return;
- }
// Will become true, if everything is successful.
bool success = false;
diff --git a/dbus/object_proxy.h b/dbus/object_proxy.h
index 3a9fab1..6c786ab 100644
--- a/dbus/object_proxy.h
+++ b/dbus/object_proxy.h
@@ -29,7 +29,9 @@
// calling methods of these objects.
//
// ObjectProxy is a ref counted object, to ensure that |this| of the
-// object is is alive when callbacks referencing |this| are called.
+// object is is alive when callbacks referencing |this| are called; the
+// bus always holds at least one of those references so object proxies
+// always last as long as the bus that created them.
class ObjectProxy : public base::RefCountedThreadSafe<ObjectProxy> {
public:
// Client code should use Bus::GetObjectProxy() or
@@ -96,7 +98,8 @@
int timeout_ms,
ResponseCallback callback);
- // Requests to connect to the signal from the remote object.
+ // Requests to connect to the signal from the remote object, replacing
+ // any previous |signal_callback| connected to that signal.
//
// |signal_callback| will be called in the origin thread, when the
// signal is received from the remote object. As it's called in the