summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/chromeos/dbus/cros_dbus_service.cc9
-rw-r--r--chromeos/dbus/ibus/ibus_panel_service.cc1
-rw-r--r--dbus/bus.cc14
-rw-r--r--dbus/bus.h21
-rw-r--r--dbus/bus_unittest.cc3
-rw-r--r--dbus/mock_bus.h6
-rw-r--r--dbus/signal_sender_verification_unittest.cc67
-rw-r--r--dbus/test_service.cc7
-rw-r--r--dbus/test_service.h8
9 files changed, 125 insertions, 11 deletions
diff --git a/chrome/browser/chromeos/dbus/cros_dbus_service.cc b/chrome/browser/chromeos/dbus/cros_dbus_service.cc
index 2843aa0..aefccdb 100644
--- a/chrome/browser/chromeos/dbus/cros_dbus_service.cc
+++ b/chrome/browser/chromeos/dbus/cros_dbus_service.cc
@@ -49,7 +49,16 @@ class CrosDBusServiceImpl : public CrosDBusService {
if (service_started_)
return;
+ // There are some situations, described in http://crbug.com/234382#c27,
+ // where processes on Linux can wind up stuck in an uninterruptible state
+ // for tens of seconds. If this happens when Chrome is trying to exit,
+ // this unkillable process can wind up clinging to ownership of
+ // kLibCrosServiceName while the system is trying to restart the browser.
+ // This leads to a fatal situation if we don't allow the new browser
+ // instance to replace the old as the owner of kLibCrosServiceName as seen
+ // in http://crbug.com/234382. Hence, REQUIRE_PRIMARY_ALLOW_REPLACEMENT.
bus_->RequestOwnership(kLibCrosServiceName,
+ dbus::Bus::REQUIRE_PRIMARY_ALLOW_REPLACEMENT,
base::Bind(&CrosDBusServiceImpl::OnOwnership,
base::Unretained(this)));
diff --git a/chromeos/dbus/ibus/ibus_panel_service.cc b/chromeos/dbus/ibus/ibus_panel_service.cc
index 6d53d07..bdce7c8 100644
--- a/chromeos/dbus/ibus/ibus_panel_service.cc
+++ b/chromeos/dbus/ibus/ibus_panel_service.cc
@@ -124,6 +124,7 @@ class IBusPanelServiceImpl : public IBusPanelService {
// Request well known name to ibus-daemon.
bus->RequestOwnership(
ibus::panel::kServiceName,
+ dbus::Bus::REQUIRE_PRIMARY,
base::Bind(&IBusPanelServiceImpl::OnRequestOwnership,
weak_ptr_factory_.GetWeakPtr()));
diff --git a/dbus/bus.cc b/dbus/bus.cc
index 2be649e..ec2417e 100644
--- a/dbus/bus.cc
+++ b/dbus/bus.cc
@@ -501,21 +501,23 @@ void Bus::ShutdownOnDBusThreadAndBlock() {
}
void Bus::RequestOwnership(const std::string& service_name,
+ ServiceOwnershipOptions options,
OnOwnershipCallback on_ownership_callback) {
AssertOnOriginThread();
PostTaskToDBusThread(FROM_HERE, base::Bind(
&Bus::RequestOwnershipInternal,
- this, service_name, on_ownership_callback));
+ this, service_name, options, on_ownership_callback));
}
void Bus::RequestOwnershipInternal(const std::string& service_name,
+ ServiceOwnershipOptions options,
OnOwnershipCallback on_ownership_callback) {
AssertOnDBusThread();
bool success = Connect();
if (success)
- success = RequestOwnershipAndBlock(service_name);
+ success = RequestOwnershipAndBlock(service_name, options);
PostTaskToOriginThread(FROM_HERE,
base::Bind(on_ownership_callback,
@@ -523,7 +525,8 @@ void Bus::RequestOwnershipInternal(const std::string& service_name,
success));
}
-bool Bus::RequestOwnershipAndBlock(const std::string& service_name) {
+bool Bus::RequestOwnershipAndBlock(const std::string& service_name,
+ ServiceOwnershipOptions options) {
DCHECK(connection_);
// dbus_bus_request_name() is a blocking call.
AssertOnDBusThread();
@@ -536,7 +539,7 @@ bool Bus::RequestOwnershipAndBlock(const std::string& service_name) {
ScopedDBusError error;
const int result = dbus_bus_request_name(connection_,
service_name.c_str(),
- DBUS_NAME_FLAG_DO_NOT_QUEUE,
+ options,
error.get());
if (result != DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) {
LOG(ERROR) << "Failed to get the ownership of " << service_name << ": "
@@ -568,7 +571,8 @@ bool Bus::ReleaseOwnership(const std::string& service_name) {
return true;
} else {
LOG(ERROR) << "Failed to release the ownership of " << service_name << ": "
- << (error.is_set() ? error.message() : "");
+ << (error.is_set() ? error.message() : "")
+ << ", result code: " << result;
return false;
}
}
diff --git a/dbus/bus.h b/dbus/bus.h
index 220ed78..2e6db70 100644
--- a/dbus/bus.h
+++ b/dbus/bus.h
@@ -168,6 +168,22 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
SUPPRESS_ERRORS,
};
+ // Specifies service ownership options.
+ //
+ // REQUIRE_PRIMARY indicates that you require primary ownership of the
+ // service name.
+ //
+ // ALLOW_REPLACEMENT indicates that you'll allow another connection to
+ // steal ownership of this service name from you.
+ //
+ // REQUIRE_PRIMARY_ALLOW_REPLACEMENT does the obvious.
+ enum ServiceOwnershipOptions {
+ REQUIRE_PRIMARY = (DBUS_NAME_FLAG_DO_NOT_QUEUE |
+ DBUS_NAME_FLAG_REPLACE_EXISTING),
+ REQUIRE_PRIMARY_ALLOW_REPLACEMENT = (REQUIRE_PRIMARY |
+ DBUS_NAME_FLAG_ALLOW_REPLACEMENT),
+ };
+
// Options used to create a Bus object.
struct CHROME_DBUS_EXPORT Options {
Options();
@@ -398,13 +414,15 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
//
// Must be called in the origin thread.
virtual void RequestOwnership(const std::string& service_name,
+ ServiceOwnershipOptions options,
OnOwnershipCallback on_ownership_callback);
// Requests the ownership of the given service name.
// Returns true on success, or the the service name is already obtained.
//
// BLOCKING CALL.
- virtual bool RequestOwnershipAndBlock(const std::string& service_name);
+ virtual bool RequestOwnershipAndBlock(const std::string& service_name,
+ ServiceOwnershipOptions options);
// Releases the ownership of the given service name.
// Returns true on success.
@@ -608,6 +626,7 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
// Helper function used for RequestOwnership().
void RequestOwnershipInternal(const std::string& service_name,
+ ServiceOwnershipOptions options,
OnOwnershipCallback on_ownership_callback);
// Helper function used for GetServiceOwner().
diff --git a/dbus/bus_unittest.cc b/dbus/bus_unittest.cc
index 664ceb5..d695569 100644
--- a/dbus/bus_unittest.cc
+++ b/dbus/bus_unittest.cc
@@ -372,7 +372,8 @@ TEST(BusTest, ListenForServiceOwnerChange) {
EXPECT_EQ(0, num_of_owner_changes1);
// Make an ownership change.
- ASSERT_TRUE(bus->RequestOwnershipAndBlock("org.chromium.TestService"));
+ ASSERT_TRUE(bus->RequestOwnershipAndBlock("org.chromium.TestService",
+ Bus::REQUIRE_PRIMARY));
run_loop_state.Run(1);
{
diff --git a/dbus/mock_bus.h b/dbus/mock_bus.h
index 26113d4..c3991f5 100644
--- a/dbus/mock_bus.h
+++ b/dbus/mock_bus.h
@@ -31,10 +31,12 @@ class MockBus : public Bus {
MOCK_METHOD0(ShutdownAndBlock, void());
MOCK_METHOD0(ShutdownOnDBusThreadAndBlock, void());
MOCK_METHOD0(Connect, bool());
- MOCK_METHOD2(RequestOwnership, void(
+ MOCK_METHOD3(RequestOwnership, void(
const std::string& service_name,
+ ServiceOwnershipOptions options,
OnOwnershipCallback on_ownership_callback));
- MOCK_METHOD1(RequestOwnershipAndBlock, bool(const std::string& service_name));
+ MOCK_METHOD2(RequestOwnershipAndBlock, bool(const std::string& service_name,
+ ServiceOwnershipOptions options));
MOCK_METHOD1(ReleaseOwnership, bool(const std::string& service_name));
MOCK_METHOD0(SetUpAsyncOperations, bool());
MOCK_METHOD3(SendWithReplyAndBlock, DBusMessage*(DBusMessage* request,
diff --git a/dbus/signal_sender_verification_unittest.cc b/dbus/signal_sender_verification_unittest.cc
index df2acd2..cbf4b5f 100644
--- a/dbus/signal_sender_verification_unittest.cc
+++ b/dbus/signal_sender_verification_unittest.cc
@@ -154,6 +154,14 @@ class SignalSenderVerificationTest : public testing::Test {
message_loop_.Run();
}
+ // Stopping a thread is considered an IO operation, so we need to fiddle with
+ // thread restrictions before and after calling Stop() on a TestService.
+ void SafeServiceStop(TestService* test_service) {
+ base::ThreadRestrictions::SetIOAllowed(true);
+ test_service->Stop();
+ base::ThreadRestrictions::SetIOAllowed(false);
+ }
+
base::MessageLoop message_loop_;
scoped_ptr<base::Thread> dbus_thread_;
scoped_refptr<Bus> bus_;
@@ -224,6 +232,7 @@ TEST_F(SignalSenderVerificationTest, TestOwnerChanged) {
// Reset the flag as NameOwnerChanged is already received in setup.
on_name_owner_changed_called_ = false;
+ on_ownership_called_ = false;
test_service2_->RequestOwnership(
base::Bind(&SignalSenderVerificationTest::OnOwnership,
base::Unretained(this), true));
@@ -246,6 +255,64 @@ TEST_F(SignalSenderVerificationTest, TestOwnerChanged) {
ASSERT_EQ(kNewMessage, test_signal_string_);
}
+TEST_F(SignalSenderVerificationTest, TestOwnerStealing) {
+ // 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;
+
+ // Start a test service that allows theft, using the D-Bus thread.
+ TestService::Options options;
+ options.dbus_task_runner = dbus_thread_->message_loop_proxy();
+ options.request_ownership_options = Bus::REQUIRE_PRIMARY_ALLOW_REPLACEMENT;
+ TestService stealable_test_service(options);
+ ASSERT_TRUE(stealable_test_service.StartService());
+ ASSERT_TRUE(stealable_test_service.WaitUntilServiceIsStarted());
+ ASSERT_TRUE(stealable_test_service.HasDBusThread());
+ ASSERT_TRUE(stealable_test_service.has_ownership());
+
+ // OnNameOwnerChanged will PostTask to quit the message loop.
+ message_loop_.Run();
+
+ // Send a signal to check that the service is correctly owned.
+ const char kMessage[] = "hello, world";
+
+ // Send the test signal from the exported object.
+ stealable_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_);
+
+ // Reset the flag as NameOwnerChanged was called above.
+ on_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_);
+
+ // Now the second service owns the name.
+ const char kNewMessage[] = "hello, new world";
+
+ test_service2_->SendTestSignal(kNewMessage);
+ WaitForTestSignal();
+ ASSERT_EQ(kNewMessage, test_signal_string_);
+
+ SafeServiceStop(&stealable_test_service);
+}
+
// Fails on Linux ChromiumOS Tests
TEST_F(SignalSenderVerificationTest, DISABLED_TestMultipleObjects) {
const char kMessage[] = "hello, world";
diff --git a/dbus/test_service.cc b/dbus/test_service.cc
index ae67bf7..5f59501 100644
--- a/dbus/test_service.cc
+++ b/dbus/test_service.cc
@@ -27,7 +27,8 @@ namespace dbus {
// GetManagedObjects.
const int TestService::kNumMethodsToExport = 9;
-TestService::Options::Options() {
+TestService::Options::Options()
+ : request_ownership_options(Bus::REQUIRE_PRIMARY) {
}
TestService::Options::~Options() {
@@ -35,6 +36,7 @@ TestService::Options::~Options() {
TestService::TestService(const Options& options)
: base::Thread("TestService"),
+ request_ownership_options_(options.request_ownership_options),
dbus_task_runner_(options.dbus_task_runner),
on_all_methods_exported_(false, false),
num_exported_methods_(0) {
@@ -103,6 +105,7 @@ void TestService::SendTestSignalFromRootInternal(const std::string& message) {
writer.AppendString(message);
bus_->RequestOwnership("org.chromium.TestService",
+ request_ownership_options_,
base::Bind(&TestService::OnOwnership,
base::Unretained(this),
base::Bind(&EmptyCallback)));
@@ -123,6 +126,7 @@ void TestService::RequestOwnership(base::Callback<void(bool)> callback) {
void TestService::RequestOwnershipInternal(
base::Callback<void(bool)> callback) {
bus_->RequestOwnership("org.chromium.TestService",
+ request_ownership_options_,
base::Bind(&TestService::OnOwnership,
base::Unretained(this),
callback));
@@ -160,6 +164,7 @@ void TestService::Run(base::MessageLoop* message_loop) {
bus_ = new Bus(bus_options);
bus_->RequestOwnership("org.chromium.TestService",
+ request_ownership_options_,
base::Bind(&TestService::OnOwnership,
base::Unretained(this),
base::Bind(&EmptyCallback)));
diff --git a/dbus/test_service.h b/dbus/test_service.h
index 765a630..7ddaf21 100644
--- a/dbus/test_service.h
+++ b/dbus/test_service.h
@@ -9,6 +9,7 @@
#include "base/memory/ref_counted.h"
#include "base/threading/thread.h"
#include "base/synchronization/waitable_event.h"
+#include "dbus/bus.h"
#include "dbus/exported_object.h"
namespace base {
@@ -17,7 +18,6 @@ class SequencedTaskRunner;
namespace dbus {
-class Bus;
class MethodCall;
class MessageWriter;
class Response;
@@ -37,6 +37,9 @@ class TestService : public base::Thread {
// NULL by default (i.e. don't use the D-Bus thread).
scoped_refptr<base::SequencedTaskRunner> dbus_task_runner;
+
+ // Flags governing parameters of service ownership request.
+ Bus::ServiceOwnershipOptions request_ownership_options;
};
// The number of methods we'll export.
@@ -163,6 +166,9 @@ class TestService : public base::Thread {
// Helper function for RequestOwnership().
void RequestOwnershipInternal(base::Callback<void(bool)> callback);
+ // Options to use when requesting service ownership.
+ Bus::ServiceOwnershipOptions request_ownership_options_;
+
scoped_refptr<base::SequencedTaskRunner> dbus_task_runner_;
base::WaitableEvent on_all_methods_exported_;
// The number of methods actually exported.
generated by cgit v1.1 at 2025-01-12 08:40:41 +0000