diff options
author | Eric Caruso <ejcaruso@chromium.org> | 2018-05-17 13:15:14 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2018-05-18 17:20:00 -0700 |
commit | c4075810a258bbc60c7bcf0617c1dcb995edc8a8 (patch) | |
tree | 4abd34e63480b78cfeea7d7c0aa5056cd80164d9 | |
parent | 514938dc6cc0ebcbd397764b2a59c9eec796cb97 (diff) | |
download | platform_external_libbrillo-c4075810a258bbc60c7bcf0617c1dcb995edc8a8.tar.gz platform_external_libbrillo-c4075810a258bbc60c7bcf0617c1dcb995edc8a8.tar.bz2 platform_external_libbrillo-c4075810a258bbc60c7bcf0617c1dcb995edc8a8.zip |
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>
-rw-r--r-- | brillo/dbus/dbus_method_invoker_unittest.cc | 94 | ||||
-rw-r--r-- | brillo/http/http_utils_unittest.cc | 3 | ||||
-rw-r--r-- | brillo/message_loops/message_loop_unittest.cc | 2 |
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(); }, |