CRAS: Avoid the trigger of cb for non-audio device

We should not start conn watch for non-audio bt device. There are only
two conditions we should do so:
1. The bt device connects with any audio profile CRAS supported, which
means it is an audio bt device.
2. Any new audio profile CRAS supported is added when the device is
already connected.

Update unittest to reflect the change. We should also add unit tests
testing above scenarios. Will have them in a separate CL.

BUG=chromium:755242
TEST=Pairing other BT devices when BT headphone is playing youtube. Also
test the reconnect scenario.

Change-Id: Id560a42eea73eaf8072fea6f184abb73075c3755
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/1763517
Tested-by: En-Shuo Hsu <enshuo@google.com>
Reviewed-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Commit-Queue: En-Shuo Hsu <enshuo@google.com>

2 files changed, 115 insertions, 27 deletions

diff --git a/cras/src/server/cras_bt_device.c b/cras/src/server/cras_bt_device.c
index 79fe4ab7..bc6b43d9 100644
--- a/cras/src/server/cras_bt_device.c
+++ b/cras/src/server/cras_bt_device.c
@@ -54,6 +54,11 @@ static const unsigned int CONN_WATCH_PERIOD_MS = 2000;
 static const unsigned int CONN_WATCH_MAX_RETRIES = 30;
 static const unsigned int PROFILE_CONN_RETRIES = 3;
 
+static const unsigned int CRAS_SUPPORTED_PROFILES =
+	CRAS_BT_DEVICE_PROFILE_A2DP_SINK |
+	CRAS_BT_DEVICE_PROFILE_HFP_HANDSFREE |
+	CRAS_BT_DEVICE_PROFILE_HSP_AUDIOGATEWAY;
+
 /* Object to represent a general bluetooth device, and used to
  * associate with some CRAS modules if it supports audio.
  * Members:
@@ -635,9 +640,7 @@ void cras_bt_device_set_connected(struct cras_bt_device *device, int value)
 
 	device->connected = value;
 
-	if (device->connected) {
-		cras_bt_device_start_new_conn_watch_timer(device);
-	} else if (device->conn_watch_timer) {
+	if (!device->connected && device->conn_watch_timer) {
 		cras_tm_cancel_timer(tm, device->conn_watch_timer);
 		device->conn_watch_timer = NULL;
 	}
@@ -668,11 +671,6 @@ void cras_bt_device_notify_profile_dropped(struct cras_bt_device *device,
 int cras_bt_device_add_supported_profiles(struct cras_bt_device *device,
 					  const char *uuid)
 {
-	static unsigned int audio_profiles =
-		CRAS_BT_DEVICE_PROFILE_A2DP_SINK |
-		CRAS_BT_DEVICE_PROFILE_HFP_HANDSFREE |
-		CRAS_BT_DEVICE_PROFILE_HSP_AUDIOGATEWAY;
-
 	enum cras_bt_device_profile profile =
 		cras_bt_device_profile_from_uuid(uuid);
 
@@ -685,21 +683,20 @@ int cras_bt_device_add_supported_profiles(struct cras_bt_device *device,
 
 	/* Log this event as we might need to re-intialize the BT audio nodes
 	 * if new audio profile is reported for already connected device. */
-	if (device->connected && (profile & audio_profiles))
+	if (device->connected && (profile & CRAS_SUPPORTED_PROFILES))
 		BTLOG(btlog, BT_NEW_AUDIO_PROFILE_AFTER_CONNECT,
 		      device->profiles, profile);
 	device->profiles |= profile;
 	cras_bt_device_log_profile(device, profile);
 
