chromeos-dbus-bindings: Add "ignore_interfaces" option to config.json

Some services (e.g. Avahi) include system interfaces such as
org.freedesktop.DBus.Introspectable as part of their object
description.

This causes problems when generating multiple proxies since the
same interface is included more than once and the resutling proxy
code fails to compile.

Add an optional setting "ignore_interfaces" to the config .json
file to allow to list the interfaces we are not interested in.

Also, it turns out that a lot of places use "/" object path
legitimately, so I removed that extra hacky check for "/" path
and treating it as if not specified. If some objects do use
"/" incorrectly, their XML files should be fixed to omit
<name> attribute on interface completely.

BUG=chromium:438685
TEST=FEATURES=test emerge-link chromeos-dbus-bindings

Change-Id: I692e4621f3d36e683a21f96b72bb147b2dd8d88a
Reviewed-on: https://chromium-review.googlesource.com/232865
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>

5 files changed, 52 insertions, 21 deletions

diff --git a/chromeos-dbus-bindings/generate_chromeos_dbus_bindings.cc b/chromeos-dbus-bindings/generate_chromeos_dbus_bindings.cc
index a4d108e..4189bbf 100644
--- a/chromeos-dbus-bindings/generate_chromeos_dbus_bindings.cc
+++ b/chromeos-dbus-bindings/generate_chromeos_dbus_bindings.cc
@@ -94,6 +94,23 @@ bool LoadConfig(const base::FilePath& path, ServiceConfig *config) {
     }
     om_dict->GetStringWithoutPathExpansion("object_path",
                                            &config->object_manager.object_path);
+    if (config->object_manager.name.empty()) {
+      LOG(ERROR) << "Object manager name is missing.";
+      return false;
+    }
+  }
+
+  base::ListValue* list = nullptr;  // Owned by |dict|.
+  if (dict->GetListWithoutPathExpansion("ignore_interfaces", &list)) {
+    config->ignore_interfaces.reserve(list->GetSize());
+    for (base::Value* item : *list) {
+      std::string interface_name;
+      if (!item->GetAsString(&interface_name)) {
+        LOG(ERROR) << "Invalid interface name in [ignore_interfaces] section";
+        return false;
+      }
+      config->ignore_interfaces.push_back(interface_name);
+    }
   }
 
   return true;
@@ -121,6 +138,16 @@ int main(int argc, char** argv) {
     return 1;
   }
 
