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>

2 files changed, 25 insertions, 34 deletions

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 @@ void ProxyGenerator::GenerateInterfaceProxyInterface(
   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 @@ void ProxyGenerator::GenerateInterfaceProxyInterface(
   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 @@ void ProxyGenerator::GenerateInterfaceProxy(const ServiceConfig& config,
   AddProperties(config, interface, false, text);
 
   text->PopOffset();
+  text->AddBlankLine();
   text->AddLineWithOffset("private:", kScopeOffset);
 
   text->PushOffset(kBlockOffset);
@@ -392,38 +391,38 @@ void ProxyGenerator::AddDestructor(const string& class_name,
   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 @@ void ProxyGenerator::AddPropertyPublicMethods(const string& class_name,
   text->AddLine("const PropertySet* GetProperties() const "
                 "{ return property_set_; }");
   text->AddLine("PropertySet* GetProperties() { return property_set_; }");
-  text->AddBlankLine();
 }
 
 // static
@@ -458,6 +456,7 @@ void ProxyGenerator::AddSignalHandlerRegistration(
       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 @@ void ProxyGenerator::AddSignalHandlerRegistration(
     block.PopOffset();  // Method body
     block.AddLine("}");
   }
-  block.AddBlankLine();
   text->AddBlock(block);
 }
 
@@ -547,6 +545,9 @@ void ProxyGenerator::AddProperties(const ServiceConfig& config,
   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 @@ void ProxyGenerator::AddProperties(const ServiceConfig& config,
     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 @@ void ProxyGenerator::AddProperties(const ServiceConfig& config,
           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 @@ void ProxyGenerator::AddMethodProxy(const Interface::Method& method,
                                     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 @@ void ProxyGenerator::AddMethodProxy(const Interface::Method& method,
     block.PopOffset();
     block.AddLine("}");
   }
-  block.AddBlankLine();
   text->AddBlock(block);
 }
 
@@ -652,6 +651,7 @@ void ProxyGenerator::AddAsyncMethodProxy(const Interface::Method& method,
                                          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 @@ void ProxyGenerator::AddAsyncMethodProxy(const Interface::Method& method,
     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 @@ namespace chromium {
 // 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 @@ class TestInterfaceProxyInterface {
       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 @@ namespace chromium {
 // 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 @@ class TestInterface2ProxyInterface {
       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 @@ namespace chromium {
 // 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 @@ namespace chromium {
 // Abstract interface proxy for org::chromium::TestInterface2.
 class TestInterface2ProxyInterface {
  public:
- protected:
   virtual ~TestInterface2ProxyInterface() = default;
 };
 
@@ -578,15 +574,14 @@ namespace chromium {
 // 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 @@ namespace chromium {
 // Abstract interface proxy for org::chromium::Itf2.
 class Itf2ProxyInterface {
  public:
- protected:
   virtual ~Itf2ProxyInterface() = default;
 };
 
@@ -962,12 +956,11 @@ namespace chromium {
 // 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 @@ namespace chromium {
 // Abstract interface proxy for org::chromium::Itf2.
 class Itf2ProxyInterface {
  public:
- protected:
   virtual ~Itf2ProxyInterface() = default;
 };