buffet: Make ExportedPropertySet threadsafe
In Chrome, we have both a DBus thread and some main thread (an 'origin'
thread). Objects related to DBus functionality need to accomodate this
so that we can send this upstream. This mostly just means that we need
to add assertions that developers are using the API correctly, so that
the use of weak pointers is safe.
BUG=chromium:360831
TEST=Unittests pass. buffet_BasicDBusAPI still passes.
Change-Id: Ibb48a5e65c7cb02e5edce9cbf85432bed70d7686
Reviewed-on: https://chromium-review.googlesource.com/193505
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/buffet.gyp b/buffet/buffet.gyp
index 85c17d4..eb140be 100644
--- a/buffet/buffet.gyp
+++ b/buffet/buffet.gyp
@@ -15,6 +15,7 @@
'link_settings': {
'libraries': [
'-lgflags',
+ '-lbase-dbus_test_support-<(libbase_ver)',
],
},
# TODO(sosa): Remove no-strict-aliasing: crbug.com/356745.
diff --git a/buffet/dbus_manager.cc b/buffet/dbus_manager.cc
index ceddee1..e1b3426 100644
--- a/buffet/dbus_manager.cc
+++ b/buffet/dbus_manager.cc
@@ -7,81 +7,42 @@
#include <string>
#include <base/bind.h>
+#include <dbus/object_path.h>
+#include "buffet/async_event_sequencer.h"
#include "buffet/dbus_constants.h"
+#include "buffet/dbus_utils.h"
using ::std::string;
namespace buffet {
-namespace {
+DBusManager::DBusManager(dbus::Bus* bus)
+ : bus_(bus),
+ exported_object_(bus->GetExportedObject(
+ dbus::ObjectPath(dbus_constants::kRootServicePath))) { }
-// Passes |method_call| to |handler| and passes the response to
-// |response_sender|. If |handler| returns NULL, an empty response is created
-// and sent.
-void HandleSynchronousDBusMethodCall(
- base::Callback<scoped_ptr<dbus::Response>(dbus::MethodCall*)> handler,
- dbus::MethodCall* method_call,
- dbus::ExportedObject::ResponseSender response_sender) {
- auto response = handler.Run(method_call);
- if (!response)
- response = dbus::Response::FromMethodCall(method_call);
-
- response_sender.Run(response.Pass());
+DBusManager::~DBusManager() {
+ // Unregister ourselves from the Bus. This prevents the bus from calling
+ // our callbacks in between the Manager's death and the bus unregistering
+ // our exported object on shutdown. Unretained makes no promises of memory
+ // management.
+ exported_object_->Unregister();
+ exported_object_ = nullptr;
}
-} // namespace
-
-DBusManager::DBusManager()
- : bus_(nullptr) {}
-
-DBusManager::~DBusManager() {}
-
-void DBusManager::Init() {
- InitDBus();
-}
-
-void DBusManager::Finalize() {
- ShutDownDBus();
-}
-
-void DBusManager::InitDBus() {
- dbus::Bus::Options options;
- // TODO(sosa): Should this be on the system bus?
- options.bus_type = dbus::Bus::SYSTEM;
- bus_ = new dbus::Bus(options);
- CHECK(bus_->Connect());
-
- // buffet_dbus_object is owned by the Bus.
- auto buffet_dbus_object = GetExportedObject(dbus_constants::kRootServicePath);
- ExportDBusMethod(
- buffet_dbus_object,
+void DBusManager::Init(const OnInitFinish& cb) {
+ scoped_refptr<dbus_utils::AsyncEventSequencer> sequencer(
+ new dbus_utils::AsyncEventSequencer());
+ exported_object_->ExportMethod(
dbus_constants::kRootInterface, dbus_constants::kRootTestMethod,
- base::Bind(&DBusManager::HandleTestMethod, base::Unretained(this)));
-
- CHECK(bus_->RequestOwnershipAndBlock(dbus_constants::kServiceName,
- dbus::Bus::REQUIRE_PRIMARY))
- << "Unable to take ownership of " << dbus_constants::kServiceName;
-}
-
-void DBusManager::ShutDownDBus() {
- bus_->ShutdownAndBlock();
-}
-
-dbus::ExportedObject* DBusManager::GetExportedObject(
- const string& object_path) {
- return bus_->GetExportedObject(dbus::ObjectPath(object_path));
-}
-
-void DBusManager::ExportDBusMethod(
- dbus::ExportedObject* exported_object,
- const string& interface_name,
- const string& method_name,
- base::Callback<scoped_ptr<dbus::Response>(dbus::MethodCall*)> handler) {
- DCHECK(exported_object);
- CHECK(exported_object->ExportMethodAndBlock(
- interface_name, method_name,
- base::Bind(&HandleSynchronousDBusMethodCall, handler)));
+ dbus_utils::GetExportableDBusMethod(
+ base::Bind(&DBusManager::HandleTestMethod, base::Unretained(this))),
+ sequencer->GetExportHandler(
+ dbus_constants::kRootInterface, dbus_constants::kRootTestMethod,
+ "Failed exporting DBusManager's test method",
+ true));
+ sequencer->OnAllTasksCompletedCall({cb});
}
scoped_ptr<dbus::Response> DBusManager::HandleTestMethod(
diff --git a/buffet/dbus_manager.h b/buffet/dbus_manager.h
index a990ead..13aa8f4 100644
--- a/buffet/dbus_manager.h
+++ b/buffet/dbus_manager.h
@@ -17,10 +17,12 @@
// Class that manages dbus interactions in buffet.
class DBusManager {
public:
- DBusManager();
+ typedef base::Callback<void(bool success)> OnInitFinish;
+
+ DBusManager(dbus::Bus* bus);
virtual ~DBusManager();
- void Init();
+ void Init(const OnInitFinish& cb);
void Finalize();
// Get an object owned by the ::dbus::Bus object. This object
@@ -46,7 +48,8 @@
scoped_ptr<::dbus::Response> HandleTestMethod(
::dbus::MethodCall* method_call);
- scoped_refptr<::dbus::Bus> bus_;
+ scoped_refptr<::dbus::Bus> bus_; // Must outlive this object.
+ dbus::ExportedObject* exported_object_; // Owned by the bus.
DISALLOW_COPY_AND_ASSIGN(DBusManager);
};
diff --git a/buffet/dbus_utils.cc b/buffet/dbus_utils.cc
index fcda628..673afe3 100644
--- a/buffet/dbus_utils.cc
+++ b/buffet/dbus_utils.cc
@@ -2,14 +2,33 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include <base/logging.h>
-
#include "buffet/dbus_utils.h"
+#include <base/logging.h>
+#include <base/bind.h>
+
namespace buffet {
namespace dbus_utils {
+namespace {
+
+// Passes |method_call| to |handler| and passes the response to
+// |response_sender|. If |handler| returns NULL, an empty response is created
+// and sent.
+void HandleSynchronousDBusMethodCall(
+ base::Callback<scoped_ptr<dbus::Response>(dbus::MethodCall*)> handler,
+ dbus::MethodCall* method_call,
+ dbus::ExportedObject::ResponseSender response_sender) {
+ auto response = handler.Run(method_call);
+ if (!response)
+ response = dbus::Response::FromMethodCall(method_call);
+
+ response_sender.Run(response.Pass());
+}
+
+} // namespace
+
scoped_ptr<dbus::Response> GetBadArgsError(dbus::MethodCall* method_call,
const std::string& message) {
LOG(ERROR) << "Error while handling DBus call: " << message;
@@ -18,6 +37,11 @@
return scoped_ptr<dbus::Response>(resp.release());
}
+dbus::ExportedObject::MethodCallCallback GetExportableDBusMethod(
+ base::Callback<scoped_ptr<dbus::Response>(dbus::MethodCall*)> handler) {
+ return base::Bind(&HandleSynchronousDBusMethodCall, handler);
+}
+
} // namespace dbus_utils
} // namespace buffet
diff --git a/buffet/dbus_utils.h b/buffet/dbus_utils.h
index dea6d02..d4d5799 100644
--- a/buffet/dbus_utils.h
+++ b/buffet/dbus_utils.h
@@ -8,6 +8,7 @@
#include <string>
#include <base/memory/scoped_ptr.h>
+#include <dbus/exported_object.h>
#include <dbus/message.h>
namespace buffet {
@@ -17,6 +18,9 @@
scoped_ptr<dbus::Response> GetBadArgsError(dbus::MethodCall* method_call,
const std::string& message);
+dbus::ExportedObject::MethodCallCallback GetExportableDBusMethod(
+ base::Callback<scoped_ptr<dbus::Response>(dbus::MethodCall*)> handler);
+
} // namespace dbus_utils
} // namespace buffet
diff --git a/buffet/exported_property_set.cc b/buffet/exported_property_set.cc
index caf0c31..f6f7037 100644
--- a/buffet/exported_property_set.cc
+++ b/buffet/exported_property_set.cc
@@ -5,30 +5,50 @@
#include "buffet/exported_property_set.h"
#include <base/bind.h>
+#include <dbus/bus.h> // For kPropertyInterface
#include <dbus/property.h> // For kPropertyInterface
+#include "buffet/async_event_sequencer.h"
#include "buffet/dbus_utils.h"
namespace buffet {
namespace dbus_utils {
-ExportedPropertySet::ExportedPropertySet(dbus::ExportedObject* exported_object)
- : exported_object_(exported_object), weak_ptr_factory_(this) { }
+namespace {
+const char kExportFailedMessage[] = "Failed to register DBus method.";
+} // namespace
-void ExportedPropertySet::ClaimPropertiesInterface() {
- exported_object_->ExportMethodAndBlock(
+ExportedPropertySet::ExportedPropertySet(dbus::Bus* bus,
+ const dbus::ObjectPath& path)
+ : bus_(bus), exported_object_(bus->GetExportedObject(path)),
+ weak_ptr_factory_(this) { }
+
+void ExportedPropertySet::Init(const OnInitFinish& cb) {
+ bus_->AssertOnOriginThread();
+ scoped_refptr<AsyncEventSequencer> sequencer(new AsyncEventSequencer());
+ exported_object_->ExportMethod(
dbus::kPropertiesInterface, dbus::kPropertiesGetAll,
base::Bind(&ExportedPropertySet::HandleGetAll,
- weak_ptr_factory_.GetWeakPtr()));
- exported_object_->ExportMethodAndBlock(
+ weak_ptr_factory_.GetWeakPtr()),
+ sequencer->GetExportHandler(
+ dbus::kPropertiesInterface, dbus::kPropertiesGetAll,
+ kExportFailedMessage, false));
+ exported_object_->ExportMethod(
dbus::kPropertiesInterface, dbus::kPropertiesGet,
base::Bind(&ExportedPropertySet::HandleGet,
- weak_ptr_factory_.GetWeakPtr()));
- exported_object_->ExportMethodAndBlock(
+ weak_ptr_factory_.GetWeakPtr()),
+ sequencer->GetExportHandler(
+ dbus::kPropertiesInterface, dbus::kPropertiesGet,
+ kExportFailedMessage, false));
+ exported_object_->ExportMethod(
dbus::kPropertiesInterface, dbus::kPropertiesSet,
base::Bind(&ExportedPropertySet::HandleSet,
- weak_ptr_factory_.GetWeakPtr()));
+ weak_ptr_factory_.GetWeakPtr()),
+ sequencer->GetExportHandler(
+ dbus::kPropertiesInterface, dbus::kPropertiesSet,
+ kExportFailedMessage, false));
+ sequencer->OnAllTasksCompletedCall({cb});
}
ExportedPropertySet::~ExportedPropertySet() { }
@@ -37,6 +57,7 @@
const std::string& interface_name,
const std::string& property_name,
ExportedPropertyBase* exported_property) {
+ bus_->AssertOnOriginThread();
properties_[interface_name][property_name] = exported_property;
// Technically, the property set exists longer than the properties themselves,
// so we could use Unretained here rather than a weak pointer.
@@ -50,6 +71,7 @@
void ExportedPropertySet::HandleGetAll(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
+ bus_->AssertOnOriginThread();
dbus::MessageReader reader(method_call);
std::string interface_name;
if (!reader.PopString(&interface_name)) {
@@ -87,6 +109,7 @@
void ExportedPropertySet::HandleGet(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
+ bus_->AssertOnOriginThread();
dbus::MessageReader reader(method_call);
std::string interface_name, property_name;
if (!reader.PopString(&interface_name)) {
@@ -127,6 +150,7 @@
void ExportedPropertySet::HandleSet(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
+ bus_->AssertOnOriginThread();
scoped_ptr<dbus::ErrorResponse> error_resp(
dbus::ErrorResponse::FromMethodCall(
method_call, "org.freedesktop.DBus.Error.NotSupported", ""));
@@ -138,20 +162,9 @@
const std::string& interface,
const std::string& name,
const ExportedPropertyBase* property) {
+ bus_->AssertOnOriginThread();
dbus::Signal signal(dbus::kPropertiesInterface, dbus::kPropertiesChanged);
- WriteSignalForPropertyUpdate(interface, name, property, &signal);
- // This sends the signal asyncronously. However, the raw message inside
- // the signal object is ref-counted, so we're fine to allocate the Signal
- // object on our local stack.
- exported_object_->SendSignal(&signal);
-}
-
-void ExportedPropertySet::WriteSignalForPropertyUpdate(
- const std::string& interface,
- const std::string& name,
- const ExportedPropertyBase* property,
- dbus::Signal* signal) const {
- dbus::MessageWriter writer(signal);
+ dbus::MessageWriter writer(&signal);
dbus::MessageWriter array_writer(nullptr);
dbus::MessageWriter dict_writer(nullptr);
writer.AppendString(interface);
@@ -164,8 +177,12 @@
// The interface specification tells us to include this list of properties
// which have changed, but for whom no value is conveyed. Currently, we
// don't do anything interesting here.
- std::vector<std::string> invalidated_properties;
- writer.AppendArrayOfStrings(invalidated_properties);
+ writer.OpenArray("s", &array_writer);
+ writer.CloseContainer(&array_writer);
+ // This sends the signal asyncronously. However, the raw message inside
+ // the signal object is ref-counted, so we're fine to allocate the Signal
+ // object on our local stack.
+ exported_object_->SendSignal(&signal);
}
template <typename T>
diff --git a/buffet/exported_property_set.h b/buffet/exported_property_set.h
index 84dbe2a..c464a8b 100644
--- a/buffet/exported_property_set.h
+++ b/buffet/exported_property_set.h
@@ -94,13 +94,17 @@
class ExportedPropertySet {
public:
- ExportedPropertySet(dbus::ExportedObject* exported_object);
- ~ExportedPropertySet();
+ typedef base::Callback<void(bool success)> OnInitFinish;
- // Claims the org.freedesktop.DBus.Properties interface. This
- // needs to be done after all properties are initialized to
- // appropriate values.
- void ClaimPropertiesInterface();
+ ExportedPropertySet(dbus::Bus* bus, const dbus::ObjectPath& path);
+ virtual ~ExportedPropertySet();
+
+ // Claims the method associated with the org.freedesktop.DBus.Properties
+ // interface. This needs to be done after all properties are initialized to
+ // appropriate values. This method will call |cb| when all methods
+ // are exported to the DBus object. |cb| will be called on the origin
+ // thread.
+ void Init(const OnInitFinish& cb);
protected:
void RegisterProperty(const std::string& interface_name,
@@ -118,16 +122,11 @@
// Instead, use setters in exposed interfaces.
void HandleSet(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender);
+ void HandlePropertyUpdated(const std::string& interface,
+ const std::string& name,
+ const ExportedPropertyBase* property);
- virtual void HandlePropertyUpdated(const std::string& interface,
- const std::string& name,
- const ExportedPropertyBase* property);
-
- void WriteSignalForPropertyUpdate(const std::string& interface,
- const std::string& name,
- const ExportedPropertyBase* property,
- dbus::Signal* signal) const;
-
+ dbus::Bus* bus_;
dbus::ExportedObject* exported_object_; // weak; owned by the Bus object.
// This is a map from interface name -> property name -> pointer to property.
std::map<std::string,
diff --git a/buffet/exported_property_set_unittest.cc b/buffet/exported_property_set_unittest.cc
index 0bd3c3d..959a46a 100644
--- a/buffet/exported_property_set_unittest.cc
+++ b/buffet/exported_property_set_unittest.cc
@@ -12,9 +12,16 @@
#include <dbus/message.h>
#include <dbus/property.h>
#include <dbus/object_path.h>
+#include <dbus/mock_bus.h>
+#include <dbus/mock_exported_object.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+using ::testing::AnyNumber;
+using ::testing::Return;
+using ::testing::Invoke;
+using ::testing::_;
+
namespace buffet {
namespace dbus_utils {
@@ -41,6 +48,7 @@
const char kTestInterface3[] = "org.chromium.TestInterface3";
const std::string kTestString("lies");
+const dbus::ObjectPath kMethodsExportedOnPath(std::string("/export"));
const dbus::ObjectPath kTestObjectPathInit(std::string("/path_init"));
const dbus::ObjectPath kTestObjectPathUpdate(std::string("/path_update"));
@@ -48,7 +56,6 @@
class ExportedPropertySetTest : public ::testing::Test {
public:
- ExportedPropertySetTest() {}
struct Properties : public ExportedPropertySet {
public:
ExportedProperty<bool> bool_prop_;
@@ -66,7 +73,8 @@
ExportedProperty<std::vector<dbus::ObjectPath>> pathlist_prop_;
ExportedProperty<std::vector<uint8>> uint8list_prop_;
- Properties() : ExportedPropertySet(nullptr) {
+ Properties(dbus::Bus* bus, const dbus::ObjectPath& path)
+ : ExportedPropertySet(bus, path) {
// The empty string is not a valid value for an ObjectPath.
path_prop_.SetValue(kTestObjectPathInit);
RegisterProperty(kTestInterface1, kBoolPropName, &bool_prop_);
@@ -103,26 +111,23 @@
dbus::ExportedObject::ResponseSender response_sender) {
HandleSet(method_call, response_sender);
}
-
- void CallWriteSignalForPropertyUpdate(const std::string& interface,
- const std::string& name,
- const ExportedPropertyBase* property,
- dbus::Signal* signal) {
- WriteSignalForPropertyUpdate(interface, name, property, signal);
- }
-
- MOCK_METHOD3(PropertyUpdated, void(const std::string&, const std::string&,
- const ExportedPropertyBase*));
-
- private:
- virtual void HandlePropertyUpdated(
- const std::string& interface,
- const std::string& name,
- const ExportedPropertyBase* property) override {
- PropertyUpdated(interface, name, property);
- }
};
+ virtual void SetUp() {
+ dbus::Bus::Options options;
+ options.bus_type = dbus::Bus::SYSTEM;
+ bus_ = new dbus::MockBus(options);
+ // By default, don't worry about threading assertions.
+ EXPECT_CALL(*bus_, AssertOnOriginThread()).Times(AnyNumber());
+ EXPECT_CALL(*bus_, AssertOnDBusThread()).Times(AnyNumber());
+ // Use a mock exported object.
+ mock_exported_object_ = new dbus::MockExportedObject(
+ bus_.get(), kMethodsExportedOnPath);
+ EXPECT_CALL(*bus_, GetExportedObject(kMethodsExportedOnPath))
+ .Times(1).WillOnce(Return(mock_exported_object_.get()));
+ p_.reset(new Properties(bus_.get(), kMethodsExportedOnPath));
+ }
+
void StoreResponse(scoped_ptr<dbus::Response> method_response) {
last_response_.reset(method_response.release());
}
@@ -131,7 +136,7 @@
auto response_sender = base::Bind(&ExportedPropertySetTest::StoreResponse,
base::Unretained(this));
method_call->SetSerial(123);
- p_.CallHandleGetAll(method_call, response_sender);
+ p_->CallHandleGetAll(method_call, response_sender);
ASSERT_NE(dynamic_cast<dbus::ErrorResponse*>(last_response_.get()),
nullptr);
}
@@ -140,7 +145,7 @@
auto response_sender = base::Bind(&ExportedPropertySetTest::StoreResponse,
base::Unretained(this));
method_call->SetSerial(123);
- p_.CallHandleGet(method_call, response_sender);
+ p_->CallHandleGet(method_call, response_sender);
ASSERT_NE(dynamic_cast<dbus::ErrorResponse*>(last_response_.get()),
nullptr);
}
@@ -155,65 +160,38 @@
writer.AppendString(property_name);
auto response_sender = base::Bind(&ExportedPropertySetTest::StoreResponse,
base::Unretained(this));
- p_.CallHandleGet(&method_call, response_sender);
+ p_->CallHandleGet(&method_call, response_sender);
return last_response_.Pass();
}
scoped_ptr<dbus::Response> last_response_;
- Properties p_;
+ scoped_refptr<dbus::MockBus> bus_;
+ scoped_refptr<dbus::MockExportedObject> mock_exported_object_;
+ scoped_ptr<Properties> p_;
};
TEST_F(ExportedPropertySetTest, UpdateNotifications) {
- ::testing::InSequence dummy;
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface1, kBoolPropName,
- &p_.bool_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface1, kUint8PropName,
- &p_.uint8_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface1, kInt16PropName,
- &p_.int16_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface2, kUint16PropName,
- &p_.uint16_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface2, kInt32PropName,
- &p_.int32_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kUint32PropName,
- &p_.uint32_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kInt64PropName,
- &p_.int64_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kUint64PropName,
- &p_.uint64_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kDoublePropName,
- &p_.double_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kStringPropName,
- &p_.string_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kPathPropName,
- &p_.path_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kStringListPropName,
- &p_.stringlist_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kPathListPropName,
- &p_.pathlist_prop_));
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface3, kUint8ListPropName,
- &p_.uint8list_prop_));
- p_.bool_prop_.SetValue(true);
- p_.uint8_prop_.SetValue(1);
- p_.int16_prop_.SetValue(1);
- p_.uint16_prop_.SetValue(1);
- p_.int32_prop_.SetValue(1);
- p_.uint32_prop_.SetValue(1);
- p_.int64_prop_.SetValue(1);
- p_.uint64_prop_.SetValue(1);
- p_.double_prop_.SetValue(1.0);
- p_.string_prop_.SetValue(kTestString);
- p_.path_prop_.SetValue(kTestObjectPathUpdate);
- p_.stringlist_prop_.SetValue({kTestString});
- p_.pathlist_prop_.SetValue({kTestObjectPathUpdate});
- p_.uint8list_prop_.SetValue({1});
+ EXPECT_CALL(*mock_exported_object_, SendSignal(_)).Times(14);
+ p_->bool_prop_.SetValue(true);
+ p_->uint8_prop_.SetValue(1);
+ p_->int16_prop_.SetValue(1);
+ p_->uint16_prop_.SetValue(1);
+ p_->int32_prop_.SetValue(1);
+ p_->uint32_prop_.SetValue(1);
+ p_->int64_prop_.SetValue(1);
+ p_->uint64_prop_.SetValue(1);
+ p_->double_prop_.SetValue(1.0);
+ p_->string_prop_.SetValue(kTestString);
+ p_->path_prop_.SetValue(kTestObjectPathUpdate);
+ p_->stringlist_prop_.SetValue({kTestString});
+ p_->pathlist_prop_.SetValue({kTestObjectPathUpdate});
+ p_->uint8list_prop_.SetValue({1});
}
TEST_F(ExportedPropertySetTest, UpdateToSameValue) {
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface1, kBoolPropName,
- &p_.bool_prop_)).Times(1);
- p_.bool_prop_.SetValue(true);
- p_.bool_prop_.SetValue(true);
+ EXPECT_CALL(*mock_exported_object_, SendSignal(_)).Times(1);
+ p_->bool_prop_.SetValue(true);
+ p_->bool_prop_.SetValue(true);
}
TEST_F(ExportedPropertySetTest, GetAllNoArgs) {
@@ -247,7 +225,7 @@
writer.AppendString(kTestInterface2);
auto response_sender = base::Bind(&ExportedPropertySetTest::StoreResponse,
base::Unretained(this));
- p_.CallHandleGetAll(&method_call, response_sender);
+ p_->CallHandleGetAll(&method_call, response_sender);
dbus::MessageReader response_reader(last_response_.get());
dbus::MessageReader dict_reader(nullptr);
dbus::MessageReader entry_reader(nullptr);
@@ -472,24 +450,21 @@
method_call.SetSerial(123);
auto response_sender = base::Bind(&ExportedPropertySetTest::StoreResponse,
base::Unretained(this));
- p_.CallHandleSet(&method_call, response_sender);
+ p_->CallHandleSet(&method_call, response_sender);
ASSERT_TRUE(
dynamic_cast<dbus::ErrorResponse*>(last_response_.get()) != nullptr);
}
-TEST_F(ExportedPropertySetTest, SignalsAreParsable) {
- EXPECT_CALL(p_, PropertyUpdated(kTestInterface1, kUint8PropName,
- &p_.uint8_prop_)).Times(1);
- p_.uint8_prop_.SetValue(57);
- dbus::Signal signal(dbus::kPropertiesInterface, dbus::kPropertiesChanged);
- p_.CallWriteSignalForPropertyUpdate(kTestInterface1, kUint8PropName,
- &p_.uint8_prop_, &signal);
+namespace {
+
+void VerifySignal(dbus::Signal* signal) {
+ ASSERT_NE(signal, nullptr);
std::string interface_name;
std::string property_name;
uint8 value;
- dbus::MessageReader reader(&signal);
- dbus::MessageReader array_reader(nullptr);
- dbus::MessageReader dict_reader(nullptr);
+ dbus::MessageReader reader(signal);
+ dbus::MessageReader array_reader(signal);
+ dbus::MessageReader dict_reader(signal);
ASSERT_TRUE(reader.PopString(&interface_name));
ASSERT_TRUE(reader.PopArray(&array_reader));
ASSERT_TRUE(array_reader.PopDictEntry(&dict_reader));
@@ -497,15 +472,24 @@
ASSERT_TRUE(dict_reader.PopVariantOfByte(&value));
ASSERT_FALSE(dict_reader.HasMoreData());
ASSERT_FALSE(array_reader.HasMoreData());
+ ASSERT_TRUE(reader.HasMoreData());
// Read the (empty) list of invalidated property names.
- std::vector<std::string> invalidated_properties;
- ASSERT_TRUE(reader.PopArrayOfStrings(&invalidated_properties));
+ ASSERT_TRUE(reader.PopArray(&array_reader));
+ ASSERT_FALSE(array_reader.HasMoreData());
ASSERT_FALSE(reader.HasMoreData());
ASSERT_EQ(value, 57);
ASSERT_EQ(property_name, std::string(kUint8PropName));
ASSERT_EQ(interface_name, std::string(kTestInterface1));
}
+} // namespace
+
+TEST_F(ExportedPropertySetTest, SignalsAreParsable) {
+ EXPECT_CALL(*mock_exported_object_, SendSignal(_))
+ .Times(1).WillOnce(Invoke(&VerifySignal));
+ p_->uint8_prop_.SetValue(57);
+}
+
} // namespace dbus_utils
} // namespace buffet
diff --git a/buffet/main.cc b/buffet/main.cc
index 37a9031..6d8ddc2 100644
--- a/buffet/main.cc
+++ b/buffet/main.cc
@@ -14,9 +14,12 @@
#include <base/strings/stringprintf.h>
#include <sysexits.h>
+#include "buffet/async_event_sequencer.h"
#include "buffet/dbus_manager.h"
#include "buffet/manager.h"
+using buffet::dbus_utils::AsyncEventSequencer;
+
namespace {
static const char kLogRoot[] = "logroot";
@@ -72,6 +75,31 @@
logging::InitLogging(settings);
}
+void TakeServiceOwnership(scoped_refptr<dbus::Bus> bus, bool success) {
+ // Success should always be true since we've said that failures are
+ // fatal.
+ CHECK(success) << "Init of one or more objects has failed.";
+ CHECK(bus->RequestOwnershipAndBlock(buffet::dbus_constants::kServiceName,
+ dbus::Bus::REQUIRE_PRIMARY))
+ << "Unable to take ownership of " << buffet::dbus_constants::kServiceName;
+}
+
+void EnterMainLoop(base::MessageLoopForIO* message_loop,
+ scoped_refptr<dbus::Bus> bus) {
+ scoped_refptr<AsyncEventSequencer> sequencer(new AsyncEventSequencer());
+ buffet::DBusManager dbus_manager(bus.get());
+ dbus_manager.Init(sequencer->GetHandler("DBusManager.Init() failed.", true));
+ buffet::Manager manager(bus.get());
+ manager.Init(sequencer->GetHandler("Manager.Init() failed.", true));
+ sequencer->OnAllTasksCompletedCall(
+ {base::Bind(&TakeServiceOwnership, bus)});
+ // Release our handle on the sequencer so that it gets deleted after
+ // both callbacks return.
+ sequencer = nullptr;
+ LOG(INFO) << "Entering mainloop.";
+ message_loop->Run();
+}
+
} // namespace
int main(int argc, char* argv[]) {
@@ -94,15 +122,15 @@
base::AtExitManager at_exit_manager;
base::MessageLoopForIO message_loop;
- // Initialize the dbus_manager.
- buffet::DBusManager dbus_manager;
- dbus_manager.Init();
- {
- // The Manager needs the dbus_manager to remain in scope for its lifetime.
- buffet::Manager manager(&dbus_manager);
- message_loop.Run();
- }
+ dbus::Bus::Options options;
+ // TODO(sosa): Should this be on the system bus?
+ options.bus_type = dbus::Bus::SYSTEM;
+ scoped_refptr<dbus::Bus> bus(new dbus::Bus(options));
+ CHECK(bus->Connect());
+ // Our top level objects expect the bus to exist in a connected state for
+ // the duration of their lifetimes.
+ EnterMainLoop(&message_loop, bus);
+ bus->ShutdownAndBlock();
- dbus_manager.Finalize();
return EX_OK;
}
diff --git a/buffet/manager.cc b/buffet/manager.cc
index e0a9b7a..b23d639 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -6,7 +6,9 @@
#include <base/bind.h>
#include <base/bind_helpers.h>
+#include <dbus/object_path.h>
+#include "buffet/async_event_sequencer.h"
#include "buffet/dbus_constants.h"
#include "buffet/dbus_manager.h"
#include "buffet/dbus_utils.h"
@@ -15,25 +17,10 @@
namespace buffet {
-Manager::Manager(DBusManager* dbus_manager) : dbus_manager_(dbus_manager) {
- dbus::ExportedObject* exported_object = dbus_manager_->GetExportedObject(
- dbus_constants::kManagerServicePath);
- dbus_manager_->ExportDBusMethod(exported_object,
- dbus_constants::kManagerInterface,
- dbus_constants::kManagerRegisterDeviceMethod,
- base::Bind(&Manager::HandleRegisterDevice,
- base::Unretained(this)));
- dbus_manager_->ExportDBusMethod(exported_object,
- dbus_constants::kManagerInterface,
- dbus_constants::kManagerUpdateStateMethod,
- base::Bind(&Manager::HandleUpdateState,
- base::Unretained(this)));
- properties_.reset(new Properties(exported_object));
- // TODO(wiley): Initialize all properties appropriately before claiming
- // the properties interface.
- properties_->state_.SetValue("{}");
- properties_->ClaimPropertiesInterface();
-}
+Manager::Manager(dbus::Bus* bus)
+ : bus_(bus),
+ exported_object_(bus->GetExportedObject(
+ dbus::ObjectPath(dbus_constants::kManagerServicePath))) { }
Manager::~Manager() {
// Prevent the properties object from making calls to the exported object.
@@ -42,9 +29,42 @@
// our callbacks in between the Manager's death and the bus unregistering
// our exported object on shutdown. Unretained makes no promises of memory
// management.
- auto exported_object = dbus_manager_->GetExportedObject(
- dbus_constants::kManagerServicePath);
- exported_object->Unregister();
+ exported_object_->Unregister();
+ exported_object_ = nullptr;
+}
+
+void Manager::Init(const OnInitFinish& cb) {
+ scoped_refptr<dbus_utils::AsyncEventSequencer> sequencer(
+ new dbus_utils::AsyncEventSequencer());
+ exported_object_->ExportMethod(
+ dbus_constants::kManagerInterface,
+ dbus_constants::kManagerRegisterDeviceMethod,
+ dbus_utils::GetExportableDBusMethod(
+ base::Bind(&Manager::HandleRegisterDevice,
+ base::Unretained(this))),
+ sequencer->GetExportHandler(
+ dbus_constants::kManagerInterface,
+ dbus_constants::kManagerRegisterDeviceMethod,
+ "Failed exporting RegisterDevice method",
+ true));
+ exported_object_->ExportMethod(
+ dbus_constants::kManagerInterface,
+ dbus_constants::kManagerUpdateStateMethod,
+ dbus_utils::GetExportableDBusMethod(
+ base::Bind(&Manager::HandleUpdateState,
+ base::Unretained(this))),
+ sequencer->GetExportHandler(
+ dbus_constants::kManagerInterface,
+ dbus_constants::kManagerUpdateStateMethod,
+ "Failed exporting UpdateState method",
+ true));
+ properties_.reset(new Properties(bus_));
+ // TODO(wiley): Initialize all properties appropriately before claiming
+ // the properties interface.
+ properties_->state_.SetValue("{}");
+ properties_->Init(
+ sequencer->GetHandler("Manager properties export failed.", true));
+ sequencer->OnAllTasksCompletedCall({cb});
}
scoped_ptr<dbus::Response> Manager::HandleRegisterDevice(
diff --git a/buffet/manager.h b/buffet/manager.h
index cc65868..95d0548 100644
--- a/buffet/manager.h
+++ b/buffet/manager.h
@@ -8,6 +8,7 @@
#include <base/basictypes.h>
#include <base/memory/scoped_ptr.h>
#include <dbus/message.h>
+#include <dbus/object_path.h>
#include "buffet/dbus_constants.h"
#include "buffet/exported_property_set.h"
@@ -21,15 +22,19 @@
// device state.
class Manager {
public:
- Manager(DBusManager* dbus_manager);
+ typedef base::Callback<void(bool success)> OnInitFinish;
+
+ Manager(dbus::Bus* bus);
~Manager();
+ void Init(const OnInitFinish& cb);
private:
struct Properties: public dbus_utils::ExportedPropertySet {
public:
dbus_utils::ExportedProperty<std::string> state_;
- Properties(dbus::ExportedObject *manager_object)
- : dbus_utils::ExportedPropertySet(manager_object) {
+ Properties(dbus::Bus* bus)
+ : dbus_utils::ExportedPropertySet(
+ bus, dbus::ObjectPath(dbus_constants::kManagerServicePath)) {
RegisterProperty(dbus_constants::kManagerInterface, "State", &state_);
}
virtual ~Properties() {}
@@ -42,7 +47,8 @@
scoped_ptr<dbus::Response> HandleUpdateState(
dbus::MethodCall* method_call);
- DBusManager* dbus_manager_; // Weak; DBusManager should outlive Manager.
+ dbus::Bus* bus_;
+ dbus::ExportedObject* exported_object_; // weak; owned by the Bus object.
scoped_ptr<Properties> properties_;
DISALLOW_COPY_AND_ASSIGN(Manager);
