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>
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 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 @@
" --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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
std::string name;
std::string type;
std::string access;
- std::string doc_string_;
+ std::string doc_string;
};
Interface() = default;
@@ -70,10 +70,11 @@
: 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 @@
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 @@
}
// 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 @@
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 @@
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 @@
}
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
)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 @@
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 @@
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 @@
{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 @@
<< 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::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 @@
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 @@
<< " 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 @@
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::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 @@
} 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 @@
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 @@
// 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 @@
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 @@
// 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 @@
// 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 @@
// 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 @@
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 @@
<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 @@
} // 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());