chromeos-dbus-bindings: Handle mocks all the way up to eleven.

(cherry-pick of platform2 240e46d3b77a969611ec6855cbb2e95a4293a3b6)

gmock doesn't define MOCK_METHODn for n greater than 10 due to some
legacy compiler limitations in older Visual Studio versions. While
hanving methods with more than 10 arguments is rare and a bad idea in
general, we have some DBus methods that will generate wrappers with
more than 10 arguments, between DBus method arguments and the added
callbacks, and timeouts.

This patch avoids the problem by not generating an invalid MOCK_METHOD
entry for such methods and leaving the chance for the user of such
mock to subclass it and implement such method in a different way or
ignore the problem if the method in the mock was not needed at all. To
do this we provide a default implementation with a warning.

BUG=chromium:419827
TEST=Added unittests. Generated flimflam.ManagerProxy mocks that now compile.

Change-Id: I742217b1cdd8bfb4ee5a37bec51485e21cb654d9
Reviewed-on: https://chromium-review.googlesource.com/289835
Commit-Queue: Alex Deymo <deymo@chromium.org>
Trybot-Ready: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>

3 files changed, 106 insertions, 27 deletions

diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index 2e96975..b40aabd 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -7,6 +7,7 @@
 #include <utility>
 
 #include <base/files/file_path.h>
+#include <base/format_macros.h>
 #include <base/logging.h>
 #include <base/strings/stringprintf.h>
 #include <chromeos/strings/string_utils.h>