-	return (profile & audio_profiles);
+	return (profile & CRAS_SUPPORTED_PROFILES);
 }
 
 void cras_bt_device_update_properties(struct cras_bt_device *device,
 				      DBusMessageIter *properties_array_iter,
 				      DBusMessageIter *invalidated_array_iter)
 {
-	int has_new_audio_profile = 0;
-
+	int watch_needed = 0;
 	while (dbus_message_iter_get_arg_type(properties_array_iter) !=
 	       DBUS_TYPE_INVALID) {
 		DBusMessageIter properties_dict_iter, variant_iter;
@@ -750,6 +747,10 @@ void cras_bt_device_update_properties(struct cras_bt_device *device,
 				device->trusted = value;
 			} else if (strcmp(key, "Connected") == 0) {
 				cras_bt_device_set_connected(device, value);
+				watch_needed = device->connected &&
+					       cras_bt_device_supports_profile(
+						       device,
+						       CRAS_SUPPORTED_PROFILES);
 			}
 
 		} else if (strcmp(dbus_message_iter_get_signature(&variant_iter),
@@ -766,9 +767,18 @@ void cras_bt_device_update_properties(struct cras_bt_device *device,
 				dbus_message_iter_get_basic(&uuid_array_iter,
 							    &uuid);
 
+				/*
+				 * If updated properties includes new audio
+				 * profile, and device is connected, we need
+				 * to start connection watcher. This is needed
+				 * because on some bluetooth device, supported
+				 * profiles do not present when device
+				 * interface is added and they are updated
+				 * later.
+				 */
 				if (cras_bt_device_add_supported_profiles(
 					    device, uuid))
-					has_new_audio_profile = 1;
+					watch_needed = device->connected;
 
 				dbus_message_iter_next(&uuid_array_iter);
 			}
@@ -808,14 +818,8 @@ void cras_bt_device_update_properties(struct cras_bt_device *device,
 		dbus_message_iter_next(invalidated_array_iter);
 	}
 
-	/* If updated properties includes new audio profile, and device is
-	 * connected, we need to start connection watcher. This is needed
-	 * because on some bluetooth device, supported profiles do not present
-	 * when device interface is added and they are updated later.
-	 */
-	if (has_new_audio_profile && device->connected) {
+	if (watch_needed)
 		cras_bt_device_start_new_conn_watch_timer(device);
-	}
 }
 
 /* Converts bluetooth address string into sockaddr structure. The address
diff --git a/cras/src/tests/bt_device_unittest.cc b/cras/src/tests/bt_device_unittest.cc
index 2d1fd253..d2e9a13f 100644
--- a/cras/src/tests/bt_device_unittest.cc
+++ b/cras/src/tests/bt_device_unittest.cc
@@ -42,6 +42,13 @@ static const char* dbus_message_new_method_call_method;
 static struct cras_bt_device* cras_a2dp_connected_device_ret;
 static struct cras_bt_device* cras_a2dp_suspend_connected_device_dev;
 
+struct MockDBusMessage {
+  int type;
+  void* value;
+  MockDBusMessage* next;
+  MockDBusMessage* recurse;
+};
+
 void ResetStubData() {
   cras_bt_io_get_profile_ret = NULL;
   cras_bt_io_create_called = 0;
@@ -61,6 +68,28 @@ void ResetStubData() {
   cras_a2dp_connected_device_ret = NULL;
 }
 
+static void FreeMockDBusMessage(MockDBusMessage* head) {
+  if (head->next != NULL)
+    FreeMockDBusMessage(head->next);
+  if (head->recurse != NULL)
+    FreeMockDBusMessage(head->recurse);
+  if (head->type == DBUS_TYPE_STRING)
+    free((char*)head->value);
+  delete head;
+}
+
+static struct MockDBusMessage* NewMockDBusConnectedMessage() {
+  MockDBusMessage* msg = new MockDBusMessage{DBUS_TYPE_ARRAY, NULL};
+  MockDBusMessage* dict =
+      new MockDBusMessage{DBUS_TYPE_STRING, (void*)strdup("Connected")};
+  MockDBusMessage* variant = new MockDBusMessage{DBUS_TYPE_BOOLEAN, (void*)1};
+
+  msg->recurse = dict;
+  dict->next = new MockDBusMessage{DBUS_TYPE_INVALID, NULL};
+  dict->next->recurse = variant;
+  return msg;
+}
+
 namespace {
 
 class BtDeviceTestSuite : public testing::Test {
@@ -179,7 +208,7 @@ TEST_F(BtDeviceTestSuite, SwitchProfile) {
 
 TEST_F(BtDeviceTestSuite, SetDeviceConnectedA2dpOnly) {
   struct cras_bt_device* device;
-
+  struct MockDBusMessage *msg_root, *cur;
   ResetStubData();
 
   device = cras_bt_device_create(NULL, FAKE_OBJ_PATH);
@@ -187,7 +216,8 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedA2dpOnly) {
 
   cras_bt_device_add_supported_profiles(device, A2DP_SINK_UUID);
 
-  cras_bt_device_set_connected(device, 1);
+  cur = msg_root = NewMockDBusConnectedMessage();
+  cras_bt_device_update_properties(device, (DBusMessageIter*)&cur, NULL);
   EXPECT_EQ(1, cras_tm_create_timer_called);
   EXPECT_NE((void*)NULL, cras_tm_create_timer_cb);
 
@@ -204,10 +234,12 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedA2dpOnly) {
   EXPECT_EQ(1, cras_a2dp_start_called);
 
   cras_bt_device_remove(device);
+  FreeMockDBusMessage(msg_root);
 }
 
 TEST_F(BtDeviceTestSuite, SetDeviceConnectedHfpHspOnly) {
   struct cras_bt_device* device;
+  struct MockDBusMessage *msg_root, *cur;
 
   ResetStubData();
 
@@ -217,7 +249,8 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedHfpHspOnly) {
   cras_bt_device_add_supported_profiles(device, HSP_HS_UUID);
   cras_bt_device_add_supported_profiles(device, HFP_HF_UUID);
 
-  cras_bt_device_set_connected(device, 1);
+  cur = msg_root = NewMockDBusConnectedMessage();
+  cras_bt_device_update_properties(device, (DBusMessageIter*)&cur, NULL);
   EXPECT_EQ(1, cras_tm_create_timer_called);
   EXPECT_NE((void*)NULL, cras_tm_create_timer_cb);
 
@@ -235,10 +268,12 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedHfpHspOnly) {
   EXPECT_EQ(1, cras_hfp_ag_start_called);
 
   cras_bt_device_remove(device);
+  FreeMockDBusMessage(msg_root);
 }
 
 TEST_F(BtDeviceTestSuite, SetDeviceConnectedA2dpHfpHsp) {
   struct cras_bt_device* device;
+  struct MockDBusMessage *msg_root, *cur;
 
   ResetStubData();
 
@@ -249,7 +284,8 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedA2dpHfpHsp) {
   cras_bt_device_add_supported_profiles(device, HSP_HS_UUID);
   cras_bt_device_add_supported_profiles(device, HFP_HF_UUID);
 
-  cras_bt_device_set_connected(device, 1);
+  cur = msg_root = NewMockDBusConnectedMessage();
+  cras_bt_device_update_properties(device, (DBusMessageIter*)&cur, NULL);
   EXPECT_EQ(1, cras_tm_create_timer_called);
   EXPECT_NE((void*)NULL, cras_tm_create_timer_cb);
 
@@ -275,10 +311,12 @@ TEST_F(BtDeviceTestSuite, SetDeviceConnectedA2dpHfpHsp) {
   EXPECT_EQ(1, cras_hfp_ag_start_called);
 
   cras_bt_device_remove(device);
+  FreeMockDBusMessage(msg_root);
 }
 
 TEST_F(BtDeviceTestSuite, DevConnectedConflictCheck) {
   struct cras_bt_device* device;
+  struct MockDBusMessage *msg_root, *cur;
 
   ResetStubData();
 
@@ -289,7 +327,8 @@ TEST_F(BtDeviceTestSuite, DevConnectedConflictCheck) {
   cras_bt_device_add_supported_profiles(device, HSP_HS_UUID);
   cras_bt_device_add_supported_profiles(device, HFP_HF_UUID);
 
-  cras_bt_device_set_connected(device, 1);
+  cur = msg_root = NewMockDBusConnectedMessage();
+  cras_bt_device_update_properties(device, (DBusMessageIter*)&cur, NULL);
   cras_bt_device_audio_gateway_initialized(device);
   cras_bt_device_a2dp_configured(device);
   EXPECT_EQ(1, cras_tm_create_timer_called);
@@ -310,10 +349,12 @@ TEST_F(BtDeviceTestSuite, DevConnectedConflictCheck) {
   EXPECT_EQ(1, cras_hfp_ag_start_called);
 
   cras_bt_device_remove(device);
+  FreeMockDBusMessage(msg_root);
 }
 
 TEST_F(BtDeviceTestSuite, A2dpDropped) {
   struct cras_bt_device* device;
+  struct MockDBusMessage *msg_root, *cur;
 
   ResetStubData();
 
@@ -324,7 +365,8 @@ TEST_F(BtDeviceTestSuite, A2dpDropped) {
   cras_bt_device_add_supported_profiles(device, HSP_HS_UUID);
   cras_bt_device_add_supported_profiles(device, HFP_HF_UUID);
 
-  cras_bt_device_set_connected(device, 1);
+  cur = msg_root = NewMockDBusConnectedMessage();
+  cras_bt_device_update_properties(device, (DBusMessageIter*)&cur, NULL);
   EXPECT_EQ(1, cras_tm_create_timer_called);
   EXPECT_NE((void*)NULL, cras_tm_create_timer_cb);
 
@@ -351,10 +393,12 @@ TEST_F(BtDeviceTestSuite, A2dpDropped) {
   EXPECT_STREQ("Disconnect", dbus_message_new_method_call_method);
 
   cras_bt_device_remove(device);
+  FreeMockDBusMessage(msg_root);
 }
 
 TEST_F(BtDeviceTestSuite, ConnectionWatchTimeout) {
   struct cras_bt_device* device;
+  struct MockDBusMessage *msg_root, *cur;
 
   ResetStubData();
 
@@ -365,7 +409,8 @@ TEST_F(BtDeviceTestSuite, ConnectionWatchTimeout) {
   cras_bt_device_add_supported_profiles(device, HSP_HS_UUID);
   cras_bt_device_add_supported_profiles(device, HFP_HF_UUID);
 
-  cras_bt_device_set_connected(device, 1);
+  cur = msg_root = NewMockDBusConnectedMessage();
+  cras_bt_device_update_properties(device, (DBusMessageIter*)&cur, NULL);
   EXPECT_EQ(1, cras_tm_create_timer_called);
   EXPECT_NE((void*)NULL, cras_tm_create_timer_cb);
 
@@ -395,6 +440,7 @@ TEST_F(BtDeviceTestSuite, ConnectionWatchTimeout) {
   EXPECT_STREQ("Disconnect", dbus_message_new_method_call_method);
 
   cras_bt_device_remove(device);
+  FreeMockDBusMessage(msg_root);
 }
 
 /* Stubs */
@@ -584,6 +630,44 @@ dbus_bool_t dbus_pending_call_set_notify(DBusPendingCall* pending,
   return true;
 }
 
+void dbus_message_iter_recurse(DBusMessageIter* iter, DBusMessageIter* sub) {
+  MockDBusMessage* msg = *(MockDBusMessage**)iter;
+  MockDBusMessage** cur = (MockDBusMessage**)sub;
+  *cur = msg->recurse;
+}
+
+dbus_bool_t dbus_message_iter_next(DBusMessageIter* iter) {
+  MockDBusMessage** cur = (MockDBusMessage**)iter;
+  MockDBusMessage* msg = *cur;
+  *cur = msg->next;
+  return true;
+}
+
+int dbus_message_iter_get_arg_type(DBusMessageIter* iter) {
+  MockDBusMessage* msg;
+
+  if (iter == NULL)
+    return DBUS_TYPE_INVALID;
+
+  msg = *(MockDBusMessage**)iter;
+  if (msg == NULL)
+    return DBUS_TYPE_INVALID;
+
+  return msg->type;
+}
+
+void dbus_message_iter_get_basic(DBusMessageIter* iter, void* value) {
+  MockDBusMessage* msg = *(MockDBusMessage**)iter;
+  switch (msg->type) {
+    case DBUS_TYPE_BOOLEAN:
+      memcpy(value, &msg->value, sizeof(int));
+      break;
+    case DBUS_TYPE_STRING:
+      memcpy(value, &msg->value, sizeof(char*));
+      break;
+  }
+}
+
 }  // extern "C"
 }  // namespace