libbrillo: avoid double-binding callbacks and operator()

The second bind is a no-op since it doesn't provide any extra
arguments, so it's just an unnecessary allocation. Also, it looks
like new versions of base::Bind don't like functor objects, so
instead of packaging the callback's environment as part of an
enclosing object we should just bind in the bits we need as
arguments.

BUG=b:37434548
TEST=unit tests

Change-Id: I4bed9857655958f300bc99fa9508f2e940fc4a9a
Reviewed-on: https://chromium-review.googlesource.com/1064880
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

3 files changed, 45 insertions, 54 deletions

diff --git a/brillo/dbus/dbus_method_invoker_unittest.cc b/brillo/dbus/dbus_method_invoker_unittest.cc
index 08d8a22..4dc1f49 100644
--- a/brillo/dbus/dbus_method_invoker_unittest.cc
+++ b/brillo/dbus/dbus_method_invoker_unittest.cc
@@ -25,6 +25,37 @@ using dbus::MessageReader;
 using dbus::MessageWriter;
 using dbus::Response;
 
+namespace {
+
+void SuccessCallback(const std::string& expected_result,
+                     int* counter,
+                     const std::string& actual_result) {
+  (*counter)++;
+  EXPECT_EQ(expected_result, actual_result);
+}
+
+void SimpleSuccessCallback(int* counter, const std::string& result) {
+  (*counter)++;
+}
+
+void ErrorCallback(const std::string& domain,
+                   const std::string& code,
+                   const std::string& message,
+                   int* counter,
+                   brillo::Error* error) {
+  (*counter)++;
+  ASSERT_NE(nullptr, error);
+  EXPECT_EQ(domain, error->GetDomain());
+  EXPECT_EQ(code, error->GetCode());
+  EXPECT_EQ(message, error->GetMessage());
+}
+
+void SimpleErrorCallback(int* counter, brillo::Error* error) {
+  (*counter)++;
+}
+
+}  // namespace
+
 namespace brillo {
 namespace dbus_utils {
 
@@ -247,46 +278,6 @@ class AsyncDBusMethodInvokerTest : public testing::Test {
     LOG(FATAL) << "Unexpected method call: " << method_call->ToString();
   }
 
-  struct SuccessCallback {
-    SuccessCallback(const std::string& in_result, int* in_counter)
-        : result(in_result), counter(in_counter) {}
-
-    explicit SuccessCallback(int* in_counter) : counter(in_counter) {}
-
-    void operator()(const std::string& actual_result) {
-      (*counter)++;
-      EXPECT_EQ(result, actual_result);
-    }
-    std::string result;
-    int* counter;
-  };
-
-  struct ErrorCallback {
-    ErrorCallback(const std::string& in_domain,
-                  const std::string& in_code,
-                  const std::string& in_message,
-                  int* in_counter)
-        : domain(in_domain),
-          code(in_code),
-          message(in_message),
-          counter(in_counter) {}
-
-    explicit ErrorCallback(int* in_counter) : counter(in_counter) {}
-
-    void operator()(brillo::Error* error) {
-      (*counter)++;
-      EXPECT_NE(nullptr, error);
-      EXPECT_EQ(domain, error->GetDomain());
-      EXPECT_EQ(code, error->GetCode());
-      EXPECT_EQ(message, error->GetMessage());
-    }
-
-    std::string domain;
-    std::string code;
-    std::string message;
-    int* counter;
-  };
-
   scoped_refptr<dbus::MockBus> bus_;
   scoped_refptr<dbus::MockObjectProxy> mock_object_proxy_;
 };
@@ -298,22 +289,22 @@ TEST_F(AsyncDBusMethodInvokerTest, TestSuccess) {
       mock_object_proxy_.get(),
       kTestInterface,
       kTestMethod1,
-      base::Bind(SuccessCallback{"4", &success_count}),
-      base::Bind(ErrorCallback{&error_count}),
+      base::Bind(SuccessCallback, "4", &success_count),
+      base::Bind(SimpleErrorCallback, &error_count),
       2, 2);
   brillo::dbus_utils::CallMethod(
       mock_object_proxy_.get(),
       kTestInterface,
       kTestMethod1,
-      base::Bind(SuccessCallback{"10", &success_count}),
-      base::Bind(ErrorCallback{&error_count}),
+      base::Bind(SuccessCallback, "10", &success_count),
+      base::Bind(SimpleErrorCallback, &error_count),
       3, 7);
   brillo::dbus_utils::CallMethod(
       mock_object_proxy_.get(),
       kTestInterface,
       kTestMethod1,
-      base::Bind(SuccessCallback{"-4", &success_count}),
-      base::Bind(ErrorCallback{&error_count}),
+      base::Bind(SuccessCallback, "-4", &success_count),
+      base::Bind(SimpleErrorCallback, &error_count),
       13, -17);
   EXPECT_EQ(0, error_count);
   EXPECT_EQ(3, success_count);
@@ -326,11 +317,12 @@ TEST_F(AsyncDBusMethodInvokerTest, TestFailure) {
       mock_object_proxy_.get(),
       kTestInterface,
       kTestMethod2,
-      base::Bind(SuccessCallback{&success_count}),
-      base::Bind(ErrorCallback{brillo::errors::dbus::kDomain,
-                               "org.MyError",
-                               "My error message",
-                               &error_count}),
+      base::Bind(SimpleSuccessCallback, &success_count),
+      base::Bind(ErrorCallback,
+                 brillo::errors::dbus::kDomain,
+                 "org.MyError",
+                 "My error message",
+                 &error_count),
       2, 2);
   EXPECT_EQ(1, error_count);
   EXPECT_EQ(0, success_count);
diff --git a/brillo/http/http_utils_unittest.cc b/brillo/http/http_utils_unittest.cc
index 1497c3d..d619bb3 100644
--- a/brillo/http/http_utils_unittest.cc
+++ b/brillo/http/http_utils_unittest.cc
@@ -315,8 +315,7 @@ TEST(HttpUtils, PostText) {
   }, fake_data);
 
   std::shared_ptr<fake::Transport> transport(new fake::Transport);
-  transport->AddHandler(
-      kFakeUrl, request_type::kPost, base::Bind(post_handler));
+  transport->AddHandler(kFakeUrl, request_type::kPost, post_handler);
 
   auto response = http::PostTextAndBlock(kFakeUrl,
                                          fake_data,
diff --git a/brillo/message_loops/message_loop_unittest.cc b/brillo/message_loops/message_loop_unittest.cc
index 7c8d1b6..8024256 100644
--- a/brillo/message_loops/message_loop_unittest.cc
+++ b/brillo/message_loops/message_loop_unittest.cc
@@ -318,7 +318,7 @@ TYPED_TEST(MessageLoopTest, AllTasksAreEqual) {
          base::Closure* timeout_callback, MessageLoop::TaskId* timeout_task) {
       (*timeout_called)++;
       (*total_calls)++;
-      *timeout_task = loop->PostTask(FROM_HERE, Bind(*timeout_callback));
+      *timeout_task = loop->PostTask(FROM_HERE, *timeout_callback);
       if (*total_calls > 100)
         loop->BreakLoop();
       },