Remove bta_closure in favor of posting messages to a message loop
Test: Sanity test with Bluetooth
Change-Id: Ifda27a3bd88d82f884c4d8af6383f1c692b69e85
diff --git a/system/bta/Android.bp b/system/bta/Android.bp
index a4df7957..ffe01ad 100644
--- a/system/bta/Android.bp
+++ b/system/bta/Android.bp
@@ -7,7 +7,6 @@
"dm",
"hd",
"hh",
- "closure",
],
include_dirs: [
"packages/modules/Bluetooth/system",
@@ -51,7 +50,6 @@
"av/bta_av_ci.cc",
"av/bta_av_main.cc",
"av/bta_av_ssm.cc",
- "closure/bta_closure.cc",
"dm/bta_dm_act.cc",
"dm/bta_dm_api.cc",
"dm/bta_dm_cfg.cc",
@@ -118,7 +116,6 @@
name: "net_test_bta",
defaults: ["fluoride_bta_defaults"],
srcs: [
- "test/bta_closure_test.cc",
"test/bta_hf_client_test.cc",
],
shared_libs: [
diff --git a/system/bta/BUILD.gn b/system/bta/BUILD.gn
index 56a7597..07779d0 100644
--- a/system/bta/BUILD.gn
+++ b/system/bta/BUILD.gn
@@ -34,7 +34,6 @@
"av/bta_av_ci.cc",
"av/bta_av_main.cc",
"av/bta_av_ssm.cc",
- "closure/bta_closure.cc",
"dm/bta_dm_act.cc",
"dm/bta_dm_api.cc",
"dm/bta_dm_cfg.cc",
diff --git a/system/bta/closure/bta_closure.cc b/system/bta/closure/bta_closure.cc
deleted file mode 100644
index 599ee33..0000000
--- a/system/bta/closure/bta_closure.cc
+++ /dev/null
@@ -1,87 +0,0 @@
-/******************************************************************************
- *
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at:
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- ******************************************************************************/
-
-#include "base/pending_task.h"
-#include "base/time/time.h"
-#include "bta_closure_int.h"
-#include "bta_sys.h"
-
-using base::PendingTask;
-using base::TaskQueue;
-using base::TimeTicks;
-
-namespace {
-
-enum {
- /* these events are handled by the state machine */
- BTA_CLOSURE_EXECUTE_EVT = BTA_SYS_EVT_START(BTA_ID_CLOSURE),
-};
-
-struct tBTA_CLOSURE_EXECUTE {
- BT_HDR hdr;
- PendingTask pending_task;
-};
-
-static const tBTA_SYS_REG bta_closure_hw_reg = {bta_closure_execute, NULL};
-tBTA_SYS_SENDMSG bta_closure_sys_sendmsg = NULL;
-
-} // namespace
-
-/* Accept bta_sys_register, and bta_sys_sendmsg. Those parameters can be used to
- * override system methods for tests.
- */
-void bta_closure_init(tBTA_SYS_REGISTER registerer, tBTA_SYS_SENDMSG sender) {
- /* register closure message handler */
- registerer(BTA_ID_CLOSURE, &bta_closure_hw_reg);
- bta_closure_sys_sendmsg = sender;
-}
-
-bool bta_closure_execute(BT_HDR* p_raw_msg) {
- if (p_raw_msg->event != BTA_CLOSURE_EXECUTE_EVT) {
- APPL_TRACE_ERROR("%s: don't know how to execute event type %d", __func__,
- p_raw_msg->event);
- return false;
- }
-
- tBTA_CLOSURE_EXECUTE* p_msg = ((tBTA_CLOSURE_EXECUTE*)p_raw_msg);
-
- APPL_TRACE_API("%s: executing closure %s", __func__,
- p_msg->pending_task.posted_from.ToString().c_str());
- p_msg->pending_task.task.Run();
-
- p_msg->pending_task.~PendingTask();
- return true;
-}
-
-/*
- * This function posts a closure for execution on the btu_bta_msg_queue. Please
- * see documentation at
- * https://www.chromium.org/developers/coding-style/important-abstractions-and-data-structures
- * for how to handle dynamic memory ownership/smart pointers with base::Owned(),
- * base::Passed(), base::ConstRef() and others.
- */
-void do_in_bta_thread(const tracked_objects::Location& from_here,
- const base::Closure& task) {
- APPL_TRACE_API("%s: posting %s", __func__, from_here.ToString().c_str());
- tBTA_CLOSURE_EXECUTE* p_msg =
- (tBTA_CLOSURE_EXECUTE*)osi_malloc(sizeof(tBTA_CLOSURE_EXECUTE));
-
- new (&p_msg->pending_task) PendingTask(from_here, task, TimeTicks(), true);
- p_msg->hdr.event = BTA_CLOSURE_EXECUTE_EVT;
- bta_closure_sys_sendmsg(p_msg);
-}
diff --git a/system/bta/closure/bta_closure_int.h b/system/bta/closure/bta_closure_int.h
deleted file mode 100644
index b0cc726..0000000
--- a/system/bta/closure/bta_closure_int.h
+++ /dev/null
@@ -1,34 +0,0 @@
-/******************************************************************************
- *
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at:
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- ******************************************************************************/
-
-#ifndef BTA_CLOSURE_INT_H
-#define BTA_CLOSURE_INT_H
-
-#include <stdbool.h>
-
-#include "bta/sys/bta_sys.h"
-#include "bta_api.h"
-#include "include/bt_trace.h"
-
-/* Accept bta_sys_register, and bta_sys_sendmsg. Those parameters can be used to
- * override system methods for tests.
- */
-void bta_closure_init(tBTA_SYS_REGISTER registerer, tBTA_SYS_SENDMSG sender);
-bool bta_closure_execute(BT_HDR* p_msg);
-
-#endif /* BTA_CLOSURE_INT_H */
diff --git a/system/bta/sys/bta_sys.h b/system/bta/sys/bta_sys.h
index e079ea4..4edcfb3 100644
--- a/system/bta/sys/bta_sys.h
+++ b/system/bta/sys/bta_sys.h
@@ -28,6 +28,9 @@
#include "bt_target.h"
#include "osi/include/alarm.h"
+#include <base/logging.h>
+#include <base/threading/thread.h>
+
/*****************************************************************************
* Constants and data types
****************************************************************************/
@@ -160,7 +163,6 @@
} tBTA_SYS_HW_MSG;
typedef void (*tBTA_SYS_REGISTER)(uint8_t id, const tBTA_SYS_REG* p_reg);
-typedef void (*tBTA_SYS_SENDMSG)(void* p_msg);
/*****************************************************************************
* Global data
@@ -221,6 +223,8 @@
extern bool bta_sys_is_register(uint8_t id);
extern uint16_t bta_sys_get_sys_features(void);
extern void bta_sys_sendmsg(void* p_msg);
+extern void do_in_bta_thread(const tracked_objects::Location& from_here,
+ const base::Closure& task);
extern void bta_sys_start_timer(alarm_t* alarm, period_ms_t interval,
uint16_t event, uint16_t layer_specific);
extern void bta_sys_disable(tBTA_SYS_HW_MODULE module);
diff --git a/system/bta/sys/bta_sys_main.cc b/system/bta/sys/bta_sys_main.cc
index 6386539..f2453f7 100644
--- a/system/bta/sys/bta_sys_main.cc
+++ b/system/bta/sys/bta_sys_main.cc
@@ -32,7 +32,6 @@
#include "bt_common.h"
#include "bta_api.h"
-#include "bta_closure_int.h"
#include "bta_sys.h"
#include "bta_sys_int.h"
#include "btm_api.h"
@@ -195,8 +194,6 @@
#if (defined BTA_AR_INCLUDED) && (BTA_AR_INCLUDED == true)
bta_ar_init();
#endif
-
- bta_closure_init(bta_sys_register, bta_sys_sendmsg);
}
void bta_sys_free(void) {
@@ -553,6 +550,27 @@
/*******************************************************************************
*
+ * Function do_in_bta_thread
+ *
+ * Description Post a closure to be ran in the bta thread
+ *
+ * Returns void
+ *
+ ******************************************************************************/
+void do_in_bta_thread(const tracked_objects::Location& from_here,
+ const base::Closure& task) {
+ base::MessageLoop* bta_message_loop = get_message_loop();
+
+ if (!bta_message_loop || !bta_message_loop->task_runner().get()) {
+ APPL_TRACE_ERROR("%s: MessageLooper not initialized", __func__);
+ return;
+ }
+
+ bta_message_loop->task_runner()->PostTask(from_here, task);
+}
+
+/*******************************************************************************
+ *
* Function bta_sys_start_timer
*
* Description Start a protocol timer for the specified amount
diff --git a/system/bta/test/bta_closure_test.cc b/system/bta/test/bta_closure_test.cc
deleted file mode 100644
index 9e1e4bb..0000000
--- a/system/bta/test/bta_closure_test.cc
+++ /dev/null
@@ -1,101 +0,0 @@
-/******************************************************************************
- *
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at:
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- ******************************************************************************/
-
-#include <gtest/gtest.h>
-#include <queue>
-
-#include "bta/closure/bta_closure_int.h"
-#include "bta/include/bta_closure_api.h"
-#include "include/bt_trace.h"
-
-namespace {
-
-/* There is no test class, because we talk to C code that accepts plain
- * functions as arguments.
- */
-
-int test_counter = 0;
-tBTA_SYS_EVT_HDLR* closure_handler = NULL;
-std::queue<BT_HDR*> msgs;
-
-void test_plus_one_task() { test_counter++; }
-
-void test_plus_two_task() { test_counter += 2; }
-
-void fake_bta_sys_sendmsg(void* p_msg) { msgs.push((BT_HDR*)p_msg); }
-
-void fake_bta_sys_register(uint8_t id, const tBTA_SYS_REG* p_reg) {
- closure_handler = p_reg->evt_hdlr;
-}
-
-bool fake_bta_sys_sendmsg_execute() {
- BT_HDR* p_msg = msgs.front();
- msgs.pop();
- return closure_handler(p_msg);
-}
-
-} // namespace
-
-// TODO(jpawlowski): there is some weird dependency issue in tests, and the
-// tests here fail to compile without this definition.
-void LogMsg(uint32_t trace_set_mask, const char* fmt_str, ...) {}
-
-TEST(ClosureTest, test_post_task) {
- test_counter = 0;
-
- bta_closure_init(fake_bta_sys_register, fake_bta_sys_sendmsg);
-
- do_in_bta_thread(FROM_HERE, base::Bind(&test_plus_one_task));
- EXPECT_EQ(1U, msgs.size()) << "Message should not be NULL";
-
- EXPECT_TRUE(fake_bta_sys_sendmsg_execute());
- EXPECT_EQ(1, test_counter);
-}
-
-TEST(ClosureTest, test_post_multiple_tasks) {
- test_counter = 0;
-
- bta_closure_init(fake_bta_sys_register, fake_bta_sys_sendmsg);
-
- do_in_bta_thread(FROM_HERE, base::Bind(&test_plus_one_task));
- do_in_bta_thread(FROM_HERE, base::Bind(&test_plus_two_task));
- do_in_bta_thread(FROM_HERE, base::Bind(&test_plus_one_task));
- do_in_bta_thread(FROM_HERE, base::Bind(&test_plus_two_task));
- do_in_bta_thread(FROM_HERE, base::Bind(&test_plus_one_task));
- do_in_bta_thread(FROM_HERE, base::Bind(&test_plus_two_task));
-
- EXPECT_EQ(6U, msgs.size());
-
- EXPECT_TRUE(fake_bta_sys_sendmsg_execute());
- EXPECT_EQ(1, test_counter);
-
- EXPECT_TRUE(fake_bta_sys_sendmsg_execute());
- EXPECT_EQ(3, test_counter);
-
- EXPECT_TRUE(fake_bta_sys_sendmsg_execute());
- EXPECT_EQ(4, test_counter);
-
- EXPECT_TRUE(fake_bta_sys_sendmsg_execute());
- EXPECT_EQ(6, test_counter);
-
- EXPECT_TRUE(fake_bta_sys_sendmsg_execute());
- EXPECT_EQ(7, test_counter);
-
- EXPECT_TRUE(fake_bta_sys_sendmsg_execute());
- EXPECT_EQ(9, test_counter);
-}
diff --git a/system/bta/test/bta_hf_client_test.cc b/system/bta/test/bta_hf_client_test.cc
index 7ca49db..50c24af 100644
--- a/system/bta/test/bta_hf_client_test.cc
+++ b/system/bta/test/bta_hf_client_test.cc
@@ -26,6 +26,10 @@
const RawAddress bdaddr2 = {.address = {0x66, 0x55, 0x44, 0x33, 0x22, 0x11}};
} // namespace
+// TODO(jpawlowski): there is some weird dependency issue in tests, and the
+// tests here fail to compile without this definition.
+void LogMsg(uint32_t trace_set_mask, const char* fmt_str, ...) {}
+
class BtaHfClientTest : public testing::Test {
protected:
void SetUp() override {
