Revert 169164 - Allow multiple object proxies to handle NameOwnerChanged

Failed on Linux ChromiumOS Tests:

SignalSenderVerificationTest.TestMultipleObjects: 
[4218:4244:1121/160846:447516037:ERROR:object_proxy.cc(624)] Failed to get name owner. Got org.freedesktop.DBus.Error.NameHasNoOwner: Could not get owner of name 'org.chromium.TestService': no such name
[4218:4244:1121/160846:447531010:ERROR:bus.cc(444)] Failed to get the ownership of org.chromium.TestService:
[4218:4248:1121/160846:447531228:ERROR:test_service.cc(134)] Failed to own: org.chromium.TestService
dbus/signal_sender_verification_unittest.cc:302: Failure
Value of: on_name_owner_changed_called_
Actual: false
Expected: true
[4218:4244:1121/160846:447535382:ERROR:bus.cc(604)] Requested to remove an unknown match rule: type='signal',interface='org.freedesktop.DBus',member='NameOwnerChanged',path='/org/freedesktop/DBus',sender='org.freedesktop.DBus',arg0='org.chromium.TestService'

http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/12886/steps/dbus_unittests/logs/TestMultipleObjects

Multiple object proxies may be registered for any single D-Bus service, one per interface. Returning "handled" means only the first interface object proxy gets to handle the signal, others never see it.

Change to always return "not yet handled", while this means D-Bus never knows we actually handled the NameOwnerChanged signal, that's quite ok and is actually what all other bindings do for all signals.

BUG=chromium-os:36486
TEST=dbus_unittests

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

TBR=keybuk@chromium.org
Review URL: https://codereview.chromium.org/11299149

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

2 files changed, 7 insertions, 78 deletions

diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index b20f544..87950d1 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -661,11 +661,11 @@ DBusHandlerResult ObjectProxy::HandleNameOwnerChanged(
                                                 name_owner_changed_callback_,
                                                 released_signal));
       }
+      return DBUS_HANDLER_RESULT_HANDLED;
     }
   }
 
-  // Always return unhandled to let other object proxies handle the same
-  // signal.
+  // Untrusted or uninteresting signal
   return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
diff --git a/dbus/signal_sender_verification_unittest.cc b/dbus/signal_sender_verification_unittest.cc
index 05277694..6e59ea4 100644
--- a/dbus/signal_sender_verification_unittest.cc
+++ b/dbus/signal_sender_verification_unittest.cc
@@ -51,8 +51,7 @@ class SignalSenderVerificationTest : public testing::Test {
 
     object_proxy_->SetNameOwnerChangedCallback(
         base::Bind(&SignalSenderVerificationTest::OnNameOwnerChanged,
-                   base::Unretained(this),
-                   &on_name_owner_changed_called_));
+                   base::Unretained(this)));
 
     // Connect to the "Test" signal of "org.chromium.TestInterface" from
     // the remote object.
@@ -120,17 +119,19 @@ class SignalSenderVerificationTest : public testing::Test {
     message_loop_.Quit();
   }
 
-  void OnNameOwnerChanged(bool* called_flag, dbus::Signal* signal) {
+  void OnNameOwnerChanged(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
     std::string name, old_owner, new_owner;
     ASSERT_TRUE(reader.PopString(&name));
     ASSERT_TRUE(reader.PopString(&old_owner));
     ASSERT_TRUE(reader.PopString(&new_owner));
     latest_name_owner_ = new_owner;
-    *called_flag = true;
+    on_name_owner_changed_called_ = true;
     message_loop_.Quit();
   }
 
+ protected:
+
   // Called when the "Test" signal is received, in the main thread.
   // Copy the string payload to |test_signal_string_|.
   void OnTestSignal(dbus::Signal* signal) {
@@ -147,8 +148,6 @@ class SignalSenderVerificationTest : public testing::Test {
     message_loop_.Quit();
   }
 
- protected:
-
   // Wait for the hey signal to be received.
   void WaitForTestSignal() {
     // OnTestSignal() will quit the message loop.
@@ -246,73 +245,3 @@ TEST_F(SignalSenderVerificationTest, TestOwnerChanged) {
   WaitForTestSignal();
   ASSERT_EQ(kNewMessage, test_signal_string_);
 }
-
-TEST_F(SignalSenderVerificationTest, TestMultipleObjects) {
-  const char kMessage[] = "hello, world";
-
-  dbus::ObjectProxy* object_proxy2 = bus_->GetObjectProxy(
-      "org.chromium.TestService",
-      dbus::ObjectPath("/org/chromium/DifferentObject"));
-
-  bool second_name_owner_changed_called = false;
-  object_proxy2->SetNameOwnerChangedCallback(
-      base::Bind(&SignalSenderVerificationTest::OnNameOwnerChanged,
-                 base::Unretained(this),
-                 &second_name_owner_changed_called));
-
-  // Connect to a signal on the additional remote object to trigger the
-  // name owner matching.
-  object_proxy2->ConnectToSignal(
-      "org.chromium.DifferentTestInterface",
-      "Test",
-      base::Bind(&SignalSenderVerificationTest::OnTestSignal,
-                 base::Unretained(this)),
-      base::Bind(&SignalSenderVerificationTest::OnConnected,
-                 base::Unretained(this)));
-  // Wait until the object proxy is connected to the signal.
-  message_loop_.Run();
-
-  // Send the test signal from the exported object.
-  test_service_->SendTestSignal(kMessage);
-  // Receive the signal with the object proxy. The signal is handled in
-  // SignalSenderVerificationTest::OnTestSignal() in the main thread.
-  WaitForTestSignal();
-  ASSERT_EQ(kMessage, test_signal_string_);
-
-  // Release and acquire the name ownership.
-  // latest_name_owner_ should be non empty as |test_service_| owns the name.
-  ASSERT_FALSE(latest_name_owner_.empty());
-  test_service_->ShutdownAndBlock();
-  // OnNameOwnerChanged will PostTask to quit the message loop.
-  message_loop_.Run();
-  // latest_name_owner_ should be empty as the owner is gone.
-  ASSERT_TRUE(latest_name_owner_.empty());
-
-  // Reset the flag as NameOwnerChanged is already received in setup.
-  on_name_owner_changed_called_ = false;
-  second_name_owner_changed_called = false;
-  test_service2_->RequestOwnership(
-      base::Bind(&SignalSenderVerificationTest::OnOwnership,
-                 base::Unretained(this), true));
-  // Both of OnNameOwnerChanged() and OnOwnership() should quit the MessageLoop,
-  // but there's no expected order of those 2 event.
-  message_loop_.Run();
-  if (!on_name_owner_changed_called_ || !on_ownership_called_)
-    message_loop_.Run();
-  ASSERT_TRUE(on_name_owner_changed_called_);
-  ASSERT_TRUE(on_ownership_called_);
-
-  // The callback for the second object must have also been called in the
-  // same dispatch as for the first.
-  ASSERT_TRUE(second_name_owner_changed_called);
-
-  // latest_name_owner_ becomes non empty as the new owner appears.
-  ASSERT_FALSE(latest_name_owner_.empty());
-
-  // Now the second service owns the name.
-  const char kNewMessage[] = "hello, new world";
-
-  test_service2_->SendTestSignal(kNewMessage);
-  WaitForTestSignal();
-  ASSERT_EQ(kNewMessage, test_signal_string_);
-}