libbrillo: Support adding/removing interfaces and their properties.

D-Bus specification allows interfaces to be added or removed even after
an object is exported. This CL adds the support to libbrillo's D-Bus
library to do this.

This is needed by the new Bluetooth daemon which is going to impersonate
BlueZ's ObjectManager and BlueZ may add/remove interfaces of an object
dynamically.

BUG=chromium:812468
TEST=Tested with Bluetooth daemon

Change-Id: Ia72735bfd28082cf6c43d1e774beb912fa60ec2d
Reviewed-on: https://chromium-review.googlesource.com/956745
Commit-Ready: Sonny Sasaka <sonnysasaka@chromium.org>
Tested-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

6 files changed, 90 insertions, 0 deletions

diff --git a/brillo/dbus/dbus_object.cc b/brillo/dbus/dbus_object.cc
index 15bcddd..83c4c09 100644
--- a/brillo/dbus/dbus_object.cc
+++ b/brillo/dbus/dbus_object.cc
@@ -46,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 */,
@@ -214,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() << "'.";
diff --git a/brillo/dbus/dbus_object.h b/brillo/dbus/dbus_object.h
index 48f09ff..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:
@@ -538,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.
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/exported_property_set.cc b/brillo/dbus/exported_property_set.cc
index 8d6ae65..31a99be 100644
--- a/brillo/dbus/exported_property_set.cc
+++ b/brillo/dbus/exported_property_set.cc
@@ -54,6 +54,16 @@ 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_map.erase(prop_iter);
+}
+
 VariantDictionary ExportedPropertySet::HandleGetAll(
     const std::string& interface_name) {
   bus_->AssertOnOriginThread();
diff --git a/brillo/dbus/exported_property_set.h b/brillo/dbus/exported_property_set.h
index f10511f..3d4b14b 100644
--- a/brillo/dbus/exported_property_set.h
+++ b/brillo/dbus/exported_property_set.h
@@ -111,6 +111,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,
diff --git a/brillo/dbus/exported_property_set_unittest.cc b/brillo/dbus/exported_property_set_unittest.cc
index 9233b4a..4c03aef 100644
--- a/brillo/dbus/exported_property_set_unittest.cc
+++ b/brillo/dbus/exported_property_set_unittest.cc
@@ -336,6 +336,14 @@ 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());
+}
+
 TEST_F(ExportedPropertySetTest, GetWorksWithBool) {
   auto response = GetPropertyOnInterface(kTestInterface1, kBoolPropName);
   dbus::MessageReader reader(response.get());