Merge "Clean-up BTIF profile queue on profile shutdown"

system/btif/Android.bp

@@ -120,3 +120,25 @@ cc_test {
     ],
     cflags: ["-DBUILDCFG"],
 }
+
+// btif profile queue unit tests for target
+// ========================================================
+cc_test {
+    name: "net_test_btif_profile_queue",
+    defaults: ["fluoride_defaults"],
+    include_dirs: btifCommonIncludes,
+    srcs: [
+      "src/btif_profile_queue.cc",
+      "test/btif_profile_queue_test.cc"
+    ],
+    shared_libs: [
+        "liblog",
+        "libhardware",
+        "libcutils",
+    ],
+    static_libs: [
+        "libbluetooth-types",
+        "libosi",
+    ],
+    cflags: ["-DBUILDCFG"],
+}

system/btif/include/btif_profile_queue.h

@@ -33,6 +33,7 @@ typedef bt_status_t (*btif_connect_cb_t)(RawAddress* bda, uint16_t uuid);
 
 bt_status_t btif_queue_connect(uint16_t uuid, const RawAddress* bda,
                                btif_connect_cb_t connect_cb);
+void btif_queue_cleanup(uint16_t uuid);
 void btif_queue_advance();
 bt_status_t btif_queue_connect_next(void);
 void btif_queue_release();

system/btif/src/btif_av.cc

