arc: bluetooth: Use uuid 128 bit for service uuid

components/arc/bluetooth/arc_bluetooth_bridge.cc

Index: components/arc/bluetooth/arc_bluetooth_bridge.cc
diff --git a/components/arc/bluetooth/arc_bluetooth_bridge.cc b/components/arc/bluetooth/arc_bluetooth_bridge.cc
index 3b8cf87ba4cd2e19c14df0cc88ef04ff501e8bc6..1f37d6925366674e8cc32aafd44fe3fda2fd32c1 100644
--- a/components/arc/bluetooth/arc_bluetooth_bridge.cc
+++ b/components/arc/bluetooth/arc_bluetooth_bridge.cc
@@ -194,7 +194,13 @@ bool IsGattOffsetValid(int offset) {
   return 0 <= offset && offset < kMaxGattAttributeLength;
 }
 
-// This is needed because Android only support UUID 16 bits in advertising data.
+bool isUuid16(const BluetoothUUID& uuid) {

[Rahul Chaturvedi, 2016/09/29 22:10:47]

Can't we get this from uuid.Format()?

[puthik_chromium, 2016/09/29 22:39:25]

uuid.Format() return the size of uuid string given in constructor.

For example
BluetoothUUID a("00001234-0000-1000-8000-00805f9b34fb");
BluetoothUUID b("1234");

Then the following conditions are true
a == b
a.Format() == BluetoothUUID::kFormat128Bit
b.Format() == BluetoothUUID::kFormat16Bit

+  // Convert to uuid16 and compare with original uuid.
+  return uuid == BluetoothUUID(uuid.canonical_value().substr(4, 4));
+}
+
+// This is needed because Android only support UUID 16 bits in service data
+// section in advertising data
 uint16_t GetUUID16(const BluetoothUUID& uuid) {
   // Convert xxxxyyyy-xxxx-xxxx-xxxx-xxxxxxxxxxxx to int16 yyyy
   return std::stoi(uuid.canonical_value().substr(4, 4), nullptr, 16);
@@ -1864,8 +1870,8 @@ ArcBluetoothBridge::GetAdapterProperties(
 // Android support 5 types of Advertising Data.
 // However Chrome didn't expose AdvertiseFlag and ManufacturerData.
 // So we will only expose local_name, service_uuids and service_data.
-// Note that we need to use UUID 16 bits because Android does not support
-// UUID 128 bits.
+// Note that we need to use UUID 16 bits in service_data section
+// because Android does not support UUID 128 bits there.
 // TODO(crbug.com/618442) Make Chrome expose missing data.
 mojo::Array<mojom::BluetoothAdvertisingDataPtr>
 ArcBluetoothBridge::GetAdvertisingData(BluetoothDevice* device) const {
@@ -1879,18 +1885,24 @@ ArcBluetoothBridge::GetAdvertisingData(BluetoothDevice* device) const {
   advertising_data.push_back(std::move(local_name));
 
   // ServiceUuid
-  BluetoothDevice::UUIDSet uuid_set = device->GetUUIDs();
+  const BluetoothDevice::UUIDSet& uuid_set = device->GetUUIDs();
   if (uuid_set.size() > 0) {
-    mojom::BluetoothAdvertisingDataPtr service_uuids_16 =
+    mojom::BluetoothAdvertisingDataPtr service_uuids =
         mojom::BluetoothAdvertisingData::New();
-    mojo::Array<uint16_t> uuid16s(uuid_set.size());
-    size_t i = 0;
-    for (const auto& uuid : uuid_set) {
-      uuid16s[i] = GetUUID16(uuid);
-      i++;
+    if (std::all_of(uuid_set.begin(), uuid_set.end(), isUuid16)) {
+      mojo::Array<uint16_t> uuid16s;
+      uuid16s.reserve(uuid_set.size());
+      for (const auto& uuid : uuid_set)
+        uuid16s.push_back(GetUUID16(uuid));
+      service_uuids->set_service_uuids_16(std::move(uuid16s));

[Rahul Chaturvedi, 2016/09/29 22:10:47]

Why do 16 bit UUIDs need to be handled differently? Can't we just handle all via
the BluetoothUUID type and translate it to the correct type on the Android side?

It seems odd that we're handling 16 bit UUIDs one way, but 32 bit and 128 bit
UUIDs another way.

[puthik_chromium, 2016/09/29 22:39:25]

Why do 16 bit UUIDs need to be handled differently?
Because for max length of advertising data is only 63 byte.

Actually we can implement this in 3 ways
1. Implement it like this. (may be add another case for 32 bit uuid)
2. Always use 128 bit UUIDs in Chrome and translate to correct type in Android.
3. Make BlueZ expose raw advertising data and send it to Android.

So you think the best approach is (2)?
In my opinion, (1) and (2) are not much different. I can change this to use (2)
approach but if (3) is possible I think that might be the best.

[Rahul Chaturvedi, 2016/09/29 22:53:39]

Android's implementation and APIs can change (in fact, often do with every
release). We should keep our interface as flexible as possible, so 3 sounds like
the best option.

That being said, it does seem a bit like overkill for here, so if you want to do
2, that works too. I would prefer to do any processing of the data we send to
Android to be done on Android rather than in ARC code.

+    } else {
+      mojo::Array<BluetoothUUID> uuids;
+      uuids.reserve(uuid_set.size());
+      for (const auto& uuid : uuid_set)
+        uuids.push_back(uuid);
+      service_uuids->set_service_uuids(std::move(uuids));
     }
-    service_uuids_16->set_service_uuids_16(std::move(uuid16s));
-    advertising_data.push_back(std::move(service_uuids_16));
+    advertising_data.push_back(std::move(service_uuids));
   }
 
   // Service data
@@ -1900,6 +1912,7 @@ ArcBluetoothBridge::GetAdvertisingData(BluetoothDevice* device) const {
     mojom::BluetoothServiceDataPtr service_data =
         mojom::BluetoothServiceData::New();
 
+    // Android only support UUID 16 bit here.

[Luis Héctor Chávez, 2016/09/29 21:10:15]

nit: s/support/supports/

[puthik_chromium, 2016/10/05 18:18:57]

Done.

     service_data->uuid_16bit = GetUUID16(uuid);
 
     const std::vector<uint8_t>* data = device->GetServiceDataForUUID(uuid);