Add sender verification of D-Bus signals.

dbus/signal_sender_verification_unittest.cc

+// 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.
+
+//#include <algorithm>
+//#include <string>
+//#include <vector>
+
+#include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_samples.h"
+#include "base/metrics/statistics_recorder.h"
+#include "base/threading/thread_restrictions.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "dbus/object_proxy.h"
+#include "dbus/test_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+// See comments in ObjectProxy::RunResponseCallback() for why the number was
+// chosen.
+const int kHugePayloadSize = 64 << 20;  // 64 MB
+
+}  // namespace
+
+// The test to for sender verification in ObjectProxy.
+class SignalSenderVerificationTest : public testing::Test {
+ public:
+  SignalSenderVerificationTest() {
+  }
+
+  virtual void SetUp() {
+    base::StatisticsRecorder::Initialize();
+
+    // Make the main thread not to allow IO.
+    base::ThreadRestrictions::SetIOAllowed(false);
+
+    // Start the D-Bus thread.
+    dbus_thread_.reset(new base::Thread("D-Bus Thread"));
+    base::Thread::Options thread_options;
+    thread_options.message_loop_type = MessageLoop::TYPE_IO;
+    ASSERT_TRUE(dbus_thread_->StartWithOptions(thread_options));
+
+    // Start the test service, using the D-Bus thread.
+    dbus::TestService::Options options;
+    options.dbus_thread_message_loop_proxy = dbus_thread_->message_loop_proxy();
+    test_service_.reset(new dbus::TestService(options));
+    ASSERT_TRUE(test_service_->StartService());
+    ASSERT_TRUE(test_service_->WaitUntilServiceIsStarted());
+    ASSERT_TRUE(test_service_->HasDBusThread());
+
+    // Same setup for the second TestService. This service should not have the
+    // ownership of the name at this point.
+    test_service2_.reset(new dbus::TestService(options));
+    ASSERT_TRUE(test_service2_->StartService());
+    ASSERT_TRUE(test_service2_->WaitUntilServiceIsStarted());
+    ASSERT_TRUE(test_service2_->HasDBusThread());
+
+    // Create the client, using the D-Bus thread.
+    dbus::Bus::Options bus_options;
+    bus_options.bus_type = dbus::Bus::SESSION;
+    bus_options.connection_type = dbus::Bus::PRIVATE;
+    bus_options.dbus_thread_message_loop_proxy =
+        dbus_thread_->message_loop_proxy();
+    bus_ = new dbus::Bus(bus_options);
+    object_proxy_ = bus_->GetObjectProxy(
+        "org.chromium.TestService",
+        dbus::ObjectPath("/org/chromium/TestObject"));
+    ASSERT_TRUE(bus_->HasDBusThread());
+
+    // Connect to the "Test" signal of "org.chromium.TestInterface" from
+    // the remote object.
+    object_proxy_->ConnectToSignal(
+        "org.chromium.TestInterface",
+        "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();
+  }
+
+  virtual void TearDown() {
+    bus_->ShutdownOnDBusThreadAndBlock();
+
+    // Shut down the service.
+    test_service_->ShutdownAndBlock();
+    test_service2_->ShutdownAndBlock();
+
+    // Reset to the default.
+    base::ThreadRestrictions::SetIOAllowed(true);
+
+    // Stopping a thread is considered an IO operation, so do this after
+    // allowing IO.
+    test_service_->Stop();
+    test_service2_->Stop();
+  }
+
+ 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) {
+    dbus::MessageReader reader(signal);
+    ASSERT_TRUE(reader.PopString(&test_signal_string_));
+    message_loop_.Quit();
+  }
+
+  // Called when connected to the signal.
+  void OnConnected(const std::string& interface_name,
+                   const std::string& signal_name,
+                   bool success) {
+    ASSERT_TRUE(success);
+    message_loop_.Quit();
+  }
+
+  // Wait for the hey signal to be received.
+  void WaitForTestSignal() {
+    // OnTestSignal() will quit the message loop.
+    message_loop_.Run();
+  }
+
+  MessageLoop message_loop_;
+  scoped_ptr<base::Thread> dbus_thread_;
+  scoped_refptr<dbus::Bus> bus_;
+  dbus::ObjectProxy* object_proxy_;
+  scoped_ptr<dbus::TestService> test_service_;
+  scoped_ptr<dbus::TestService> test_service2_;
+  // Text message from "Test" signal.
+  std::string test_signal_string_;
+};
+
+TEST_F(SignalSenderVerificationTest, TestSignal) {

[satorux1, 2012/10/22 05:23:32]

Please split this into two separate test cases:

SignalAccepted and SignalRejected

[Haruki Sato, 2012/10/24 08:28:05]

Done.

+  const char kMessage[] = "hello, world";
+  // 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_);
+
+  // To make sure the histogram instance is created.
+  UMA_HISTOGRAM_COUNTS("DBus.RejectedSignalCount", 0);
+
+  base::Histogram* r_histogram =
+        base::StatisticsRecorder::FindHistogram("DBus.RejectedSignalCount");
+
+  scoped_ptr<base::HistogramSamples> samples1(
+      r_histogram->SnapshotSamples());
+
+  const char kNewMessage[] = "hello, new world";
+  test_service2_->SendTestSignal(kNewMessage);
+
+  sleep(1);

[satorux1, 2012/10/22 04:50:42]

We shouldn't use sleep() in tests. Can we remove this?

[satorux1, 2012/10/22 05:23:32]

We should probably use 

base::PlatformThread::Sleep(TestTimeouts::action_max_timeout());

Please also add some comment why you need to wait here.

[Haruki Sato, 2012/10/24 08:28:05]

Done. This is the best I can think of for now.
Please let me know if any of you have an idea.
Thanks.

+
+  scoped_ptr<base::HistogramSamples> samples2(
+      r_histogram->SnapshotSamples());
+
+  ASSERT_EQ(kMessage, test_signal_string_);
+  EXPECT_EQ(samples1->TotalCount() + 1, samples2->TotalCount());
+}
+
+TEST_F(SignalSenderVerificationTest, TestOwnerChanged) {
+  const char kMessage[] = "hello, world";
+
+  // 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 aquire the name ownership.
+  test_service_->ShutdownAndBlock();
+  test_service2_->RequestOwnership();
+
+  // Now the second service owns the name.
+  const char kNewMessage[] = "hello, new world";
+
+  test_service2_->SendTestSignal(kNewMessage);
+  WaitForTestSignal();
+  ASSERT_EQ(kNewMessage, test_signal_string_);
+}
+
+// This is an identical copy of EndToEndAsyncTest.TestHugeSignal
+// TODO(haruki): This does not pass somehow. Investigate!
+// GetNameOwner does not reply (get org.freedesktop.DBus.Error.NoReply) and
+// NameOwnerChanged does not come.

[satorux1, 2012/10/22 05:23:32]

GetNameOwner() is calling a method synchronously and blocks the D-Bus thread.
That'd would explain some weirdness here?

I think we should call GetNameOwner() asynchronously, and process the incoming
signal at a later time, once the owner is resolved. That'd complicate the code a
bit, though.

[Haruki Sato, 2012/10/24 08:28:05]

Thank you for the advice. As discussed offline, I've added async version and
more robust message handling.

+TEST_F(SignalSenderVerificationTest, TestHugeSignal) {
+  const std::string kHugeMessage(kHugePayloadSize, 'o');
+
+  // Send the huge signal from the exported object.
+  test_service_->SendTestSignal(kHugeMessage);
+  // This caused a DCHECK failure before. Ensure that the issue is fixed.
+  WaitForTestSignal();
+  ASSERT_EQ(kHugeMessage, test_signal_string_);
+}