aboutsummaryrefslogtreecommitdiffstats
path: root/brillo/dbus
diff options
context:
space:
mode:
Diffstat (limited to 'brillo/dbus')
-rw-r--r--brillo/dbus/data_serialization.cc9
-rw-r--r--brillo/dbus/data_serialization.h20
-rw-r--r--brillo/dbus/data_serialization_unittest.cc21
-rw-r--r--brillo/dbus/dbus_method_invoker.h26
-rw-r--r--brillo/dbus/dbus_method_invoker_unittest.cc146
-rw-r--r--brillo/dbus/dbus_object.cc91
-rw-r--r--brillo/dbus/dbus_object.h32
-rw-r--r--brillo/dbus/dbus_object_unittest.cc35
-rw-r--r--brillo/dbus/dbus_signal_handler.h10
-rw-r--r--brillo/dbus/exported_object_manager.h7
-rw-r--r--brillo/dbus/exported_property_set.cc15
-rw-r--r--brillo/dbus/exported_property_set.h9
-rw-r--r--brillo/dbus/exported_property_set_unittest.cc12
-rw-r--r--brillo/dbus/file_descriptor.h57
14 files changed, 347 insertions, 143 deletions
diff --git a/brillo/dbus/data_serialization.cc b/brillo/dbus/data_serialization.cc
index 746b241..4cae471 100644
--- a/brillo/dbus/data_serialization.cc
+++ b/brillo/dbus/data_serialization.cc
@@ -62,7 +62,7 @@ void AppendValueToWriter(dbus::MessageWriter* writer,
}
void AppendValueToWriter(dbus::MessageWriter* writer,
- const base::ScopedFD& value) {
+ const FileDescriptor& value) {
writer->AppendFileDescriptor(value.get());
}
@@ -142,9 +142,8 @@ bool PopValueFromReader(dbus::MessageReader* reader, dbus::ObjectPath* value) {
bool PopValueFromReader(dbus::MessageReader* reader,
base::ScopedFD* value) {
dbus::MessageReader variant_reader(nullptr);
- bool ok = details::DescendIntoVariantIfPresent(&reader, &variant_reader) &&
- reader->PopFileDescriptor(value);
- return ok;
+ return details::DescendIntoVariantIfPresent(&reader, &variant_reader) &&
+ reader->PopFileDescriptor(value);
}
namespace {
@@ -307,7 +306,7 @@ bool PopValueFromReader(dbus::MessageReader* reader, brillo::Any* value) {
return false;
case dbus::Message::UNIX_FD:
CHECK(dbus::IsDBusTypeUnixFdSupported()) << "UNIX_FD data not supported";
- // base::ScopedFD is not a copyable type. Cannot be returned via
+ // File descriptors don't use copyable types. Cannot be returned via
// brillo::Any. Fail here.
LOG(ERROR) << "Cannot return FileDescriptor via Any";
return false;
diff --git a/brillo/dbus/data_serialization.h b/brillo/dbus/data_serialization.h
index 6919b19..1600919 100644
--- a/brillo/dbus/data_serialization.h
+++ b/brillo/dbus/data_serialization.h
@@ -36,7 +36,8 @@
// | (UVW...) | std::tuple<U,V,W,...>
// DICT | a{KV} | std::map<K,V>
// VARIANT | v | brillo::Any
-// UNIX_FD | h | base::ScopedFD
+// UNIX_FD | h | brillo::dbus_utils::FileDescriptor (write)
+// | | base::ScopedFD (read)
// SIGNATURE | g | (unsupported)
//
// Additional overloads/specialization can be provided for custom types.
@@ -57,9 +58,11 @@
#include <utility>
#include <vector>
+#include <base/files/scoped_file.h>
#include <base/logging.h>
#include <base/files/scoped_file.h>
#include <brillo/brillo_export.h>
+#include <brillo/dbus/file_descriptor.h>
#include <brillo/type_name_undecorate.h>
#include <dbus/message.h>
@@ -412,21 +415,28 @@ struct DBusType<dbus::ObjectPath> {
}
};
-// base::ScopedFD -------------------------------------------------------------
+// brillo::dbus_utils::FileDescriptor/base::ScopedFD --------------------------
BRILLO_EXPORT void AppendValueToWriter(dbus::MessageWriter* writer,
- const base::ScopedFD& value);
+ const FileDescriptor& value);
BRILLO_EXPORT bool PopValueFromReader(dbus::MessageReader* reader,
base::ScopedFD* value);
template<>
-struct DBusType<base::ScopedFD> {
+struct DBusType<FileDescriptor> {
inline static std::string GetSignature() {
return DBUS_TYPE_UNIX_FD_AS_STRING;
}
inline static void Write(dbus::MessageWriter* writer,
- const base::ScopedFD& value) {
+ const FileDescriptor& value) {
AppendValueToWriter(writer, value);
}
+};
+
+template<>
+struct DBusType<base::ScopedFD> {
+ inline static std::string GetSignature() {
+ return DBUS_TYPE_UNIX_FD_AS_STRING;
+ }
inline static bool Read(dbus::MessageReader* reader,
base::ScopedFD* value) {
return PopValueFromReader(reader, value);
diff --git a/brillo/dbus/data_serialization_unittest.cc b/brillo/dbus/data_serialization_unittest.cc
index 8771fe9..c7d5e0f 100644
--- a/brillo/dbus/data_serialization_unittest.cc
+++ b/brillo/dbus/data_serialization_unittest.cc
@@ -12,7 +12,6 @@
#include "brillo/dbus/test.pb.h"
-using base::ScopedFD;
using dbus::Message;
using dbus::MessageReader;
using dbus::MessageWriter;
@@ -34,7 +33,8 @@ TEST(DBusUtils, Supported_BasicTypes) {
EXPECT_TRUE(IsTypeSupported<double>::value);
EXPECT_TRUE(IsTypeSupported<std::string>::value);
EXPECT_TRUE(IsTypeSupported<ObjectPath>::value);
- EXPECT_TRUE(IsTypeSupported<ScopedFD>::value);
+ EXPECT_TRUE(IsTypeSupported<FileDescriptor>::value);
+ EXPECT_TRUE(IsTypeSupported<base::ScopedFD>::value);
EXPECT_TRUE(IsTypeSupported<Any>::value);
EXPECT_TRUE(IsTypeSupported<google::protobuf::MessageLite>::value);
EXPECT_TRUE(IsTypeSupported<dbus_utils_test::TestMessage>::value);
@@ -90,7 +90,8 @@ TEST(DBusUtils, Signatures_BasicTypes) {
EXPECT_EQ("d", GetDBusSignature<double>());
EXPECT_EQ("s", GetDBusSignature<std::string>());
EXPECT_EQ("o", GetDBusSignature<ObjectPath>());
- EXPECT_EQ("h", GetDBusSignature<ScopedFD>());
+ EXPECT_EQ("h", GetDBusSignature<FileDescriptor>());
+ EXPECT_EQ("h", GetDBusSignature<base::ScopedFD>());
EXPECT_EQ("v", GetDBusSignature<Any>());
}
@@ -106,7 +107,8 @@ TEST(DBusUtils, Signatures_Arrays) {
EXPECT_EQ("ad", GetDBusSignature<std::vector<double>>());
EXPECT_EQ("as", GetDBusSignature<std::vector<std::string>>());
EXPECT_EQ("ao", GetDBusSignature<std::vector<ObjectPath>>());
- EXPECT_EQ("ah", GetDBusSignature<std::vector<ScopedFD>>());
+ EXPECT_EQ("ah", GetDBusSignature<std::vector<FileDescriptor>>());
+ EXPECT_EQ("ah", GetDBusSignature<std::vector<base::ScopedFD>>());
EXPECT_EQ("av", GetDBusSignature<std::vector<Any>>());
EXPECT_EQ("a(is)",
(GetDBusSignature<std::vector<std::pair<int, std::string>>>()));
@@ -239,24 +241,17 @@ TEST(DBusUtils, AppendAndPopFileDescriptor) {
MessageWriter writer(message.get());
// Append stdout.
- ScopedFD temp(1);
- // Descriptor should not be valid until checked.
- EXPECT_FALSE(temp.is_valid());
- // NB: thread IO requirements not relevant for unit tests.
- temp.CheckValidity();
- EXPECT_TRUE(temp.is_valid());
+ FileDescriptor temp = 1;
AppendValueToWriter(&writer, temp);
EXPECT_EQ("h", message->GetSignature());
- ScopedFD fd_value;
+ base::ScopedFD fd_value;
MessageReader reader(message.get());
EXPECT_TRUE(reader.HasMoreData());
EXPECT_TRUE(PopValueFromReader(&reader, &fd_value));
EXPECT_FALSE(reader.HasMoreData());
- // Descriptor is automatically checked for validity as part of
- // PopValueFromReader() call.
EXPECT_TRUE(fd_value.is_valid());
}
diff --git a/brillo/dbus/dbus_method_invoker.h b/brillo/dbus/dbus_method_invoker.h
index 2a8708b..f8b6990 100644
--- a/brillo/dbus/dbus_method_invoker.h
+++ b/brillo/dbus/dbus_method_invoker.h
@@ -67,8 +67,10 @@
#include <tuple>
#include <base/bind.h>
+#include <base/files/scoped_file.h>
#include <brillo/dbus/dbus_param_reader.h>
#include <brillo/dbus/dbus_param_writer.h>
+#include <brillo/dbus/file_descriptor.h>
#include <brillo/dbus/utils.h>
#include <brillo/errors/error.h>
#include <brillo/errors/error_codes.h>
@@ -140,6 +142,30 @@ inline std::unique_ptr<dbus::Response> CallMethodAndBlock(
args...);
}
+namespace internal {
+// In order to support non-copyable file descriptor types, we have this
+// internal::HackMove() helper function that does really nothing for normal
+// types but uses Pass() for file descriptors so we can move them out from
+// the temporaries created inside DBusParamReader<...>::Invoke().
+// If only libchrome supported real rvalues so we can just do std::move() and
+// be done with it.
+template <typename T>
+inline const T& HackMove(const T& val) {
+ return val;
+}
+
+// Even though |val| here is passed as const&, the actual value is created
+// inside DBusParamReader<...>::Invoke() and is temporary in nature, so it is
+// safe to move the file descriptor out of |val|. That's why we are doing
+// const_cast here. It is a bit hacky, but there is no negative side effects.
+inline base::ScopedFD HackMove(const base::ScopedFD& val) {
+ return std::move(const_cast<base::ScopedFD&>(val));
+}
+inline FileDescriptor HackMove(const FileDescriptor& val) {
+ return std::move(const_cast<FileDescriptor&>(val));
+}
+} // namespace internal
+
// Extracts the parameters of |ResultTypes...| types from the message reader
// and puts the values in the resulting |tuple|. Returns false on error and
// provides additional error details in |error| object.
diff --git a/brillo/dbus/dbus_method_invoker_unittest.cc b/brillo/dbus/dbus_method_invoker_unittest.cc
index dfd10c4..34f4c6f 100644
--- a/brillo/dbus/dbus_method_invoker_unittest.cc
+++ b/brillo/dbus/dbus_method_invoker_unittest.cc
@@ -26,6 +26,37 @@ using dbus::MessageReader;
using dbus::MessageWriter;
using dbus::Response;
+namespace {
+
+void SuccessCallback(const std::string& expected_result,
+ int* counter,
+ const std::string& actual_result) {
+ (*counter)++;
+ EXPECT_EQ(expected_result, actual_result);
+}
+
+void SimpleSuccessCallback(int* counter, const std::string& result) {
+ (*counter)++;
+}
+
+void ErrorCallback(const std::string& domain,
+ const std::string& code,
+ const std::string& message,
+ int* counter,
+ brillo::Error* error) {
+ (*counter)++;
+ ASSERT_NE(nullptr, error);
+ EXPECT_EQ(domain, error->GetDomain());
+ EXPECT_EQ(code, error->GetCode());
+ EXPECT_EQ(message, error->GetMessage());
+}
+
+void SimpleErrorCallback(int* counter, brillo::Error* error) {
+ (*counter)++;
+}
+
+} // namespace
+
namespace brillo {
namespace dbus_utils {
@@ -95,8 +126,7 @@ class DBusMethodInvokerTest : public testing::Test {
if (reader.PopFileDescriptor(&fd)) {
auto response = Response::CreateEmpty();
MessageWriter writer(response.get());
- fd.CheckValidity();
- writer.AppendFileDescriptor(fd);
+ writer.AppendFileDescriptor(fd.get());
return response.release();
}
}
@@ -131,19 +161,19 @@ class DBusMethodInvokerTest : public testing::Test {
return result;
}
- // Sends a file descriptor received over D-Bus back to the caller.
- base::ScopedFD EchoFD(const base::ScopedFD& fd_in) {
+ // Sends a file descriptor received over D-Bus back to the caller using the
+ // new types.
+ base::ScopedFD EchoFD(int fd_in) {
std::unique_ptr<dbus::Response> response =
- brillo::dbus_utils::CallMethodAndBlock(mock_object_proxy_.get(),
- kTestInterface, kTestMethod4,
- nullptr, fd_in);
+ brillo::dbus_utils::CallMethodAndBlock(
+ mock_object_proxy_.get(), kTestInterface, kTestMethod4,
+ nullptr, brillo::dbus_utils::FileDescriptor{fd_in});
EXPECT_NE(nullptr, response.get());
base::ScopedFD fd_out;
using brillo::dbus_utils::ExtractMethodCallResults;
EXPECT_TRUE(ExtractMethodCallResults(response.get(), nullptr, &fd_out));
return fd_out;
}
-
scoped_refptr<dbus::MockBus> bus_;
scoped_refptr<dbus::MockObjectProxy> mock_object_proxy_;
};
@@ -180,15 +210,18 @@ TEST_F(DBusMethodInvokerTest, TestFileDescriptors) {
// Passing a file descriptor over D-Bus would effectively duplicate the fd.
// So the resulting file descriptor value would be different but it still
// should be valid.
- base::ScopedFD fd_stdin(0);
- fd_stdin.CheckValidity();
- EXPECT_NE(fd_stdin.value(), EchoFD(fd_stdin).value());
- base::ScopedFD fd_stdout(1);
- fd_stdout.CheckValidity();
- EXPECT_NE(fd_stdout.value(), EchoFD(fd_stdout).value());
- base::ScopedFD fd_stderr(2);
- fd_stderr.CheckValidity();
- EXPECT_NE(fd_stderr.value(), EchoFD(fd_stderr).value());
+ int fd_stdin = 0;
+ base::ScopedFD out_fd = EchoFD(fd_stdin);
+ EXPECT_NE(fd_stdin, out_fd.get());
+ EXPECT_TRUE(out_fd.is_valid());
+ int fd_stdout = 1;
+ out_fd = EchoFD(fd_stdout);
+ EXPECT_NE(fd_stdout, out_fd.get());
+ EXPECT_TRUE(out_fd.is_valid());
+ int fd_stderr = 2;
+ out_fd = EchoFD(fd_stderr);
+ EXPECT_NE(fd_stderr, out_fd.get());
+ EXPECT_TRUE(out_fd.is_valid());
}
//////////////////////////////////////////////////////////////////////////////
@@ -246,62 +279,6 @@ class AsyncDBusMethodInvokerTest : public testing::Test {
LOG(FATAL) << "Unexpected method call: " << method_call->ToString();
}
- base::Callback<void(const std::string&)> SuccessCallback(
- const std::string& in_result, int* in_counter) {
- return base::Bind(
- [](const std::string& result,
- int* counter,
- const std::string& actual_result) {
- (*counter)++;
- EXPECT_EQ(result, actual_result);
- },
- in_result,
- base::Unretained(in_counter));
- }
-
- base::Callback<void(const std::string&)> SuccessCallback(int* in_counter) {
- return base::Bind(
- [](int* counter, const std::string& actual_result) {
- (*counter)++;
- EXPECT_EQ("", actual_result);
- },
- base::Unretained(in_counter));
- }
-
- AsyncErrorCallback ErrorCallback(int* in_counter) {
- return base::Bind(
- [](int* counter, brillo::Error* error) {
- (*counter)++;
- EXPECT_NE(nullptr, error);
- EXPECT_EQ("", error->GetDomain());
- EXPECT_EQ("", error->GetCode());
- EXPECT_EQ("", error->GetMessage());
- },
- base::Unretained(in_counter));
- }
-
- AsyncErrorCallback ErrorCallback(const std::string& domain,
- const std::string& code,
- const std::string& message,
- int* in_counter) {
- return base::Bind(
- [](const std::string& domain,
- const std::string& code,
- const std::string& message,
- int* counter,
- brillo::Error* error) {
- (*counter)++;
- EXPECT_NE(nullptr, error);
- EXPECT_EQ(domain, error->GetDomain());
- EXPECT_EQ(code, error->GetCode());
- EXPECT_EQ(message, error->GetMessage());
- },
- domain,
- code,
- message,
- base::Unretained(in_counter));
- }
-
scoped_refptr<dbus::MockBus> bus_;
scoped_refptr<dbus::MockObjectProxy> mock_object_proxy_;
};
@@ -313,22 +290,22 @@ TEST_F(AsyncDBusMethodInvokerTest, TestSuccess) {
mock_object_proxy_.get(),
kTestInterface,
kTestMethod1,
- base::Bind(SuccessCallback("4", &success_count)),
- base::Bind(ErrorCallback(&error_count)),
+ base::Bind(SuccessCallback, "4", &success_count),
+ base::Bind(SimpleErrorCallback, &error_count),
2, 2);
brillo::dbus_utils::CallMethod(
mock_object_proxy_.get(),
kTestInterface,
kTestMethod1,
- base::Bind(SuccessCallback("10", &success_count)),
- base::Bind(ErrorCallback(&error_count)),
+ base::Bind(SuccessCallback, "10", &success_count),
+ base::Bind(SimpleErrorCallback, &error_count),
3, 7);
brillo::dbus_utils::CallMethod(
mock_object_proxy_.get(),
kTestInterface,
kTestMethod1,
- base::Bind(SuccessCallback("-4", &success_count)),
- base::Bind(ErrorCallback(&error_count)),
+ base::Bind(SuccessCallback, "-4", &success_count),
+ base::Bind(SimpleErrorCallback, &error_count),
13, -17);
EXPECT_EQ(0, error_count);
EXPECT_EQ(3, success_count);
@@ -341,11 +318,12 @@ TEST_F(AsyncDBusMethodInvokerTest, TestFailure) {
mock_object_proxy_.get(),
kTestInterface,
kTestMethod2,
- base::Bind(SuccessCallback(&success_count)),
- base::Bind(ErrorCallback(brillo::errors::dbus::kDomain,
- "org.MyError",
- "My error message",
- &error_count)),
+ base::Bind(SimpleSuccessCallback, &success_count),
+ base::Bind(ErrorCallback,
+ brillo::errors::dbus::kDomain,
+ "org.MyError",
+ "My error message",
+ &error_count),
2, 2);
EXPECT_EQ(1, error_count);
EXPECT_EQ(0, success_count);
diff --git a/brillo/dbus/dbus_object.cc b/brillo/dbus/dbus_object.cc
index 59350a5..512cd6f 100644
--- a/brillo/dbus/dbus_object.cc
+++ b/brillo/dbus/dbus_object.cc
@@ -16,6 +16,23 @@
namespace brillo {
namespace dbus_utils {
+namespace {
+
+void SetupDefaultPropertyHandlers(DBusInterface* prop_interface,
+ ExportedPropertySet* property_set) {
+ prop_interface->AddSimpleMethodHandler(dbus::kPropertiesGetAll,
+ base::Unretained(property_set),
+ &ExportedPropertySet::HandleGetAll);
+ prop_interface->AddSimpleMethodHandlerWithError(
+ dbus::kPropertiesGet, base::Unretained(property_set),
+ &ExportedPropertySet::HandleGet);
+ prop_interface->AddSimpleMethodHandlerWithError(
+ dbus::kPropertiesSet, base::Unretained(property_set),
+ &ExportedPropertySet::HandleSet);
+}
+
+} // namespace
+
//////////////////////////////////////////////////////////////////////////////
DBusInterface::DBusInterface(DBusObject* dbus_object,
@@ -29,6 +46,11 @@ void DBusInterface::AddProperty(const std::string& property_name,
interface_name_, property_name, prop_base);
}
+void DBusInterface::RemoveProperty(const std::string& property_name) {
+ dbus_object_->property_set_.UnregisterProperty(interface_name_,
+ property_name);
+}
+
void DBusInterface::ExportAsync(
ExportedObjectManager* object_manager,
dbus::Bus* /* bus */,
@@ -152,7 +174,21 @@ void DBusInterface::AddSignalImpl(
DBusObject::DBusObject(ExportedObjectManager* object_manager,
const scoped_refptr<dbus::Bus>& bus,
const dbus::ObjectPath& object_path)
- : property_set_(bus.get()), bus_(bus), object_path_(object_path) {
+ : DBusObject::DBusObject(object_manager,
+ bus,
+ object_path,
+ base::Bind(&SetupDefaultPropertyHandlers)) {}
+
+DBusObject::DBusObject(
+ ExportedObjectManager* object_manager,
+ const scoped_refptr<dbus::Bus>& bus,
+ const dbus::ObjectPath& object_path,
+ PropertyHandlerSetupCallback property_handler_setup_callback)
+ : property_set_(bus.get()),
+ bus_(bus),
+ object_path_(object_path),
+ property_handler_setup_callback_(
+ std::move(property_handler_setup_callback)) {
if (object_manager)
object_manager_ = object_manager->AsWeakPtr();
}
@@ -183,6 +219,21 @@ DBusInterface* DBusObject::FindInterface(
return (itf_iter == interfaces_.end()) ? nullptr : itf_iter->second.get();
}
+void DBusObject::RemoveInterface(const std::string& interface_name) {
+ auto itf_iter = interfaces_.find(interface_name);
+ CHECK(itf_iter != interfaces_.end())
+ << "Interface " << interface_name << " has not been added.";
+ interfaces_.erase(itf_iter);
+}
+
+void DBusObject::ExportInterfaceAsync(
+ const std::string& interface_name,
+ const AsyncEventSequencer::CompletionAction& completion_callback) {
+ AddOrGetInterface(interface_name)
+ ->ExportAsync(object_manager_.get(), bus_.get(), exported_object_,
+ object_path_, completion_callback);
+}
+
void DBusObject::RegisterAsync(
const AsyncEventSequencer::CompletionAction& completion_callback) {
VLOG(1) << "Registering D-Bus object '" << object_path_.value() << "'.";
@@ -190,21 +241,7 @@ void DBusObject::RegisterAsync(
scoped_refptr<AsyncEventSequencer> sequencer(new AsyncEventSequencer());
exported_object_ = bus_->GetExportedObject(object_path_);
- // Add the org.freedesktop.DBus.Properties interface to the object.
- DBusInterface* prop_interface = AddOrGetInterface(dbus::kPropertiesInterface);
- prop_interface->AddSimpleMethodHandler(
- dbus::kPropertiesGetAll,
- base::Unretained(&property_set_),
- &ExportedPropertySet::HandleGetAll);
- prop_interface->AddSimpleMethodHandlerWithError(
- dbus::kPropertiesGet,
- base::Unretained(&property_set_),
- &ExportedPropertySet::HandleGet);
- prop_interface->AddSimpleMethodHandlerWithError(
- dbus::kPropertiesSet,
- base::Unretained(&property_set_),
- &ExportedPropertySet::HandleSet);
- property_set_.OnPropertiesInterfaceExported(prop_interface);
+ RegisterPropertiesInterface();
// Export interface methods
for (const auto& pair : interfaces_) {
@@ -225,21 +262,7 @@ void DBusObject::RegisterAndBlock() {
CHECK(exported_object_ == nullptr) << "Object already registered.";
exported_object_ = bus_->GetExportedObject(object_path_);
- // Add the org.freedesktop.DBus.Properties interface to the object.
- DBusInterface* prop_interface = AddOrGetInterface(dbus::kPropertiesInterface);
- prop_interface->AddSimpleMethodHandler(
- dbus::kPropertiesGetAll,
- base::Unretained(&property_set_),
- &ExportedPropertySet::HandleGetAll);
- prop_interface->AddSimpleMethodHandlerWithError(
- dbus::kPropertiesGet,
- base::Unretained(&property_set_),
- &ExportedPropertySet::HandleGet);
- prop_interface->AddSimpleMethodHandlerWithError(
- dbus::kPropertiesSet,
- base::Unretained(&property_set_),
- &ExportedPropertySet::HandleSet);
- property_set_.OnPropertiesInterfaceExported(prop_interface);
+ RegisterPropertiesInterface();
// Export interface methods
for (const auto& pair : interfaces_) {
@@ -275,5 +298,11 @@ bool DBusObject::SendSignal(dbus::Signal* signal) {
return false;
}
+void DBusObject::RegisterPropertiesInterface() {
+ DBusInterface* prop_interface = AddOrGetInterface(dbus::kPropertiesInterface);
+ property_handler_setup_callback_.Run(prop_interface, &property_set_);
+ property_set_.OnPropertiesInterfaceExported(prop_interface);
+}
+
} // namespace dbus_utils
} // namespace brillo
diff --git a/brillo/dbus/dbus_object.h b/brillo/dbus/dbus_object.h
index 6cd34dd..61c954f 100644
--- a/brillo/dbus/dbus_object.h
+++ b/brillo/dbus/dbus_object.h
@@ -372,6 +372,9 @@ class BRILLO_EXPORT DBusInterface final {
void AddProperty(const std::string& property_name,
ExportedPropertyBase* prop_base);
+ // Unregisters a D-Bus property.
+ void RemoveProperty(const std::string& property_name);
+
// Registers a D-Bus signal that has a specified number and types (|Args|) of
// arguments. Returns a weak pointer to the DBusSignal object which can be
// used to send the signal on this interface when needed:
@@ -507,6 +510,9 @@ class BRILLO_EXPORT DBusInterface final {
// by this object.
class BRILLO_EXPORT DBusObject {
public:
+ using PropertyHandlerSetupCallback = base::Callback<void(
+ DBusInterface* prop_interface, ExportedPropertySet* property_set)>;
+
// object_manager - ExportedObjectManager instance that notifies D-Bus
// listeners of a new interface being claimed and property
// changes on those interfaces.
@@ -514,6 +520,17 @@ class BRILLO_EXPORT DBusObject {
DBusObject(ExportedObjectManager* object_manager,
const scoped_refptr<dbus::Bus>& bus,
const dbus::ObjectPath& object_path);
+
+ // property_handler_setup_callback - To be called when setting up property
+ // method handlers. Clients can register
+ // their own custom property method handlers
+ // (GetAll/Get/Set) by passing in this
+ // callback.
+ DBusObject(ExportedObjectManager* object_manager,
+ const scoped_refptr<dbus::Bus>& bus,
+ const dbus::ObjectPath& object_path,
+ PropertyHandlerSetupCallback property_handler_setup_callback);
+
virtual ~DBusObject();
// Returns an proxy handler for the interface |interface_name|. If the
@@ -524,6 +541,16 @@ class BRILLO_EXPORT DBusObject {
// interface registered by this name.
DBusInterface* FindInterface(const std::string& interface_name) const;
+ // Removes the previously added proxy handler for the interface
+ // |interface_name|.
+ void RemoveInterface(const std::string& interface_name);
+
+ // Exports a proxy handler for the interface |interface_name|. If the
+ // interface proxy does not exist yet, it will be automatically created.
+ void ExportInterfaceAsync(
+ const std::string& interface_name,
+ const AsyncEventSequencer::CompletionAction& completion_callback);
+
// Registers the object instance with D-Bus. This is an asynchronous call
// that will call |completion_callback| when the object and all of its
// interfaces are registered.
@@ -555,6 +582,9 @@ class BRILLO_EXPORT DBusObject {
scoped_refptr<dbus::Bus> GetBus() { return bus_; }
private:
+ // Add the org.freedesktop.DBus.Properties interface to the object.
+ void RegisterPropertiesInterface();
+
// A map of all the interfaces added to this object.
std::map<std::string, std::unique_ptr<DBusInterface>> interfaces_;
// Exported property set for properties registered with the interfaces
@@ -568,6 +598,8 @@ class BRILLO_EXPORT DBusObject {
dbus::ObjectPath object_path_;
// D-Bus object instance once this object is successfully exported.
dbus::ExportedObject* exported_object_ = nullptr; // weak; owned by |bus_|.
+ // Sets up property method handlers.
+ PropertyHandlerSetupCallback property_handler_setup_callback_;
friend class DBusInterface;
DISALLOW_COPY_AND_ASSIGN(DBusObject);
diff --git a/brillo/dbus/dbus_object_unittest.cc b/brillo/dbus/dbus_object_unittest.cc
index eaec3d8..932a5c8 100644
--- a/brillo/dbus/dbus_object_unittest.cc
+++ b/brillo/dbus/dbus_object_unittest.cc
@@ -43,6 +43,8 @@ const char kTestMethod_NoOp[] = "NoOp";
const char kTestMethod_WithMessage[] = "TestWithMessage";
const char kTestMethod_WithMessageAsync[] = "TestWithMessageAsync";
+const char kTestInterface4[] = "org.chromium.Test.LateInterface";
+
struct Calc {
int Add(int x, int y) { return x + y; }
int Negate(int x) { return -x; }
@@ -89,6 +91,10 @@ void TestWithMessageAsync(
response->Return(message->GetSender());
}
+void OnInterfaceExported(bool success) {
+ // Does nothing.
+}
+
} // namespace
class DBusObjectTest : public ::testing::Test {
@@ -315,6 +321,35 @@ TEST_F(DBusObjectTest, TestWithMessageAsync) {
EXPECT_EQ(sender, message);
}
+TEST_F(DBusObjectTest, TestRemovedInterface) {
+ // Removes the interface to be tested.
+ dbus_object_->RemoveInterface(kTestInterface3);
+
+ const std::string sender{":1.2345"};
+ dbus::MethodCall method_call(kTestInterface3, kTestMethod_WithMessage);
+ method_call.SetSerial(123);
+ method_call.SetSender(sender);
+ auto response = testing::CallMethod(*dbus_object_, &method_call);
+ // The response should contain error UnknownInterface since the interface has
+ // been intentionally removed.
+ EXPECT_EQ(DBUS_ERROR_UNKNOWN_INTERFACE, response->GetErrorName());
+}
+
+TEST_F(DBusObjectTest, TestInterfaceExportedLate) {
+ // Registers a new interface late.
+ dbus_object_->ExportInterfaceAsync(kTestInterface4,
+ base::Bind(&OnInterfaceExported));
+
+ const std::string sender{":1.2345"};
+ dbus::MethodCall method_call(kTestInterface4, kTestMethod_WithMessage);
+ method_call.SetSerial(123);
+ method_call.SetSender(sender);
+ auto response = testing::CallMethod(*dbus_object_, &method_call);
+ // The response should contain error UnknownMethod rather than
+ // UnknownInterface since the interface has been registered late.
+ EXPECT_EQ(DBUS_ERROR_UNKNOWN_METHOD, response->GetErrorName());
+}
+
TEST_F(DBusObjectTest, TooFewParams) {
dbus::MethodCall method_call(kTestInterface1, kTestMethod_Add);
method_call.SetSerial(123);
diff --git a/brillo/dbus/dbus_signal_handler.h b/brillo/dbus/dbus_signal_handler.h
index 9c3246c..15cdae1 100644
--- a/brillo/dbus/dbus_signal_handler.h
+++ b/brillo/dbus/dbus_signal_handler.h
@@ -5,6 +5,7 @@
#ifndef LIBBRILLO_BRILLO_DBUS_DBUS_SIGNAL_HANDLER_H_
#define LIBBRILLO_BRILLO_DBUS_DBUS_SIGNAL_HANDLER_H_
+#include <functional>
#include <string>
#include <brillo/bind_lambda.h>
@@ -58,10 +59,11 @@ void ConnectToSignal(
};
// Register our stub handler with D-Bus ObjectProxy.
- object_proxy->ConnectToSignal(interface_name,
- signal_name,
- base::Bind(dbus_signal_callback, signal_callback),
- on_connected_callback);
+ object_proxy->ConnectToSignal(
+ interface_name,
+ signal_name,
+ base::Bind(dbus_signal_callback, signal_callback),
+ on_connected_callback);
}
} // namespace dbus_utils
diff --git a/brillo/dbus/exported_object_manager.h b/brillo/dbus/exported_object_manager.h
index 01dab5b..ea68f33 100644
--- a/brillo/dbus/exported_object_manager.h
+++ b/brillo/dbus/exported_object_manager.h
@@ -108,6 +108,13 @@ class BRILLO_EXPORT ExportedObjectManager
const scoped_refptr<dbus::Bus>& GetBus() const { return bus_; }
+ // Due to D-Bus forwarding, clients may need to access the underlying
+ // DBusObject to handle signals/methods.
+ // TODO(sonnysasaka): Refactor this accessor into a stricter API once we know
+ // what D-Bus forwarding needs when it's completed, without exposing
+ // DBusObject directly.
+ brillo::dbus_utils::DBusObject* dbus_object() { return &dbus_object_; };
+
private:
BRILLO_PRIVATE ObjectMap HandleGetManagedObjects();
diff --git a/brillo/dbus/exported_property_set.cc b/brillo/dbus/exported_property_set.cc
index 8d6ae65..018843e 100644
--- a/brillo/dbus/exported_property_set.cc
+++ b/brillo/dbus/exported_property_set.cc
@@ -54,6 +54,17 @@ void ExportedPropertySet::RegisterProperty(
exported_property->SetUpdateCallback(cb);
}
+void ExportedPropertySet::UnregisterProperty(const std::string& interface_name,
+ const std::string& property_name) {
+ bus_->AssertOnOriginThread();
+ auto& prop_map = properties_[interface_name];
+ auto prop_iter = prop_map.find(property_name);
+ CHECK(prop_iter != prop_map.end())
+ << "Property '" << property_name << "' doesn't exist";
+ prop_iter->second->ClearUpdateCallback();
+ prop_map.erase(prop_iter);
+}
+
VariantDictionary ExportedPropertySet::HandleGetAll(
const std::string& interface_name) {
bus_->AssertOnOriginThread();
@@ -160,6 +171,10 @@ void ExportedPropertyBase::SetUpdateCallback(const OnUpdateCallback& cb) {
on_update_callback_ = cb;
}
+void ExportedPropertyBase::ClearUpdateCallback() {
+ on_update_callback_.Reset();
+}
+
void ExportedPropertyBase::SetAccessMode(
ExportedPropertyBase::Access access_mode) {
access_mode_ = access_mode;
diff --git a/brillo/dbus/exported_property_set.h b/brillo/dbus/exported_property_set.h
index f10511f..971e932 100644
--- a/brillo/dbus/exported_property_set.h
+++ b/brillo/dbus/exported_property_set.h
@@ -71,6 +71,9 @@ class BRILLO_EXPORT ExportedPropertyBase {
// interface of the exported object.
virtual void SetUpdateCallback(const OnUpdateCallback& cb);
+ // Clears the update callback that was previously set with SetUpdateCallback.
+ virtual void ClearUpdateCallback();
+
// Returns the contained value as Any.
virtual brillo::Any GetValue() const = 0;
@@ -111,6 +114,10 @@ class BRILLO_EXPORT ExportedPropertySet {
const std::string& property_name,
ExportedPropertyBase* exported_property);
+ // Unregisters a property from this exported property set.
+ void UnregisterProperty(const std::string& interface_name,
+ const std::string& property_name);
+
// D-Bus methods for org.freedesktop.DBus.Properties interface.
VariantDictionary HandleGetAll(const std::string& interface_name);
bool HandleGet(brillo::ErrorPtr* error,
@@ -208,7 +215,7 @@ class ExportedProperty : public ExportedPropertyBase {
if (!validator_.is_null() && !validator_.Run(error, value.Get<T>())) {
return false;
}
- value_ = value.Get<T>();
+ SetValue(value.Get<T>());
return true;
}
diff --git a/brillo/dbus/exported_property_set_unittest.cc b/brillo/dbus/exported_property_set_unittest.cc
index 9233b4a..93aceb4 100644
--- a/brillo/dbus/exported_property_set_unittest.cc
+++ b/brillo/dbus/exported_property_set_unittest.cc
@@ -336,6 +336,18 @@ TEST_F(ExportedPropertySetTest, GetExtraArgs) {
AssertMethodReturnsError(&method_call);
}
+TEST_F(ExportedPropertySetTest, GetRemovedProperty) {
+ DBusInterface* itf1 = p_->dbus_object_.AddOrGetInterface(kTestInterface1);
+ itf1->RemoveProperty(kBoolPropName);
+
+ auto response = GetPropertyOnInterface(kTestInterface1, kBoolPropName);
+ ASSERT_EQ(DBUS_ERROR_UNKNOWN_PROPERTY, response->GetErrorName());
+
+ // Signal should not be emitted for removed property.
+ EXPECT_CALL(*mock_exported_object_, SendSignal(_)).Times(0);
+ p_->bool_prop_.SetValue(true);
+}
+
TEST_F(ExportedPropertySetTest, GetWorksWithBool) {
auto response = GetPropertyOnInterface(kTestInterface1, kBoolPropName);
dbus::MessageReader reader(response.get());
diff --git a/brillo/dbus/file_descriptor.h b/brillo/dbus/file_descriptor.h
new file mode 100644
index 0000000..f7be44f
--- /dev/null
+++ b/brillo/dbus/file_descriptor.h
@@ -0,0 +1,57 @@
+// Copyright 2018 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIBBRILLO_BRILLO_DBUS_FILE_DESCRIPTOR_H_
+#define LIBBRILLO_BRILLO_DBUS_FILE_DESCRIPTOR_H_
+
+#include <base/files/scoped_file.h>
+#include <base/macros.h>
+
+namespace brillo {
+namespace dbus_utils {
+
+// This struct wraps file descriptors to give them a type other than int.
+// Implicit conversions are provided because this should be as transparent
+// a wrapper as possible to match the libchrome bindings below when this
+// class is used by chromeos-dbus-bindings.
+//
+// Because we might pass these around and the calling code neither passes
+// ownership nor knows when this will be destroyed, it actually dups the FD
+// so that the calling code and binding code both have a clear handle on the
+// lifetimes of their respective copies of the FD.
+struct FileDescriptor {
+ FileDescriptor() = default;
+ FileDescriptor(int fd) : fd(dup(fd)) {}
+ FileDescriptor(FileDescriptor&& other) : fd(std::move(other.fd)) {}
+ FileDescriptor(base::ScopedFD&& other) : fd(std::move(other)) {}
+
+ inline FileDescriptor& operator=(int new_fd) {
+ fd.reset(dup(new_fd));
+ return *this;
+ }
+
+ FileDescriptor& operator=(FileDescriptor&& other) {
+ fd = std::move(other.fd);
+ return *this;
+ }
+
+ FileDescriptor& operator=(base::ScopedFD&& other) {
+ fd = std::move(other);
+ return *this;
+ }
+
+ int release() { return fd.release(); }
+
+ int get() const { return fd.get(); }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(FileDescriptor);
+
+ base::ScopedFD fd;
+};
+
+} // namespace dbus_utils
+} // namespace brillo
+
+#endif // LIBBRILLO_BRILLO_DBUS_FILE_DESCRIPTOR_H_