chromeos-dbus-bindings: Use object path and service name in generated code

To eliminate unnecessary dependencies on string constants from D-Bus
service providers, allow generated proxies extract fixed object paths
from the 'name' attribute of <node> in the XML file (if available) and
be able to pass in additional the D-Bus service configuration parameters
(such as D-Bus service name) as input to the generator, so that the proxy
code knows these values too and the caller doesn't have to provide this
information.

With these changes the object path and/or service names become embedded
into the generated proxies and are no longer required to be provided as
parameters to the constructors. If there are multiple instances of D-Bus
objects with the same interface are available (or the object could
appear at different bus paths), the path cannot be embedded into
the proxy and must still be provided in the constructor of the generated
class.

The service config can now be provided in --service-config=... argument
to the generator which is expected to be a JSON file.
If the service name it is not specified, the generated proxy code will
require this parameter in the constructor call, as before.

Also updated other targets that use the proxy generator to follow the
new constructor syntax (buffet, peerd, privetd).

BUG=chromium:431737
TEST=FEATURES=test emerge-link chromeos-dbus-bindings peerd buffet privetd

Change-Id: I4bb8387a9b21b75e3508fa13d14b79fbe653c929
Reviewed-on: https://chromium-review.googlesource.com/231920
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>

11 files changed, 418 insertions, 142 deletions