+  ServiceConfig config;
+  if (cl->HasSwitch(switches::kServiceConfig)) {
+    std::string config_file = cl->GetSwitchValueASCII(switches::kServiceConfig);
+    if (!config_file.empty() &&
+        !LoadConfig(SanitizeFilePath(config_file), &config)) {
+      LOG(ERROR) << "Failed to load DBus service config file " << config_file;
+      return 1;
+    }
+  }
+
   chromeos_dbus_bindings::XmlInterfaceParser parser;
   for (const auto& input : input_files) {
     std::string contents;
@@ -128,22 +155,12 @@ int main(int argc, char** argv) {
       LOG(ERROR) << "Failed to read file " << input;
       return 1;
     }
-    if (!parser.ParseXmlInterfaceFile(contents)) {
+    if (!parser.ParseXmlInterfaceFile(contents, config.ignore_interfaces)) {
       LOG(ERROR) << "Failed to parse interface file.";
       return 1;
     }
   }
 
-  ServiceConfig config;
-  if (cl->HasSwitch(switches::kServiceConfig)) {
-    std::string config_file = cl->GetSwitchValueASCII(switches::kServiceConfig);
-    if (!config_file.empty() &&
-        !LoadConfig(SanitizeFilePath(config_file), &config)) {
-      LOG(ERROR) << "Failed to load DBus service config file " << config_file;
-      return 1;
-    }
-  }
-
   if (cl->HasSwitch(switches::kMethodNames)) {
     std::string method_name_file =
         cl->GetSwitchValueASCII(switches::kMethodNames);
diff --git a/chromeos-dbus-bindings/header_generator.h b/chromeos-dbus-bindings/header_generator.h
index 1be7d34..0c28bd7 100644
--- a/chromeos-dbus-bindings/header_generator.h
+++ b/chromeos-dbus-bindings/header_generator.h
@@ -40,6 +40,10 @@ struct ServiceConfig {
     // The D-Bus path to Object Manager instance.
     std::string object_path;
   } object_manager;
+
+  // A list of interfaces we should ignore and not generate any adaptors and
+  // proxies for.
+  std::vector<std::string> ignore_interfaces;
 };
 
 class HeaderGenerator {
diff --git a/chromeos-dbus-bindings/xml_interface_parser.cc b/chromeos-dbus-bindings/xml_interface_parser.cc
index 2146ebd..46c36ac 100644
--- a/chromeos-dbus-bindings/xml_interface_parser.cc
+++ b/chromeos-dbus-bindings/xml_interface_parser.cc
@@ -56,7 +56,9 @@ string GetElementPath(const vector<string>& path) {
 
 }  // anonymous namespace
 
-bool XmlInterfaceParser::ParseXmlInterfaceFile(const std::string& contents) {
+bool XmlInterfaceParser::ParseXmlInterfaceFile(
+    const std::string& contents,
+    const std::vector<std::string>& ignore_interfaces) {
   auto parser = XML_ParserCreate(nullptr);
   XML_SetUserData(parser, this);
   XML_SetElementHandler(parser,
@@ -78,6 +80,16 @@ bool XmlInterfaceParser::ParseXmlInterfaceFile(const std::string& contents) {
   }
 
   CHECK(element_path_.empty());
+
+  if (!ignore_interfaces.empty()) {
+    // Remove interfaces whose names are in |ignore_interfaces| list.
+    auto condition = [&ignore_interfaces](const Interface& itf) {
+      return std::find(ignore_interfaces.begin(), ignore_interfaces.end(),
+                       itf.name) != ignore_interfaces.end();
+    };
+    auto p = std::remove_if(interfaces_.begin(), interfaces_.end(), condition);
+    interfaces_.erase(p, interfaces_.end());
+  }
   return true;
 }
 
@@ -93,12 +105,7 @@ void XmlInterfaceParser::OnOpenElement(
     // 'name' attribute is optional for <node> element.
     string name;
     GetElementAttribute(attributes, element_path_, kNameAttribute, &name);
-    // Treat object path of "/" as empty/unspecified, since that happens a lot
-    // in existing XML files. People don't know that 'name' can be omitted, so
-    // they use "/" to denote some fictional D-Bus path for the object.
     base::TrimWhitespaceASCII(name, base::TRIM_ALL, &name);
-    if (name == "/")
-      name.clear();
     node_names_.push_back(name);
   } else if (element_name == kInterfaceTag) {
     CHECK_EQ(kNodeTag, prev_element)
diff --git a/chromeos-dbus-bindings/xml_interface_parser.h b/chromeos-dbus-bindings/xml_interface_parser.h
index 431d0dc..219ea0d 100644
--- a/chromeos-dbus-bindings/xml_interface_parser.h
+++ b/chromeos-dbus-bindings/xml_interface_parser.h
@@ -30,7 +30,8 @@ class XmlInterfaceParser {
   XmlInterfaceParser() = default;
   virtual ~XmlInterfaceParser() = default;
 
-  virtual bool ParseXmlInterfaceFile(const std::string& contents);
+  bool ParseXmlInterfaceFile(const std::string& contents,
+                             const std::vector<std::string>& ignore_interfaces);
   const std::vector<Interface>& interfaces() const { return interfaces_; }
 
  private:
diff --git a/chromeos-dbus-bindings/xml_interface_parser_unittest.cc b/chromeos-dbus-bindings/xml_interface_parser_unittest.cc
index dcb163a..1d332ae 100644
--- a/chromeos-dbus-bindings/xml_interface_parser_unittest.cc
+++ b/chromeos-dbus-bindings/xml_interface_parser_unittest.cc
@@ -38,6 +38,7 @@ const char kGoodInterfaceFileContents[] = R"literal_string(
       <arg name="BSS" type="o"/>
     </signal>
   </interface>
+  <interface name="DummyInterface" />
   <node name="/"/>
   <node/>
 </node>
@@ -64,12 +65,13 @@ class XmlInterfaceParserTest : public Test {
 };
 
 TEST_F(XmlInterfaceParserTest, BadInputFile) {
-  EXPECT_FALSE(parser_.ParseXmlInterfaceFile(kBadInterfaceFileContents0));
-  EXPECT_FALSE(parser_.ParseXmlInterfaceFile(kBadInterfaceFileContents1));
+  EXPECT_FALSE(parser_.ParseXmlInterfaceFile(kBadInterfaceFileContents0, {}));
+  EXPECT_FALSE(parser_.ParseXmlInterfaceFile(kBadInterfaceFileContents1, {}));
 }
 
 TEST_F(XmlInterfaceParserTest, GoodInputFile) {
-  EXPECT_TRUE(parser_.ParseXmlInterfaceFile(kGoodInterfaceFileContents));
+  EXPECT_TRUE(parser_.ParseXmlInterfaceFile(kGoodInterfaceFileContents,
+                                            {"DummyInterface"}));
   const vector<Interface>& interfaces = parser_.interfaces();
   ASSERT_EQ(1, interfaces.size());
   const Interface& interface = interfaces.back();