@@ -115,6 +116,7 @@ bool ProxyGenerator::GenerateMocks(const ServiceConfig& config,
   text.AddLine("#include <vector>");
   text.AddBlankLine();
   text.AddLine("#include <base/callback_forward.h>");
+  text.AddLine("#include <base/logging.h>");
   text.AddLine("#include <base/macros.h>");
   text.AddLine("#include <chromeos/any.h>");
   text.AddLine("#include <chromeos/errors/error.h>");
@@ -306,7 +308,7 @@ void ProxyGenerator::GenerateInterfaceMock(const ServiceConfig& config,
 
   text->AddLine(StringPrintf("// Mock object for %s.",
                              base_interface_name.c_str()));
-  text->AddLine(StringPrintf("class %s final : public %s {",
+  text->AddLine(StringPrintf("class %s : public %s {",
                              mock_name.c_str(), base_interface_name.c_str()));
   text->AddLineWithOffset("public:", kScopeOffset);
   text->PushOffset(kBlockOffset);
@@ -714,7 +716,6 @@ void ProxyGenerator::AddAsyncMethodProxy(const Interface::Method& method,
 void ProxyGenerator::AddMethodMock(const Interface::Method& method,
                                    const string& interface_name,
                                    IndentedText* text) {
-  IndentedText block;
   DbusSignature signature;
   vector<string> arguments;
   for (const auto& argument : method.input_arguments) {
@@ -735,25 +736,13 @@ void ProxyGenerator::AddMethodMock(const Interface::Method& method,
   }
   arguments.push_back("chromeos::ErrorPtr* /*error*/");
   arguments.push_back("int /*timeout_ms*/");
-
-  block.AddLineAndPushOffsetTo(
-      StringPrintf("MOCK_METHOD%ju(%s,", arguments.size(), method.name.c_str()),
-      1, '(');
-  block.AddLineAndPushOffsetTo(
-      StringPrintf("bool(%s,", arguments.front().c_str()), 1, '(');
-  for (size_t i = 1; i < arguments.size() - 1; i++)
-    block.AddLine(StringPrintf("%s,", arguments[i].c_str()));
-  block.AddLine(StringPrintf("%s));", arguments.back().c_str()));
-  block.PopOffset();
-  block.PopOffset();
-  text->AddBlock(block);
+  AddMockMethodDeclaration(method.name, "bool", arguments, text);
 }
 
 // static
 void ProxyGenerator::AddAsyncMethodMock(const Interface::Method& method,
                                         const string& interface_name,
                                         IndentedText* text) {
-  IndentedText block;
   DbusSignature signature;
   vector<string> arguments;
   for (const auto& argument : method.input_arguments) {
@@ -779,17 +768,56 @@ void ProxyGenerator::AddAsyncMethodMock(const Interface::Method& method,
   arguments.push_back(
       "const base::Callback<void(chromeos::Error*)>& /*error_callback*/");
   arguments.push_back("int /*timeout_ms*/");
+  AddMockMethodDeclaration(method.name + "Async", "void", arguments, text);
+}
 
-  block.AddLineAndPushOffsetTo(
-      StringPrintf("MOCK_METHOD%ju(%sAsync,", arguments.size(),
-                   method.name.c_str()), 1, '(');
-  block.AddLineAndPushOffsetTo(
-      StringPrintf("void(%s,", arguments.front().c_str()), 1, '(');
-  for (size_t i = 1; i < arguments.size() - 1; i++)
-    block.AddLine(StringPrintf("%s,", arguments[i].c_str()));
-  block.AddLine(StringPrintf("%s));", arguments.back().c_str()));
-  block.PopOffset();
-  block.PopOffset();
+void ProxyGenerator::AddMockMethodDeclaration(const string& method_name,
+                                              const string& return_type,
+                                              const vector<string>& arguments,
+                                              IndentedText* text) {
+  IndentedText block;
+  // GMOCK doesn't go all the way up to 11, so we need to handle methods with
+  // 11 arguments or more in a different way.
+  if (arguments.size() >= 11) {
+    block.AddLineAndPushOffsetTo(
+        StringPrintf("%s %s(%s,",
+                     return_type.c_str(),
+                     method_name.c_str(),
+                     arguments.front().c_str()),
+        1, '(');
+    for (size_t i = 1; i < arguments.size() - 1; i++)
+      block.AddLine(StringPrintf("%s,", arguments[i].c_str()));
+    block.AddLine(StringPrintf("%s) override {", arguments.back().c_str()));
+    block.PopOffset();
+    block.PushOffset(kBlockOffset);
+    block.AddLine(StringPrintf(
+        "LOG(WARNING) << \"%s(): gmock can't handle methods with %" PRIuS
+        " arguments. You can override this method in a subclass if you need"
+        " to.\";",
+        method_name.c_str(), arguments.size()));
+    if (return_type == "void") {
+      // No return added here.
+    } else if (return_type == "bool") {
+      block.AddLine("return false;");
+    } else {
+      LOG(FATAL) << "The return type is not supported.";
+    }
+    block.PopOffset();
+    block.AddLine("}");
+  } else {
+    block.AddLineAndPushOffsetTo(
+        StringPrintf("MOCK_METHOD%ju(%s,",
+                     arguments.size(), method_name.c_str()),
+        1, '(');
+    block.AddLineAndPushOffsetTo(
+        StringPrintf("%s(%s,", return_type.c_str(), arguments.front().c_str()),
+        1, '(');
+    for (size_t i = 1; i < arguments.size() - 1; i++)
+      block.AddLine(StringPrintf("%s,", arguments[i].c_str()));
+    block.AddLine(StringPrintf("%s));", arguments.back().c_str()));
+    block.PopOffset();
+    block.PopOffset();
+  }
   text->AddBlock(block);
 }
 
diff --git a/chromeos-dbus-bindings/proxy_generator.h b/chromeos-dbus-bindings/proxy_generator.h
index ba9a224..56fce06 100644
--- a/chromeos-dbus-bindings/proxy_generator.h
+++ b/chromeos-dbus-bindings/proxy_generator.h
@@ -118,6 +118,14 @@ class ProxyGenerator : public HeaderGenerator {
                                  const std::string& interface_name,
                                  IndentedText* text);
 
+  // Generates the MOCK_METHOD entry for the given arguments handling methods
+  // with more than 10 arguments.
+  static void AddMockMethodDeclaration(
+      const std::string& method_name,
+      const std::string& return_type,
+      const std::vector<std::string>& arguments,
+      IndentedText* text);
+
   // Generates a mock for the signal handler registration method.
   static void AddSignalHandlerRegistrationMock(
       const Interface::Signal& signal,
diff --git a/chromeos-dbus-bindings/proxy_generator_mock_unittest.cc b/chromeos-dbus-bindings/proxy_generator_mock_unittest.cc
index a07a8c9..6ff4a24 100644
--- a/chromeos-dbus-bindings/proxy_generator_mock_unittest.cc
+++ b/chromeos-dbus-bindings/proxy_generator_mock_unittest.cc
@@ -36,6 +36,7 @@ const char kExpectedContent[] = R"literal_string(
 #include <vector>
 
 #include <base/callback_forward.h>
+#include <base/logging.h>
 #include <base/macros.h>
 #include <chromeos/any.h>
 #include <chromeos/errors/error.h>
@@ -48,7 +49,7 @@ namespace org {
 namespace chromium {
 
 // Mock object for TestInterfaceProxyInterface.
-class TestInterfaceProxyMock final : public TestInterfaceProxyInterface {
+class TestInterfaceProxyMock : public TestInterfaceProxyInterface {
  public:
   TestInterfaceProxyMock() = default;
 
@@ -88,6 +89,33 @@ class TestInterfaceProxyMock final : public TestInterfaceProxyInterface {
                void(const base::Callback<void()>& /*success_callback*/,
                     const base::Callback<void(chromeos::Error*)>& /*error_callback*/,
                     int /*timeout_ms*/));
+  bool AllTheWayUpToEleven(bool /*in_arg1*/,
+                           bool /*in_arg2*/,
+                           bool /*in_arg3*/,
+                           bool /*in_arg4*/,
+                           bool /*in_arg5*/,
+                           bool /*in_arg6*/,
+                           bool /*in_arg7*/,
+                           bool /*in_arg8*/,
+                           bool* /*out_arg9*/,
+                           chromeos::ErrorPtr* /*error*/,
+                           int /*timeout_ms*/) override {
+    LOG(WARNING) << "AllTheWayUpToEleven(): gmock can't handle methods with 11 arguments. You can override this method in a subclass if you need to.";
+    return false;
+  }
+  void AllTheWayUpToElevenAsync(bool /*in_arg1*/,
+                                bool /*in_arg2*/,
+                                bool /*in_arg3*/,
+                                bool /*in_arg4*/,
+                                bool /*in_arg5*/,
+                                bool /*in_arg6*/,
+                                bool /*in_arg7*/,
+                                bool /*in_arg8*/,
+                                const base::Callback<void(bool /*arg9*/)>& /*success_callback*/,
+                                const base::Callback<void(chromeos::Error*)>& /*error_callback*/,
+                                int /*timeout_ms*/) override {
+    LOG(WARNING) << "AllTheWayUpToElevenAsync(): gmock can't handle methods with 11 arguments. You can override this method in a subclass if you need to.";
+  }
   MOCK_METHOD2(RegisterCloserSignalHandler,
                void(const base::Closure& /*signal_callback*/,
                     dbus::ObjectProxy::OnConnectedCallback /*on_connected_callback*/));
@@ -106,7 +134,7 @@ namespace org {
 namespace chromium {
 
 // Mock object for TestInterface2ProxyInterface.
-class TestInterface2ProxyMock final : public TestInterface2ProxyInterface {
+class TestInterface2ProxyMock : public TestInterface2ProxyInterface {
  public:
   TestInterface2ProxyMock() = default;
 
@@ -166,6 +194,21 @@ TEST_F(ProxyGeneratorMockTest, GenerateMocks) {
       vector<Interface::Argument>{{"", kDBusTypeBool}},
       vector<Interface::Argument>{});
   interface.methods.emplace_back("ExperimentNumberSix");
+  // gmock can't handle more than 10 args. The generated method will also
+  // include the timeout and error arguments in the synchronous case, and two
+  // callbacks and the timeout in the asynchronous case.
+  interface.methods.emplace_back(
+      "AllTheWayUpToEleven",
+      vector<Interface::Argument>{
+          {"arg1", kDBusTypeBool},
+          {"arg2", kDBusTypeBool},
+          {"arg3", kDBusTypeBool},
+          {"arg4", kDBusTypeBool},
+          {"arg5", kDBusTypeBool},
+          {"arg6", kDBusTypeBool},
+          {"arg7", kDBusTypeBool},
+          {"arg8", kDBusTypeBool}},
+      vector<Interface::Argument>{{"arg9", kDBusTypeBool}});
   interface.signals.emplace_back("Closer");
   interface.signals.emplace_back(
       "TheCurseOfKaZar",