@@ -1410,6 +1410,7 @@ static void cleanup(int service_uuid) {
 static void cleanup_src(void) {
   BTIF_TRACE_EVENT("%s", __func__);
 
+  btif_queue_cleanup(UUID_SERVCLASS_AUDIO_SOURCE);
   if (bt_av_src_callbacks) {
     bt_av_src_callbacks = NULL;
     if (bt_av_sink_callbacks == NULL) cleanup(BTA_A2DP_SOURCE_SERVICE_ID);
@@ -1419,6 +1420,7 @@ static void cleanup_src(void) {
 static void cleanup_sink(void) {
   BTIF_TRACE_EVENT("%s", __func__);
 
+  btif_queue_cleanup(UUID_SERVCLASS_AUDIO_SINK);
   if (bt_av_sink_callbacks) {
     bt_av_sink_callbacks = NULL;
     if (bt_av_src_callbacks == NULL) cleanup(BTA_A2DP_SINK_SERVICE_ID);

system/btif/src/btif_hf.cc

@@ -1494,6 +1494,7 @@ bool btif_hf_call_terminated_recently() {
 static void cleanup(void) {
   BTIF_TRACE_EVENT("%s", __func__);
 
+  btif_queue_cleanup(UUID_SERVCLASS_AG_HANDSFREE);
   if (bt_hf_callbacks) {
 #if (defined(BTIF_HF_SERVICES) && (BTIF_HF_SERVICES & BTA_HFP_SERVICE_MASK))
     btif_disable_service(BTA_HFP_SERVICE_ID);

system/btif/src/btif_hf_client.cc

@@ -704,6 +704,7 @@ static bt_status_t request_last_voice_tag_number(const RawAddress* bd_addr) {
 static void cleanup(void) {
   BTIF_TRACE_EVENT("%s", __func__);
 
+  btif_queue_cleanup(UUID_SERVCLASS_HF_HANDSFREE);
   if (bt_hf_client_callbacks) {
     btif_disable_service(BTA_HFP_HS_SERVICE_ID);
     bt_hf_client_callbacks = NULL;

system/btif/src/btif_profile_queue.cc

@@ -44,6 +44,7 @@
 typedef enum {
   BTIF_QUEUE_CONNECT_EVT,
   BTIF_QUEUE_ADVANCE_EVT,
+  BTIF_QUEUE_CLEANUP_EVT
 } btif_queue_event_t;
 
 typedef struct {
@@ -67,6 +68,7 @@ static const size_t MAX_REASONABLE_REQUESTS = 10;
 
 static void queue_int_add(connect_node_t* p_param) {
   if (!connect_queue) {
+    LOG_INFO(LOG_TAG, "%s: allocating profile queue", __func__);
     connect_queue = list_new(osi_free);
     CHECK(connect_queue != NULL);
   }
@@ -105,6 +107,32 @@ static void queue_int_advance() {
   }
 }
 
+static void queue_int_cleanup(uint16_t* p_uuid) {
+  if (!p_uuid) {
+    LOG_ERROR(LOG_TAG, "%s: UUID is null", __func__);
+    return;
+  }
+  uint16_t uuid = *p_uuid;
+  LOG_INFO(LOG_TAG, "%s: UUID=%04X", __func__, uuid);
+  if (!connect_queue) {
+    return;
+  }
+  connect_node_t* connection_request;
+  const list_node_t* node = list_begin(connect_queue);
+  while (node && node != list_end(connect_queue)) {
+    connection_request = (connect_node_t*)list_node(node);
+    node = list_next(node);
+    if (connection_request->uuid == uuid) {
+      LOG_INFO(LOG_TAG,
+               "%s: removing connection request UUID=%04X, bd_addr=%s, busy=%d",
+               __func__, connection_request->uuid,
+               connection_request->bda.ToString().c_str(),
+               connection_request->busy);
+      list_remove(connect_queue, connection_request);
+    }
+  }
+}
+
 static void queue_int_handle_evt(uint16_t event, char* p_param) {
   switch (event) {
     case BTIF_QUEUE_CONNECT_EVT:
@@ -114,6 +142,10 @@ static void queue_int_handle_evt(uint16_t event, char* p_param) {
     case BTIF_QUEUE_ADVANCE_EVT:
       queue_int_advance();
       break;
+
+    case BTIF_QUEUE_CLEANUP_EVT:
+      queue_int_cleanup((uint16_t*)(p_param));
+      return;
   }
 
   if (stack_manager_get_interface()->get_stack_is_running())
@@ -142,6 +174,20 @@ bt_status_t btif_queue_connect(uint16_t uuid, const RawAddress* bda,
                                (char*)&node, sizeof(connect_node_t), NULL);
 }
 
+/*******************************************************************************
+ *
+ * Function         btif_queue_cleanup
+ *
+ * Description      Clean up existing connection requests for a UUID
+ *
+ * Returns          void, always succeed
+ *
+ ******************************************************************************/
+void btif_queue_cleanup(uint16_t uuid) {
+  btif_transfer_context(queue_int_handle_evt, BTIF_QUEUE_CLEANUP_EVT,
+                        (char*)&uuid, sizeof(uint16_t), NULL);
+}
+
 /*******************************************************************************
  *
  * Function         btif_queue_advance
@@ -186,6 +232,7 @@ bt_status_t btif_queue_connect_next(void) {
  *
  ******************************************************************************/
 void btif_queue_release() {
+  LOG_INFO(LOG_TAG, "%s", __func__);
   list_free(connect_queue);
   connect_queue = NULL;
 }

system/btif/test/btif_profile_queue_test.cc

+/******************************************************************************
+ *
+ *  Copyright (C) 2017 Google, Inc.
+ *
+ *  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 "btif/include/btif_profile_queue.h"
+#include "stack_manager.h"
+#include "types/raw_address.h"
+
+static bool sStackRunning;
+
+bool get_stack_is_running(void) { return sStackRunning; }
+
+static stack_manager_t sStackManager = {nullptr, nullptr, nullptr, nullptr,
+                                        get_stack_is_running};
+
+const stack_manager_t* stack_manager_get_interface() { return &sStackManager; }
+
+typedef void(tBTIF_CBACK)(uint16_t event, char* p_param);
+typedef void(tBTIF_COPY_CBACK)(uint16_t event, char* p_dest, char* p_src);
+bt_status_t btif_transfer_context(tBTIF_CBACK* p_cback, uint16_t event,
+                                  char* p_params, int param_len,
+                                  tBTIF_COPY_CBACK* p_copy_cback) {
+  p_cback(event, p_params);
+  return BT_STATUS_SUCCESS;
+}
+
+enum ResultType {
+  NOT_SET = 0,
+  UNKNOWN,
+  UUID1_ADDR1,
+  UUID1_ADDR2,
+  UUID2_ADDR1,
+  UUID2_ADDR2
+};
+
+static ResultType sResult;
+
+class BtifProfileQueueTest : public ::testing::Test {
+ public:
+  static const uint16_t kTestUuid1 = 0x9527;
+  static const uint16_t kTestUuid2 = 0x819F;
+  static const RawAddress kTestAddr1;
+  static const RawAddress kTestAddr2;
+
+ protected:
+  void SetUp() override {
+    sStackRunning = true;
+    sResult = NOT_SET;
+  };
+  void TearDown() override { btif_queue_release(); };
+};
+
+const RawAddress BtifProfileQueueTest::kTestAddr1{
+    {0x11, 0x22, 0x33, 0x44, 0x55, 0x66}};
+const RawAddress BtifProfileQueueTest::kTestAddr2{
+    {0xAB, 0xCD, 0xEF, 0x12, 0x34, 0x56}};
+
+static bt_status_t test_connect_cb(RawAddress* bda, uint16_t uuid) {
+  sResult = UNKNOWN;
+  if (*bda == BtifProfileQueueTest::kTestAddr1) {
+    if (uuid == BtifProfileQueueTest::kTestUuid1) {
+      sResult = UUID1_ADDR1;
+    } else if (uuid == BtifProfileQueueTest::kTestUuid2) {
+      sResult = UUID2_ADDR1;
+    }
+  } else if (*bda == BtifProfileQueueTest::kTestAddr2) {
+    if (uuid == BtifProfileQueueTest::kTestUuid1) {
+      sResult = UUID1_ADDR2;
+    } else if (uuid == BtifProfileQueueTest::kTestUuid2) {
+      sResult = UUID2_ADDR2;
+    }
+  }
+  return BT_STATUS_SUCCESS;
+}
+
+TEST_F(BtifProfileQueueTest, test_connect) {
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, UUID1_ADDR1);
+}
+
+TEST_F(BtifProfileQueueTest, test_connect_same_uuid_do_not_repeat) {
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, UUID1_ADDR1);
+  // Second connection request on the same UUID do not repeat
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, NOT_SET);
+  // Not even after we advance the queue
+  sResult = NOT_SET;
+  btif_queue_advance();
+  btif_queue_connect_next();
+  EXPECT_EQ(sResult, NOT_SET);
+}
+
+TEST_F(BtifProfileQueueTest, test_multiple_connects) {
+  // First item is executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, UUID1_ADDR1);
+  // Second item with advance is executed
+  sResult = NOT_SET;
+  btif_queue_advance();
+  btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, UUID2_ADDR1);
+}
+
+TEST_F(BtifProfileQueueTest, test_multiple_connects_without_advance) {
+  // First item is executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, UUID1_ADDR1);
+  // Second item without advance is not executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, NOT_SET);
+  sResult = NOT_SET;
+  // Connect next doesn't work
+  btif_queue_connect_next();
+  EXPECT_EQ(sResult, NOT_SET);
+  // Advance moves queue to execute next item
+  sResult = NOT_SET;
+  btif_queue_advance();
+  EXPECT_EQ(sResult, UUID2_ADDR1);
+}
+
+TEST_F(BtifProfileQueueTest, test_cleanup_first_allow_second) {
+  // First item is executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, UUID1_ADDR1);
+  // Second item without advance is not executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, NOT_SET);
+  // Connect next doesn't work
+  sResult = NOT_SET;
+  btif_queue_connect_next();
+  EXPECT_EQ(sResult, NOT_SET);
+  // Cleanup UUID1 allows the next profile connection to be executed
+  sResult = NOT_SET;
+  btif_queue_cleanup(kTestUuid1);
+  btif_queue_connect_next();
+  EXPECT_EQ(sResult, UUID2_ADDR1);
+}
+
+TEST_F(BtifProfileQueueTest, test_cleanup_both) {
+  // First item is executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, UUID1_ADDR1);
+  // Second item without advance is not executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, NOT_SET);
+  // Connect next doesn't work
+  sResult = NOT_SET;
+  btif_queue_connect_next();
+  EXPECT_EQ(sResult, NOT_SET);
+  // Cleanup both leaves nothing to execute
+  sResult = NOT_SET;
+  btif_queue_cleanup(kTestUuid1);
+  btif_queue_cleanup(kTestUuid2);
+  btif_queue_connect_next();
+  EXPECT_EQ(sResult, NOT_SET);
+}
+
+TEST_F(BtifProfileQueueTest, test_cleanup_both_reverse_order) {
+  // First item is executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, UUID1_ADDR1);
+  // Second item without advance is not executed
+  sResult = NOT_SET;
+  btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb);
+  EXPECT_EQ(sResult, NOT_SET);
+  // Connect next doesn't work
+  sResult = NOT_SET;
+  btif_queue_connect_next();
+  EXPECT_EQ(sResult, NOT_SET);
+  // Cleanup both in reverse order leaves nothing to execute
+  sResult = NOT_SET;
+  btif_queue_cleanup(kTestUuid2);
+  btif_queue_cleanup(kTestUuid1);
+  btif_queue_connect_next();
+  EXPECT_EQ(sResult, NOT_SET);
+}

system/test/run_unit_tests.sh

@@ -7,6 +7,7 @@ known_tests=(
   net_test_btcore
   net_test_bta
   net_test_btif
+  net_test_btif_profile_queue
   net_test_device
   net_test_hci
   net_test_stack