chromeos-dbus-bindings: Move interface virtual destructors to public.
In order to pass around ownership of *Proxy objects using
*ProxyInterface classes we need to be able to destroy them from the
abstract interface. This patch moves the destructor to the public side.
To fix blank lines, this patch moves the blank lines added on all
methods to a line *before* the block instead of a line after the
block.
BUG=chromium:419827
TEST=Updated unittest. Generated code still compiles.
Change-Id: If7c7c2e0dfcb7887f523558726fc65a62221d89e
Reviewed-on: https://chromium-review.googlesource.com/289318
Trybot-Ready: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index 8202ab3..2e96975 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -191,6 +191,8 @@
text->AddLine(StringPrintf("class %s {", base_interface_name.c_str()));
text->AddLineWithOffset("public:", kScopeOffset);
text->PushOffset(kBlockOffset);
+ text->AddLine(
+ StringPrintf("virtual ~%s() = default;", base_interface_name.c_str()));
for (const auto& method : interface.methods) {
AddMethodProxy(method, interface.name, true, text);
@@ -202,10 +204,6 @@
AddProperties(config, interface, true, text);
text->PopOffset();
- text->AddLineWithOffset("protected:", kScopeOffset);
- text->AddLineWithOffset(
- StringPrintf("virtual ~%s() = default;", base_interface_name.c_str()),
- kBlockOffset);
text->AddLine("};");
text->AddBlankLine();
@@ -249,6 +247,7 @@
AddProperties(config, interface, false, text);
text->PopOffset();
+ text->AddBlankLine();
text->AddLineWithOffset("private:", kScopeOffset);
text->PushOffset(kBlockOffset);
@@ -392,38 +391,38 @@
IndentedText block;
block.AddLine(StringPrintf("~%s() override {", class_name.c_str()));
block.AddLine("}");
- block.AddBlankLine();
text->AddBlock(block);
}
// static
void ProxyGenerator::AddReleaseObjectProxy(IndentedText* text) {
+ text->AddBlankLine();
text->AddLine("void ReleaseObjectProxy(const base::Closure& callback) {");
text->AddLineWithOffset(
"bus_->RemoveObjectProxy(service_name_, object_path_, callback);",
kBlockOffset);
text->AddLine("}");
- text->AddBlankLine();
}
// static
void ProxyGenerator::AddGetObjectPath(IndentedText* text) {
+ text->AddBlankLine();
text->AddLine("const dbus::ObjectPath& GetObjectPath() const {");
text->AddLineWithOffset("return object_path_;", kBlockOffset);
text->AddLine("}");
- text->AddBlankLine();
}
// static
void ProxyGenerator::AddGetObjectProxy(IndentedText* text) {
+ text->AddBlankLine();
text->AddLine("dbus::ObjectProxy* GetObjectProxy() const { "
"return dbus_object_proxy_; }");
- text->AddBlankLine();
}
// static
void ProxyGenerator::AddPropertyPublicMethods(const string& class_name,
IndentedText* text) {
+ text->AddBlankLine();
text->AddLine("void SetPropertyChangedCallback(");
text->AddLineWithOffset(
StringPrintf("const base::Callback<void(%s*, "
@@ -436,7 +435,6 @@
text->AddLine("const PropertySet* GetProperties() const "
"{ return property_set_; }");
text->AddLine("PropertySet* GetProperties() { return property_set_; }");
- text->AddBlankLine();
}
// static
@@ -458,6 +456,7 @@
bool declaration_only,
IndentedText* text) {
IndentedText block;
+ block.AddBlankLine();
block.AddLine(StringPrintf("%svoid Register%sSignalHandler(",
declaration_only ? "virtual " : "",
signal.name.c_str()));
@@ -480,7 +479,6 @@
block.PopOffset(); // Method body
block.AddLine("}");
}
- block.AddBlankLine();
text->AddBlock(block);
}
@@ -547,6 +545,9 @@
if (config.object_manager.name.empty())
return;
+ if (declaration_only && !interface.properties.empty())
+ text->AddBlankLine();
+
DbusSignature signature;
for (const auto& prop : interface.properties) {
if (declaration_only) {
@@ -559,6 +560,8 @@
CHECK(signature.Parse(prop.type, &type));
MakeConstReferenceIfNeeded(&type);
string name = NameParser{prop.name}.MakeVariableName();
+ if (!declaration_only)
+ text->AddBlankLine();
text->AddLine(
StringPrintf("%s%s %s() const%s",
declaration_only ? "virtual " : "",
@@ -570,12 +573,8 @@
StringPrintf("return property_set_->%s.value();", name.c_str()),
kBlockOffset);
text->AddLine("}");
- text->AddBlankLine();
}
}
-
- if (declaration_only && !interface.properties.empty())
- text->AddBlankLine();
}
// static
@@ -585,6 +584,7 @@
IndentedText* text) {
IndentedText block;
DbusSignature signature;
+ block.AddBlankLine();
block.AddComments(method.doc_string);
block.AddLine(StringPrintf("%sbool %s(",
declaration_only ? "virtual " : "",
@@ -641,7 +641,6 @@
block.PopOffset();
block.AddLine("}");
}
- block.AddBlankLine();
text->AddBlock(block);
}
@@ -652,6 +651,7 @@
IndentedText* text) {
IndentedText block;
DbusSignature signature;
+ block.AddBlankLine();
block.AddComments(method.doc_string);
block.AddLine(StringPrintf("%svoid %sAsync(",
declaration_only ? "virtual " : "",
@@ -707,7 +707,6 @@
block.PopOffset();
block.AddLine("}");
}
- block.AddBlankLine();
text->AddBlock(block);
}
diff --git a/chromeos-dbus-bindings/proxy_generator_unittest.cc b/chromeos-dbus-bindings/proxy_generator_unittest.cc
index 7ce1c3e..75b7e9f 100644
--- a/chromeos-dbus-bindings/proxy_generator_unittest.cc
+++ b/chromeos-dbus-bindings/proxy_generator_unittest.cc
@@ -59,6 +59,8 @@
// Abstract interface proxy for org::chromium::TestInterface.
class TestInterfaceProxyInterface {
public:
+ virtual ~TestInterfaceProxyInterface() = default;
+
virtual bool Elements(
const std::string& in_space_walk,
const std::vector<dbus::ObjectPath>& in_ramblin_man,
@@ -115,9 +117,6 @@
const base::Callback<void(const std::vector<std::string>&,
uint8_t)>& signal_callback,
dbus::ObjectProxy::OnConnectedCallback on_connected_callback) = 0;
-
- protected:
- virtual ~TestInterfaceProxyInterface() = default;
};
} // namespace chromium
@@ -314,6 +313,8 @@
// Abstract interface proxy for org::chromium::TestInterface2.
class TestInterface2ProxyInterface {
public:
+ virtual ~TestInterface2ProxyInterface() = default;
+
virtual bool GetPersonInfo(
std::string* out_name,
int32_t* out_age,
@@ -324,9 +325,6 @@
const base::Callback<void(const std::string& /*name*/, int32_t /*age*/)>& success_callback,
const base::Callback<void(chromeos::Error*)>& error_callback,
int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT) = 0;
-
- protected:
- virtual ~TestInterface2ProxyInterface() = default;
};
} // namespace chromium
@@ -431,12 +429,11 @@
// Abstract interface proxy for org::chromium::TestInterface.
class TestInterfaceProxyInterface {
public:
+ virtual ~TestInterfaceProxyInterface() = default;
+
virtual void RegisterCloserSignalHandler(
const base::Closure& signal_callback,
dbus::ObjectProxy::OnConnectedCallback on_connected_callback) = 0;
-
- protected:
- virtual ~TestInterfaceProxyInterface() = default;
};
} // namespace chromium
@@ -496,7 +493,6 @@
// Abstract interface proxy for org::chromium::TestInterface2.
class TestInterface2ProxyInterface {
public:
- protected:
virtual ~TestInterface2ProxyInterface() = default;
};
@@ -578,15 +574,14 @@
// Abstract interface proxy for org::chromium::Itf1.
class Itf1ProxyInterface {
public:
+ virtual ~Itf1ProxyInterface() = default;
+
virtual void RegisterCloserSignalHandler(
const base::Closure& signal_callback,
dbus::ObjectProxy::OnConnectedCallback on_connected_callback) = 0;
static const char* DataName() { return "Data"; }
virtual const std::string& data() const = 0;
-
- protected:
- virtual ~Itf1ProxyInterface() = default;
};
} // namespace chromium
@@ -687,7 +682,6 @@
// Abstract interface proxy for org::chromium::Itf2.
class Itf2ProxyInterface {
public:
- protected:
virtual ~Itf2ProxyInterface() = default;
};
@@ -962,12 +956,11 @@
// Abstract interface proxy for org::chromium::Itf1.
class Itf1ProxyInterface {
public:
+ virtual ~Itf1ProxyInterface() = default;
+
virtual void RegisterCloserSignalHandler(
const base::Closure& signal_callback,
dbus::ObjectProxy::OnConnectedCallback on_connected_callback) = 0;
-
- protected:
- virtual ~Itf1ProxyInterface() = default;
};
} // namespace chromium
@@ -1041,7 +1034,6 @@
// Abstract interface proxy for org::chromium::Itf2.
class Itf2ProxyInterface {
public:
- protected:
virtual ~Itf2ProxyInterface() = default;
};