diff --git a/chromeos-dbus-bindings/adaptor_generator.cc b/chromeos-dbus-bindings/adaptor_generator.cc
index d1cb882..5ffdf70 100644
--- a/chromeos-dbus-bindings/adaptor_generator.cc
+++ b/chromeos-dbus-bindings/adaptor_generator.cc
@@ -73,7 +73,7 @@ void AdaptorGenerator::GenerateInterfaceAdaptor(
   text->AddBlankLine();
   text->AddLine(StringPrintf("// Interface definition for %s.",
                              full_itf_name.c_str()));
-  text->AddComments(interface.doc_string_);
+  text->AddComments(interface.doc_string);
   text->AddLine(StringPrintf("class %s {", itf_name.c_str()));
   text->AddLineWithOffset("public:", kScopeOffset);
   text->PushOffset(kBlockOffset);
@@ -92,6 +92,15 @@ void AdaptorGenerator::GenerateInterfaceAdaptor(
   AddRegisterWithDBusObject(itf_name, interface, text);
   AddSendSignalMethods(interface, text);
   AddPropertyMethodImplementation(interface, text);
+  if (!interface.path.empty()) {
+    text->AddBlankLine();
+    text->AddLine("static dbus::ObjectPath GetObjectPath() {");
+    text->PushOffset(kBlockOffset);
+    text->AddLine(StringPrintf("return dbus::ObjectPath{\"%s\"};",
+                               interface.path.c_str()));
+    text->PopOffset();
+    text->AddLine("}");
+  }
   text->PopOffset();
 
   text->AddBlankLine();
@@ -245,7 +254,7 @@ void AdaptorGenerator::AddInterfaceMethods(const Interface& interface,
         output_arguments_copy.clear();
         break;
     }
-    block.AddComments(method.doc_string_);
+    block.AddComments(method.doc_string);
     string method_start = StringPrintf("virtual %s %s(",
                                        return_type.c_str(),
                                        method.name.c_str());
@@ -293,7 +302,7 @@ void AdaptorGenerator::AddSendSignalMethods(
     block.AddBlankLine();
 
   for (const auto& signal : interface.signals) {
-    block.AddComments(signal.doc_string_);
+    block.AddComments(signal.doc_string);
     string method_start = StringPrintf("void Send%sSignal(",
                                        signal.name.c_str());
     string method_end = ") {";
@@ -388,7 +397,7 @@ void AdaptorGenerator::AddPropertyMethodImplementation(
     string variable_name = GetPropertyVariableName(property.name);
 
     // Getter method.
-    block.AddComments(property.doc_string_);
+    block.AddComments(property.doc_string);
     block.AddLine(StringPrintf("%s Get%s() const {",
                                type.c_str(),
                                property.name.c_str()));
diff --git a/chromeos-dbus-bindings/adaptor_generator_unittest.cc b/chromeos-dbus-bindings/adaptor_generator_unittest.cc
index 9076823..583b1ea 100644
--- a/chromeos-dbus-bindings/adaptor_generator_unittest.cc
+++ b/chromeos-dbus-bindings/adaptor_generator_unittest.cc
@@ -138,6 +138,10 @@ class TestAdaptor {
     character_name_.SetValue(character_name);
   }
 
+  static dbus::ObjectPath GetObjectPath() {
+    return dbus::ObjectPath{"/org/chromium/Test"};
+  }
+
  private:
   using SignalUpdateType = chromeos::dbus_utils::DBusSignal<>;
   std::weak_ptr<SignalUpdateType> signal_Update_;
@@ -224,6 +228,7 @@ class AdaptorGeneratorTest : public Test {
 TEST_F(AdaptorGeneratorTest, GenerateAdaptors) {
   Interface interface;
   interface.name = kInterfaceName;
+  interface.path = "/org/chromium/Test";
   interface.methods.emplace_back(
       kMethod0Name,
       vector<Interface::Argument>{
diff --git a/chromeos-dbus-bindings/generate_chromeos_dbus_bindings.cc b/chromeos-dbus-bindings/generate_chromeos_dbus_bindings.cc
index e45292e..946156b 100644
--- a/chromeos-dbus-bindings/generate_chromeos_dbus_bindings.cc
+++ b/chromeos-dbus-bindings/generate_chromeos_dbus_bindings.cc
@@ -2,12 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
 #include <string>
 
 #include <base/command_line.h>
 #include <base/files/file_path.h>
+#include <base/files/file_util.h>
+#include <base/json/json_reader.h>
 #include <base/logging.h>
 #include <base/strings/string_util.h>
+#include <base/values.h>
 #include <chromeos/syslog_logging.h>
 
 #include "chromeos-dbus-bindings/adaptor_generator.h"
@@ -18,6 +22,7 @@
 using chromeos_dbus_bindings::AdaptorGenerator;
 using chromeos_dbus_bindings::MethodNameGenerator;
 using chromeos_dbus_bindings::ProxyGenerator;
+using chromeos_dbus_bindings::ServiceConfig;
 
 namespace switches {
 
@@ -25,6 +30,7 @@ static const char kHelp[] = "help";
 static const char kMethodNames[] = "method-names";
 static const char kAdaptor[] = "adaptor";
 static const char kProxy[] = "proxy";
+static const char kServiceConfig[] = "service-config";
 static const char kHelpMessage[] = "\n"
     "generate-chromeos-dbus-bindings itf1.xml [itf2.xml...] [switches]\n"
     "    itf1.xml, ... = the input interface file(s) [mandatory].\n"
@@ -34,10 +40,55 @@ static const char kHelpMessage[] = "\n"
     "  --adaptor=<adaptor header filename>\n"
     "    The output header file name containing the DBus adaptor class.\n"
     "  --proxy=<proxy header filename>\n"
-    "    The output header file name containing the DBus proxy class.\n";
+    "    The output header file name containing the DBus proxy class.\n"
+    "  --service-config=<config.json>\n"
+    "    The DBus service configuration file for the generator.\n";
 
 }  // namespace switches
 
+namespace {
+// GYP sometimes enclosed the target file name in extra set of quotes like:
+//    generate-chromeos-dbus-bindings in.xml "--adaptor=\"out.h\""
+// So, this function helps us to remove them.
+base::FilePath RemoveQuotes(const std::string& path) {
+  std::string unquoted;
+  base::TrimString(path, "\"'", &unquoted);
+  return base::FilePath{unquoted};
+}
+
+// Makes a canonical path by making the path absolute and by removing any
+// '..' which makes base::ReadFileToString() to fail.
+base::FilePath SanitizeFilePath(const std::string& path) {
+  base::FilePath path_in = RemoveQuotes(path);
+  base::FilePath path_out = base::MakeAbsoluteFilePath(path_in);
+  if (path_out.value().empty()) {
+    LOG(WARNING) << "Failed to canonicalize '" << path << "'";
+    path_out = path_in;
+  }
+  return path_out;
+}
+
+
+// Load the service configuration from the provided JSON file.
+bool LoadConfig(const base::FilePath& path, ServiceConfig *config) {
+  std::string contents;
+  if (!base::ReadFileToString(path, &contents))
+    return false;
+
+  std::unique_ptr<base::Value> json{base::JSONReader::Read(contents)};
+  if (!json)
+    return false;
+
+  base::DictionaryValue* dict = nullptr;  // Aliased with |json|.
+  if (!json->GetAsDictionary(&dict))
+    return false;
+
+  dict->GetStringWithoutPathExpansion("service_name", &config->service_name);
+  return true;
+}
+
+}   // anonymous namespace
+
 int main(int argc, char** argv) {
   CommandLine::Init(argc, argv);
   CommandLine* cl = CommandLine::ForCurrentProcess();
@@ -60,19 +111,34 @@ int main(int argc, char** argv) {
 
   chromeos_dbus_bindings::XmlInterfaceParser parser;
   for (const auto& input : input_files) {
-    if (!parser.ParseXmlInterfaceFile(base::FilePath(input))) {
+    std::string contents;
+    if (!base::ReadFileToString(SanitizeFilePath(input), &contents)) {
+      LOG(ERROR) << "Failed to read file " << input;
+      return 1;
+    }
+    if (!parser.ParseXmlInterfaceFile(contents)) {
       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);
     VLOG(1) << "Outputting method names to " << method_name_file;
     if (!MethodNameGenerator::GenerateMethodNames(
             parser.interfaces(),
-            base::FilePath(method_name_file))) {
+            RemoveQuotes(method_name_file))) {
       LOG(ERROR) << "Failed to output method names.";
       return 1;
     }
@@ -80,12 +146,9 @@ int main(int argc, char** argv) {
 
   if (cl->HasSwitch(switches::kAdaptor)) {
     std::string adaptor_file = cl->GetSwitchValueASCII(switches::kAdaptor);
-    // GYP sometimes enclosed the target file name in extra set of quotes like:
-    // generate-chromeos-dbus-bindings in.xml "--adaptor=\"out.h\""
-    base::TrimString(adaptor_file, "\"'", &adaptor_file);
     VLOG(1) << "Outputting adaptor to " << adaptor_file;
     if (!AdaptorGenerator::GenerateAdaptors(parser.interfaces(),
-                                            base::FilePath(adaptor_file))) {
+                                            RemoveQuotes(adaptor_file))) {
       LOG(ERROR) << "Failed to output adaptor.";
       return 1;
      }
@@ -93,12 +156,9 @@ int main(int argc, char** argv) {
 
   if (cl->HasSwitch(switches::kProxy)) {
     std::string proxy_file = cl->GetSwitchValueASCII(switches::kProxy);
-    // GYP sometimes enclosed the target file name in extra set of quotes like:
-    // generate-chromeos-dbus-bindings in.xml "--proxy=\"out.h\""
-    base::TrimString(proxy_file, "\"'", &proxy_file);
     VLOG(1) << "Outputting proxy to " << proxy_file;
-    if (!ProxyGenerator::GenerateProxies(parser.interfaces(),
-                                         base::FilePath(proxy_file))) {
+    if (!ProxyGenerator::GenerateProxies(config, parser.interfaces(),
+                                         RemoveQuotes(proxy_file))) {
       LOG(ERROR) << "Failed to output proxy.";
       return 1;
      }
diff --git a/chromeos-dbus-bindings/header_generator.h b/chromeos-dbus-bindings/header_generator.h
index e4896ca..938e93b 100644
--- a/chromeos-dbus-bindings/header_generator.h
+++ b/chromeos-dbus-bindings/header_generator.h
@@ -21,6 +21,10 @@ namespace chromeos_dbus_bindings {
 struct Interface;
 class  IndentedText;
 
+struct ServiceConfig {
+  std::string service_name;
+};
+
 class HeaderGenerator {
  protected:
   // Create a unique header guard string to protect multiple includes of header.
diff --git a/chromeos-dbus-bindings/interface.h b/chromeos-dbus-bindings/interface.h
index b7f5b0e..db1c8b1 100644
--- a/chromeos-dbus-bindings/interface.h
+++ b/chromeos-dbus-bindings/interface.h
@@ -38,7 +38,7 @@ struct Interface {
     std::string name;
     std::vector<Argument> input_arguments;
     std::vector<Argument> output_arguments;
-    std::string doc_string_;
+    std::string doc_string;
     Kind kind{Kind::kNormal};
     bool is_const{false};
   };
@@ -49,7 +49,7 @@ struct Interface {
     explicit Signal(const std::string& name_in) : name(name_in) {}
     std::string name;
     std::vector<Argument> arguments;
-    std::string doc_string_;
+    std::string doc_string;
   };
   struct Property {
     Property(const std::string& name_in,
@@ -59,7 +59,7 @@ struct Interface {
     std::string name;
     std::string type;
     std::string access;
-    std::string doc_string_;
+    std::string doc_string;
   };
 
   Interface() = default;
@@ -70,10 +70,11 @@ struct Interface {
       : name(name_in), methods(methods_in), signals(signals_in),
         properties(properties_in) {}
   std::string name;
+  std::string path;
   std::vector<Method> methods;
   std::vector<Signal> signals;
   std::vector<Property> properties;
-  std::string doc_string_;
+  std::string doc_string;
 };
 
 }  // namespace chromeos_dbus_bindings
diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index ca6efd7..17d299a 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -4,6 +4,8 @@
 
 #include "chromeos-dbus-bindings/proxy_generator.h"
 
+#include <utility>
+
 #include <base/logging.h>
 #include <base/strings/stringprintf.h>
 #include <chromeos/strings/string_utils.h>
@@ -12,13 +14,22 @@
 #include "chromeos-dbus-bindings/indented_text.h"
 
 using base::StringPrintf;
+using std::pair;
 using std::string;
 using std::vector;
 
 namespace chromeos_dbus_bindings {
 
+namespace {
+string GetParamString(const pair<string, string>& param_def) {
+  return StringPrintf("const %s& %s",
+                      param_def.first.c_str(), param_def.second.c_str());
+}
+}  // anonymous namespace
+
 // static
 bool ProxyGenerator::GenerateProxies(
+    const ServiceConfig& config,
     const std::vector<Interface>& interfaces,
     const base::FilePath& output_file) {
   IndentedText text;
@@ -50,7 +61,7 @@ bool ProxyGenerator::GenerateProxies(
   text.AddBlankLine();
 
   for (const auto& interface : interfaces) {
-    GenerateInterfaceProxy(interface, &text);
+    GenerateInterfaceProxy(config, interface, &text);
   }
 
   text.AddLine(StringPrintf("#endif  // %s", header_guard.c_str()));
@@ -58,7 +69,8 @@ bool ProxyGenerator::GenerateProxies(
 }
 
 // static
-void ProxyGenerator::GenerateInterfaceProxy(const Interface& interface,
+void ProxyGenerator::GenerateInterfaceProxy(const ServiceConfig& config,
+                                            const Interface& interface,
                                             IndentedText* text) {
   vector<string> namespaces;
   string itf_name;
@@ -78,7 +90,7 @@ void ProxyGenerator::GenerateInterfaceProxy(const Interface& interface,
   text->AddLineWithOffset("public:", kScopeOffset);
   text->PushOffset(kBlockOffset);
   AddSignalReceiver(interface, text);
-  AddConstructor(interface, proxy_name, text);
+  AddConstructor(config, interface, proxy_name, text);
   AddDestructor(proxy_name, text);
   AddReleaseObjectProxy(text);
   if (!interface.signals.empty())
@@ -92,8 +104,18 @@ void ProxyGenerator::GenerateInterfaceProxy(const Interface& interface,
 
   text->PushOffset(kBlockOffset);
   text->AddLine("scoped_refptr<dbus::Bus> bus_;");
-  text->AddLine("std::string service_name_;");
-  text->AddLine("dbus::ObjectPath object_path_;");
+  if (config.service_name.empty()) {
+    text->AddLine("std::string service_name_;");
+  } else {
+    text->AddLine(StringPrintf("const std::string service_name_{\"%s\"};",
+                               config.service_name.c_str()));
+  }
+  if (interface.path.empty()) {
+    text->AddLine("dbus::ObjectPath object_path_;");
+  } else {
+    text->AddLine(StringPrintf("const dbus::ObjectPath object_path_{\"%s\"};",
+                               interface.path.c_str()));
+  }
   text->AddLine("dbus::ObjectProxy* dbus_object_proxy_;");
   text->AddBlankLine();
 
@@ -112,36 +134,56 @@ void ProxyGenerator::GenerateInterfaceProxy(const Interface& interface,
 }
 
 // static
-void ProxyGenerator::AddConstructor(const Interface& interface,
+void ProxyGenerator::AddConstructor(const ServiceConfig& config,
+                                    const Interface& interface,
                                     const string& class_name,
                                     IndentedText* text) {
   IndentedText block;
-  block.AddLine(StringPrintf("%s(", class_name.c_str()));
+  vector<std::pair<string, string>> args{{"scoped_refptr<dbus::Bus>", "bus"}};
+  if (config.service_name.empty())
+    args.emplace_back("std::string", "service_name");
+  if (interface.path.empty())
+    args.emplace_back("std::string", "object_path");
+
+  if (args.size() == 1) {
+    block.AddLine(StringPrintf("%s(%s) :", class_name.c_str(),
+                               GetParamString(args.front()).c_str()));
+  } else {
+    block.AddLine(StringPrintf("%s(", class_name.c_str()));
+    block.PushOffset(kLineContinuationOffset);
+    for (size_t i = 0; i < args.size() - 1; i++) {
+      block.AddLine(StringPrintf("%s,", GetParamString(args[i]).c_str()));
+    }
+    block.AddLine(StringPrintf("%s) :", GetParamString(args.back()).c_str()));
+  }
   block.PushOffset(kLineContinuationOffset);
-  block.AddLine("const scoped_refptr<dbus::Bus>& bus,");
-  block.AddLine("const std::string& service_name,");
-  block.AddLine("const std::string& object_path)");
-  block.AddLine(": bus_(bus),");
-  block.PushOffset(kBlockOffset);
-  block.AddLine("service_name_(service_name),");
-  block.AddLine("object_path_(object_path),");
+  for (const auto& arg : args) {
+    block.AddLine(StringPrintf("%s_(%s),", arg.second.c_str(),
+                               arg.second.c_str()));
+  }
   block.AddLine("dbus_object_proxy_(");
   block.AddLineWithOffset(
       "bus_->GetObjectProxy(service_name_, object_path_)) {",
       kLineContinuationOffset);
   block.PopOffset();
-  block.PopOffset();
+  if (args.size() > 1)
+    block.PopOffset();
   block.AddLine("}");
   if (!interface.signals.empty()) {
     block.AddBlankLine();
     block.AddLine(StringPrintf("%s(", class_name.c_str()));
     block.PushOffset(kLineContinuationOffset);
-    block.AddLine("const scoped_refptr<dbus::Bus>& bus,");
-    block.AddLine("const std::string& service_name,");
-    block.AddLine("const std::string& object_path,");
-    block.AddLine("SignalReceiver* signal_receiver)");
-    block.AddLine(StringPrintf(": %s(bus, service_name, object_path) {",
-                               class_name.c_str()));
+    vector<string> param_names;
+    for (const auto& arg : args) {
+      block.AddLine(StringPrintf("%s,", GetParamString(arg).c_str()));
+      param_names.push_back(arg.second);
+    }
+    block.AddLine("SignalReceiver* signal_receiver) :");
+    string param_list = chromeos::string_utils::Join(", ", param_names);
+    block.PushOffset(kLineContinuationOffset);
+    block.AddLine(StringPrintf("%s(%s) {", class_name.c_str(),
+                               param_list.c_str()));
+    block.PopOffset();
     block.PopOffset();
     block.PushOffset(kBlockOffset);
     for (const auto& signal : interface.signals) {
@@ -233,7 +275,7 @@ void ProxyGenerator::AddSignalReceiver(const Interface& interface,
   block.PushOffset(kBlockOffset);
   DbusSignature signature;
   for (const auto& signal : interface.signals) {
-    block.AddComments(signal.doc_string_);
+    block.AddComments(signal.doc_string);
     string signal_begin = StringPrintf(
         "virtual void %s(", GetHandlerNameForSignal(signal.name).c_str());
     string signal_end = ") {}";
@@ -275,7 +317,7 @@ void ProxyGenerator::AddMethodProxy(const Interface::Method& method,
                                     IndentedText* text) {
   IndentedText block;
   DbusSignature signature;
-  block.AddComments(method.doc_string_);
+  block.AddComments(method.doc_string);
   block.AddLine(StringPrintf("bool %s(", method.name.c_str()));
   block.PushOffset(kLineContinuationOffset);
   vector<string> argument_names;
diff --git a/chromeos-dbus-bindings/proxy_generator.h b/chromeos-dbus-bindings/proxy_generator.h
index 591648d..c0c909b 100644
--- a/chromeos-dbus-bindings/proxy_generator.h
+++ b/chromeos-dbus-bindings/proxy_generator.h
@@ -27,18 +27,21 @@ struct Interface;
 
 class ProxyGenerator : public HeaderGenerator {
  public:
-  static bool GenerateProxies(const std::vector<Interface>& interfaces,
+  static bool GenerateProxies(const ServiceConfig& config,
+                              const std::vector<Interface>& interfaces,
                               const base::FilePath& output_file);
 
  private:
   friend class ProxyGeneratorTest;
 
   // Generates one interface proxy.
-  static void GenerateInterfaceProxy(const Interface& interface,
+  static void GenerateInterfaceProxy(const ServiceConfig& config,
+                                     const Interface& interface,
                                      IndentedText* text);
 
   // Generates the constructor and destructor for the proxy.
-  static void AddConstructor(const Interface& interface,
+  static void AddConstructor(const ServiceConfig& config,
+                             const Interface& interface,
                              const std::string& class_name,
                              IndentedText* text);
   static void AddDestructor(const std::string& class_name,
diff --git a/chromeos-dbus-bindings/proxy_generator_unittest.cc b/chromeos-dbus-bindings/proxy_generator_unittest.cc
index 0b35fc8..e4f596e 100644
--- a/chromeos-dbus-bindings/proxy_generator_unittest.cc
+++ b/chromeos-dbus-bindings/proxy_generator_unittest.cc
@@ -79,21 +79,18 @@ class TestInterfaceProxy final {
 
   TestInterfaceProxy(
       const scoped_refptr<dbus::Bus>& bus,
-      const std::string& service_name,
-      const std::string& object_path)
-      : bus_(bus),
-        service_name_(service_name),
-        object_path_(object_path),
-        dbus_object_proxy_(
-            bus_->GetObjectProxy(service_name_, object_path_)) {
+      const std::string& service_name) :
+          bus_(bus),
+          service_name_(service_name),
+          dbus_object_proxy_(
+              bus_->GetObjectProxy(service_name_, object_path_)) {
   }
 
   TestInterfaceProxy(
       const scoped_refptr<dbus::Bus>& bus,
       const std::string& service_name,
-      const std::string& object_path,
-      SignalReceiver* signal_receiver)
-      : TestInterfaceProxy(bus, service_name, object_path) {
+      SignalReceiver* signal_receiver) :
+          TestInterfaceProxy(bus, service_name) {
     chromeos::dbus_utils::ConnectToSignal(
         dbus_object_proxy_,
         "org.chromium.TestInterface",
@@ -192,7 +189,7 @@ class TestInterfaceProxy final {
  private:
   scoped_refptr<dbus::Bus> bus_;
   std::string service_name_;
-  dbus::ObjectPath object_path_;
+  const dbus::ObjectPath object_path_{"/org/chromium/Test"};
   dbus::ObjectProxy* dbus_object_proxy_;
 
   DISALLOW_COPY_AND_ASSIGN(TestInterfaceProxy);
@@ -210,12 +207,12 @@ class TestInterface2Proxy final {
   TestInterface2Proxy(
       const scoped_refptr<dbus::Bus>& bus,
       const std::string& service_name,
-      const std::string& object_path)
-      : bus_(bus),
-        service_name_(service_name),
-        object_path_(object_path),
-        dbus_object_proxy_(
-            bus_->GetObjectProxy(service_name_, object_path_)) {
+      const std::string& object_path) :
+          bus_(bus),
+          service_name_(service_name),
+          object_path_(object_path),
+          dbus_object_proxy_(
+              bus_->GetObjectProxy(service_name_, object_path_)) {
   }
 
   ~TestInterface2Proxy() {
@@ -252,6 +249,125 @@ class TestInterface2Proxy final {
 
 )literal_string";
 
+const char kExpectedContentWithService[] = R"literal_string(
+#include <string>
+#include <vector>
+
+#include <base/bind.h>
+#include <base/callback.h>
+#include <base/logging.h>
+#include <base/macros.h>
+#include <base/memory/ref_counted.h>
+#include <chromeos/any.h>
+#include <chromeos/dbus/dbus_method_invoker.h>
+#include <chromeos/dbus/dbus_signal_handler.h>
+#include <chromeos/errors/error.h>
+#include <chromeos/variant_dictionary.h>
+#include <dbus/bus.h>
+#include <dbus/message.h>
+#include <dbus/object_path.h>
+#include <dbus/object_proxy.h>
+
+namespace org {
+namespace chromium {
+
+// Interface proxy for org::chromium::TestInterface.
+class TestInterfaceProxy final {
+ public:
+  class SignalReceiver {
+   public:
+    virtual void OnCloserSignal() {}
+  };
+
+  TestInterfaceProxy(const scoped_refptr<dbus::Bus>& bus) :
+      bus_(bus),
+      dbus_object_proxy_(
+          bus_->GetObjectProxy(service_name_, object_path_)) {
+  }
+
+  TestInterfaceProxy(
+      const scoped_refptr<dbus::Bus>& bus,
+      SignalReceiver* signal_receiver) :
+          TestInterfaceProxy(bus) {
+    chromeos::dbus_utils::ConnectToSignal(
+        dbus_object_proxy_,
+        "org.chromium.TestInterface",
+        "Closer",
+        base::Bind(
+            &SignalReceiver::OnCloserSignal,
+            base::Unretained(signal_receiver)),
+        base::Bind(
+            &TestInterfaceProxy::OnDBusSignalConnected,
+            base::Unretained(this)));
+  }
+
+  ~TestInterfaceProxy() {
+  }
+
+  void ReleaseObjectProxy(const base::Closure& callback) {
+    bus_->RemoveObjectProxy(service_name_, object_path_, callback);
+  }
+
+  void OnDBusSignalConnected(
+      const std::string& interface,
+      const std::string& signal,
+      bool success) {
+    if (!success) {
+      LOG(ERROR)
+          << "Failed to connect to " << interface << "." << signal
+          << " for " << service_name_ << " at "
+          << object_path_.value();
+    }
+  }
+
+ private:
+  scoped_refptr<dbus::Bus> bus_;
+  const std::string service_name_{"org.chromium.Test"};
+  const dbus::ObjectPath object_path_{"/org/chromium/Test"};
+  dbus::ObjectProxy* dbus_object_proxy_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestInterfaceProxy);
+};
+
+}  // namespace chromium
+}  // namespace org
+
+namespace org {
+namespace chromium {
+
+// Interface proxy for org::chromium::TestInterface2.
+class TestInterface2Proxy final {
+ public:
+  TestInterface2Proxy(
+      const scoped_refptr<dbus::Bus>& bus,
+      const std::string& object_path) :
+          bus_(bus),
+          object_path_(object_path),
+          dbus_object_proxy_(
+              bus_->GetObjectProxy(service_name_, object_path_)) {
+  }
+
+  ~TestInterface2Proxy() {
+  }
+
+  void ReleaseObjectProxy(const base::Closure& callback) {
+    bus_->RemoveObjectProxy(service_name_, object_path_, callback);
+  }
+
+ private:
+  scoped_refptr<dbus::Bus> bus_;
+  const std::string service_name_{"org.chromium.Test"};
+  dbus::ObjectPath object_path_;
+  dbus::ObjectProxy* dbus_object_proxy_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestInterface2Proxy);
+};
+
+}  // namespace chromium
+}  // namespace org
+
+)literal_string";
+
 }  // namespace
 
 class ProxyGeneratorTest : public Test {
@@ -275,6 +391,7 @@ class ProxyGeneratorTest : public Test {
 TEST_F(ProxyGeneratorTest, GenerateAdaptors) {
   Interface interface;
   interface.name = kInterfaceName;
+  interface.path = "/org/chromium/Test";
   interface.methods.emplace_back(
       kMethod1Name,
       vector<Interface::Argument>{
@@ -296,7 +413,7 @@ TEST_F(ProxyGeneratorTest, GenerateAdaptors) {
       vector<Interface::Argument>{
           {"", kSignal2Argument1},
           {"", kSignal2Argument2}});
-  interface.methods.back().doc_string_ = "Comment line1\nline2";
+  interface.methods.back().doc_string = "Comment line1\nline2";
   Interface interface2;
   interface2.name = kInterfaceName2;
   interface2.methods.emplace_back(
@@ -307,7 +424,8 @@ TEST_F(ProxyGeneratorTest, GenerateAdaptors) {
           {kMethod5ArgumentName2, kMethod5Argument2}});
   vector<Interface> interfaces{interface, interface2};
   base::FilePath output_path = temp_dir_.path().Append("output.h");
-  EXPECT_TRUE(ProxyGenerator::GenerateProxies(interfaces, output_path));
+  ServiceConfig config;
+  EXPECT_TRUE(ProxyGenerator::GenerateProxies(config, interfaces, output_path));
   string contents;
   EXPECT_TRUE(base::ReadFileToString(output_path, &contents));
   // The header guards contain the (temporary) filename, so we search for
@@ -317,4 +435,25 @@ TEST_F(ProxyGeneratorTest, GenerateAdaptors) {
       << kExpectedContent << "...within content...\n" << contents;
 }
 
+TEST_F(ProxyGeneratorTest, GenerateAdaptorsWithServiceName) {
+  Interface interface;
+  interface.name = kInterfaceName;
+  interface.path = "/org/chromium/Test";
+  interface.signals.emplace_back(kSignal1Name);
+  Interface interface2;
+  interface2.name = kInterfaceName2;
+  vector<Interface> interfaces{interface, interface2};
+  base::FilePath output_path = temp_dir_.path().Append("output2.h");
+  ServiceConfig config;
+  config.service_name = "org.chromium.Test";
+  EXPECT_TRUE(ProxyGenerator::GenerateProxies(config, interfaces, output_path));
+  string contents;
+  EXPECT_TRUE(base::ReadFileToString(output_path, &contents));
+  // The header guards contain the (temporary) filename, so we search for
+  // the content we need within the string.
+  EXPECT_NE(string::npos, contents.find(kExpectedContentWithService))
+      << "Expected to find the following content...\n"
+      << kExpectedContentWithService << "...within content...\n" << contents;
+}
+
 }  // namespace chromeos_dbus_bindings
diff --git a/chromeos-dbus-bindings/xml_interface_parser.cc b/chromeos-dbus-bindings/xml_interface_parser.cc
index dd30d43..2146ebd 100644
--- a/chromeos-dbus-bindings/xml_interface_parser.cc
+++ b/chromeos-dbus-bindings/xml_interface_parser.cc
@@ -9,7 +9,8 @@
 #include <base/file_util.h>
 #include <base/files/file_path.h>
 #include <base/logging.h>
-#include <base/stl_util.h>
+#include <base/strings/string_util.h>
+#include <chromeos/strings/string_utils.h>
 
 using std::string;
 using std::vector;
@@ -47,13 +48,15 @@ const char XmlInterfaceParser::kMethodKindNormal[] = "normal";
 const char XmlInterfaceParser::kMethodKindAsync[] = "async";
 const char XmlInterfaceParser::kMethodKindRaw[] = "raw";
 
-bool XmlInterfaceParser::ParseXmlInterfaceFile(
-    const base::FilePath& interface_file) {
-  string contents;
-  if (!base::ReadFileToString(interface_file, &contents)) {
-    LOG(ERROR) << "Failed to read file " << interface_file.value();
-    return false;
-  }
+namespace {
+
+string GetElementPath(const vector<string>& path) {
+  return chromeos::string_utils::Join('/', path);
+}
+
+}  // anonymous namespace
+
+bool XmlInterfaceParser::ParseXmlInterfaceFile(const std::string& contents) {
   auto parser = XML_ParserCreate(nullptr);
   XML_SetUserData(parser, this);
   XML_SetElementHandler(parser,
@@ -87,28 +90,41 @@ void XmlInterfaceParser::OnOpenElement(
   if (element_name == kNodeTag) {
     CHECK(prev_element.empty() || prev_element == kNodeTag)
         << "Unexpected tag " << element_name << " inside " << prev_element;
+    // '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)
         << "Unexpected tag " << element_name << " inside " << prev_element;
-    string interface_name = GetValidatedElementName(attributes, kInterfaceTag);
-    interfaces_.emplace_back(interface_name,
-                             std::vector<Interface::Method>{},
-                             std::vector<Interface::Signal>{},
-                             std::vector<Interface::Property>{});
+    string interface_name = GetValidatedElementName(attributes, element_path_);
+    Interface itf(interface_name,
+                  std::vector<Interface::Method>{},
+                  std::vector<Interface::Signal>{},
+                  std::vector<Interface::Property>{});
+    itf.path = node_names_.back();
+    interfaces_.push_back(std::move(itf));
   } else if (element_name == kMethodTag) {
     CHECK_EQ(kInterfaceTag, prev_element)
         << "Unexpected tag " << element_name << " inside " << prev_element;
     interfaces_.back().methods.push_back(
-        Interface::Method(GetValidatedElementName(attributes, kMethodTag)));
+        Interface::Method(GetValidatedElementName(attributes, element_path_)));
   } else if (element_name == kSignalTag) {
     CHECK_EQ(kInterfaceTag, prev_element)
         << "Unexpected tag " << element_name << " inside " << prev_element;
     interfaces_.back().signals.push_back(
-        Interface::Signal(GetValidatedElementName(attributes, kSignalTag)));
+        Interface::Signal(GetValidatedElementName(attributes, element_path_)));
   } else if (element_name == kPropertyTag) {
     CHECK_EQ(kInterfaceTag, prev_element)
         << "Unexpected tag " << element_name << " inside " << prev_element;
-    interfaces_.back().properties.push_back(ParseProperty(attributes));
+    interfaces_.back().properties.push_back(ParseProperty(attributes,
+                                                          element_path_));
   } else if (element_name == kArgumentTag) {
     if (prev_element == kMethodTag) {
       AddMethodArgument(attributes);
@@ -119,12 +135,11 @@ void XmlInterfaceParser::OnOpenElement(
                  << " inside " << prev_element;
     }
   } else if (element_name == kAnnotationTag) {
-    string element_path = prev_element + " " + element_name;
-    string name = GetValidatedElementAttribute(attributes, element_path,
+    string name = GetValidatedElementAttribute(attributes, element_path_,
                                                kNameAttribute);
     // Value is optional. Default to empty string if omitted.
     string value;
-    GetElementAttribute(attributes, element_path, kValueAttribute, &value);
+    GetElementAttribute(attributes, element_path_, kValueAttribute, &value);
     if (prev_element == kInterfaceTag) {
       // Parse interface annotations...
     } else if (prev_element == kMethodTag) {
@@ -172,13 +187,13 @@ void XmlInterfaceParser::OnCharData(const std::string& content) {
   string* doc_string_ptr = nullptr;
   string target_element = element_path_[element_path_.size() - 2];
   if (target_element == kInterfaceTag) {
-    doc_string_ptr = &(interfaces_.back().doc_string_);
+    doc_string_ptr = &(interfaces_.back().doc_string);
   } else if (target_element == kMethodTag) {
-    doc_string_ptr = &(interfaces_.back().methods.back().doc_string_);
+    doc_string_ptr = &(interfaces_.back().methods.back().doc_string);
   } else if (target_element == kSignalTag) {
-    doc_string_ptr = &(interfaces_.back().signals.back().doc_string_);
+    doc_string_ptr = &(interfaces_.back().signals.back().doc_string);
   } else if (target_element == kPropertyTag) {
-    doc_string_ptr = &(interfaces_.back().properties.back().doc_string_);
+    doc_string_ptr = &(interfaces_.back().properties.back().doc_string);
   }
 
   // If <tp:docstring> is attached to elements we don't care about, do nothing.
@@ -191,11 +206,10 @@ void XmlInterfaceParser::OnCharData(const std::string& content) {
 
 void XmlInterfaceParser::AddMethodArgument(const XmlAttributeMap& attributes) {
   string argument_direction;
+  vector<string> path = element_path_;
+  path.push_back(kArgumentTag);
   bool is_direction_paramter_present = GetElementAttribute(
-      attributes,
-      string(kMethodTag) + " " + kArgumentTag,
-      kDirectionAttribute,
-      &argument_direction);
+      attributes, path, kDirectionAttribute, &argument_direction);
   vector<Interface::Argument>* argument_list = nullptr;
   if (!is_direction_paramter_present ||
       argument_direction == kArgumentDirectionIn) {
@@ -205,12 +219,12 @@ void XmlInterfaceParser::AddMethodArgument(const XmlAttributeMap& attributes) {
   } else {
     LOG(FATAL) << "Unknown method argument direction " << argument_direction;
   }
-  argument_list->push_back(ParseArgument(attributes, kMethodTag));
+  argument_list->push_back(ParseArgument(attributes, element_path_));
 }
 
 void XmlInterfaceParser::AddSignalArgument(const XmlAttributeMap& attributes) {
   interfaces_.back().signals.back().arguments.push_back(
-      ParseArgument(attributes, kSignalTag));
+      ParseArgument(attributes, element_path_));
 }
 
 void XmlInterfaceParser::OnCloseElement(const string& element_name) {
@@ -218,19 +232,23 @@ void XmlInterfaceParser::OnCloseElement(const string& element_name) {
   CHECK(!element_path_.empty());
   CHECK_EQ(element_path_.back(), element_name);
   element_path_.pop_back();
+  if (element_name == kNodeTag) {
+    CHECK(!node_names_.empty());
+    node_names_.pop_back();
+  }
 }
 
 // static
 bool XmlInterfaceParser::GetElementAttribute(
     const XmlAttributeMap& attributes,
-    const string& element_type,
+    const vector<string>& element_path,
     const string& element_key,
     string* element_value) {
-  if (!ContainsKey(attributes, element_key)) {
+  if (attributes.find(element_key) == attributes.end()) {
     return false;
   }
   *element_value = attributes.find(element_key)->second;
-  VLOG(1) << "Got " << element_type << " element with "
+  VLOG(1) << "Got " << GetElementPath(element_path) << " element with "
           << element_key << " = " << *element_value;
   return true;
 }
@@ -238,52 +256,51 @@ bool XmlInterfaceParser::GetElementAttribute(
 // static
 string XmlInterfaceParser::GetValidatedElementAttribute(
     const XmlAttributeMap& attributes,
-    const string& element_type,
+    const vector<string>& element_path,
     const string& element_key) {
   string element_value;
   CHECK(GetElementAttribute(attributes,
-                            element_type,
+                            element_path,
                             element_key,
                             &element_value))
-      << element_type << " does not contain a " << element_key << " attribute";
-  CHECK(!element_value.empty()) << element_type << " " << element_key
-                              << " attribute is empty";
+      << GetElementPath(element_path) << " does not contain a " << element_key
+      << " attribute";
+  CHECK(!element_value.empty()) << GetElementPath(element_path) << " "
+                                << element_key << " attribute is empty";
   return element_value;
 }
 
 // static
 string XmlInterfaceParser::GetValidatedElementName(
     const XmlAttributeMap& attributes,
-    const string& element_type) {
-  return GetValidatedElementAttribute(attributes, element_type, kNameAttribute);
+    const vector<string>& element_path) {
+  return GetValidatedElementAttribute(attributes, element_path, kNameAttribute);
 }
 
 // static
 Interface::Argument XmlInterfaceParser::ParseArgument(
-    const XmlAttributeMap& attributes, const string& element_type) {
-  string element_and_argument = element_type + " " + kArgumentTag;
+    const XmlAttributeMap& attributes, const vector<string>& element_path) {
+  vector<string> path = element_path;
+  path.push_back(kArgumentTag);
   string argument_name;
   // Since the "name" field is optional, use the un-validated variant.
-  GetElementAttribute(attributes,
-                      element_and_argument,
-                      kNameAttribute,
-                      &argument_name);
+  GetElementAttribute(attributes, path, kNameAttribute, &argument_name);
 
   string argument_type = GetValidatedElementAttribute(
-      attributes, element_and_argument, kTypeAttribute);
+      attributes, path, kTypeAttribute);
   return Interface::Argument(argument_name, argument_type);
 }
 
 // static
 Interface::Property XmlInterfaceParser::ParseProperty(
-    const XmlAttributeMap& attributes) {
-  string property_name = GetValidatedElementName(attributes,
-                                                 kPropertyTag);
-  string property_type = GetValidatedElementAttribute(attributes,
-                                                      kPropertyTag,
+    const XmlAttributeMap& attributes,
+    const std::vector<std::string>& element_path) {
+  vector<string> path = element_path;
+  path.push_back(kPropertyTag);
+  string property_name = GetValidatedElementName(attributes, path);
+  string property_type = GetValidatedElementAttribute(attributes, path,
                                                       kTypeAttribute);
-  string property_access = GetValidatedElementAttribute(attributes,
-                                                        kPropertyTag,
+  string property_access = GetValidatedElementAttribute(attributes, path,
                                                         kAccessAttribute);
   return Interface::Property(property_name, property_type, property_access);
 }
diff --git a/chromeos-dbus-bindings/xml_interface_parser.h b/chromeos-dbus-bindings/xml_interface_parser.h
index 7883ac5..431d0dc 100644
--- a/chromeos-dbus-bindings/xml_interface_parser.h
+++ b/chromeos-dbus-bindings/xml_interface_parser.h
@@ -30,7 +30,7 @@ class XmlInterfaceParser {
   XmlInterfaceParser() = default;
   virtual ~XmlInterfaceParser() = default;
 
-  virtual bool ParseXmlInterfaceFile(const base::FilePath& interface_file);
+  virtual bool ParseXmlInterfaceFile(const std::string& contents);
   const std::vector<Interface>& interfaces() const { return interfaces_; }
 
  private:
@@ -84,7 +84,7 @@ class XmlInterfaceParser {
   // Finds the |element_key| element in |attributes|.  Returns true and sets
   // |element_value| on success.  Returns false otherwise.
   static bool GetElementAttribute(const XmlAttributeMap& attributes,
-                                  const std::string& element_type,
+                                  const std::vector<std::string>& element_path,
                                   const std::string& element_key,
                                   std::string* element_value);
 
@@ -92,20 +92,23 @@ class XmlInterfaceParser {
   // Returns the name on success, triggers a CHECK() otherwise.
   static std::string GetValidatedElementAttribute(
       const XmlAttributeMap& attributes,
-      const std::string& element_type,
+      const std::vector<std::string>& element_path,
       const std::string& element_key);
 
   // Calls GetValidatedElementAttribute() for the "name" property.
   static std::string GetValidatedElementName(
       const XmlAttributeMap& attributes,
-      const std::string& element_type);
+      const std::vector<std::string>& element_path);
 
   // Method for extracting signal/method tag attributes to a struct.
-  static Interface::Argument ParseArgument(const XmlAttributeMap& attributes,
-                                           const std::string& element_type);
+  static Interface::Argument ParseArgument(
+      const XmlAttributeMap& attributes,
+      const std::vector<std::string>& element_path);
 
   // Method for extracting property tag attributes to a struct.
-  static Interface::Property ParseProperty(const XmlAttributeMap& attributes);
+  static Interface::Property ParseProperty(
+      const XmlAttributeMap& attributes,
+      const std::vector<std::string>& element_path);
 
   // Expat element callback functions.
   static void HandleElementStart(void* user_data,
@@ -117,6 +120,9 @@ class XmlInterfaceParser {
   // The output of the parse.
   std::vector<Interface> interfaces_;
 
+  // A stack of <node> names used to track the object paths for interfaces.
+  std::vector<std::string> node_names_;
+
   // Tracks where in the element traversal our parse has taken us.
   std::vector<std::string> element_path_;
 
diff --git a/chromeos-dbus-bindings/xml_interface_parser_unittest.cc b/chromeos-dbus-bindings/xml_interface_parser_unittest.cc
index 7f861e3..dcb163a 100644
--- a/chromeos-dbus-bindings/xml_interface_parser_unittest.cc
+++ b/chromeos-dbus-bindings/xml_interface_parser_unittest.cc
@@ -22,7 +22,7 @@ namespace {
 const char kBadInterfaceFileContents0[] = "This has no resemblance to XML";
 const char kBadInterfaceFileContents1[] = "<node>";
 const char kGoodInterfaceFileContents[] = R"literal_string(
-<node>
+<node name="/org/chromium/Test">
   <interface name="fi.w1.wpa_supplicant1.Interface">
     <method name="Scan">
       <arg name="args" type="a{sv}" direction="in"/>
@@ -38,6 +38,8 @@ const char kGoodInterfaceFileContents[] = R"literal_string(
       <arg name="BSS" type="o"/>
     </signal>
   </interface>
+  <node name="/"/>
+  <node/>
 </node>
 )literal_string";
 const char kInterfaceName[] = "fi.w1.wpa_supplicant1.Interface";
@@ -57,35 +59,23 @@ const char kReadAccess[] = "read";
 }  // namespace
 
 class XmlInterfaceParserTest : public Test {
- public:
-  void SetUp() override {
-    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  }
-
  protected:
-  bool ParseXmlContents(const string& contents) {
-    base::FilePath path = temp_dir_.path().Append("interface.xml");
-    EXPECT_TRUE(base::WriteFile(path, contents.c_str(), contents.size()));
-    return parser_.ParseXmlInterfaceFile(path);
-  }
-
-  base::ScopedTempDir temp_dir_;
   XmlInterfaceParser parser_;
 };
 
 TEST_F(XmlInterfaceParserTest, BadInputFile) {
-  EXPECT_FALSE(parser_.ParseXmlInterfaceFile(base::FilePath()));
-  EXPECT_FALSE(ParseXmlContents(kBadInterfaceFileContents0));
-  EXPECT_FALSE(ParseXmlContents(kBadInterfaceFileContents1));
+  EXPECT_FALSE(parser_.ParseXmlInterfaceFile(kBadInterfaceFileContents0));
+  EXPECT_FALSE(parser_.ParseXmlInterfaceFile(kBadInterfaceFileContents1));
 }
 
 TEST_F(XmlInterfaceParserTest, GoodInputFile) {
-  EXPECT_TRUE(ParseXmlContents(kGoodInterfaceFileContents));
+  EXPECT_TRUE(parser_.ParseXmlInterfaceFile(kGoodInterfaceFileContents));
   const vector<Interface>& interfaces = parser_.interfaces();
   ASSERT_EQ(1, interfaces.size());
   const Interface& interface = interfaces.back();
 
   EXPECT_EQ(kInterfaceName, interface.name);
+  EXPECT_EQ("/org/chromium/Test", interface.path);
   ASSERT_EQ(2, interface.methods.size());
   ASSERT_EQ(1, interface.signals.size());