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>
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::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 @@
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 @@
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 @@
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 @@
}, 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 @@
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();
},
