arc: bluetooth: Use uuid 128 bit for service uuid

components/arc/bluetooth/arc_bluetooth_bridge.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/arc/bluetooth/arc_bluetooth_bridge.h"
 
 #include <bluetooth/bluetooth.h>
 #include <fcntl.h>
 #include <stddef.h>
 #include <sys/socket.h>
@@ skipping 176 matching lines @@
   if (status == arc::mojom::BluetoothGattStatus::GATT_SUCCESS)
     success_callback.Run();
   else
     error_callback.Run();
 }
 
 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);
 }
 
 arc::mojom::BluetoothPropertyPtr GetDiscoveryTimeoutProperty(uint32_t timeout) {
   arc::mojom::BluetoothPropertyPtr property =
       arc::mojom::BluetoothProperty::New();
   property->set_discovery_timeout(timeout);
   return property;
@@ skipping 1649 matching lines @@
     btp->set_local_le_features(std::move(le_features));
     properties.push_back(std::move(btp));
   }
 
   return properties;
 }
 
 // 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 {
   mojo::Array<mojom::BluetoothAdvertisingDataPtr> advertising_data;
 
   // LocalName
   mojom::BluetoothAdvertisingDataPtr local_name =
       mojom::BluetoothAdvertisingData::New();
   local_name->set_local_name(device->GetName() ? device->GetName().value()
                                                : nullptr);
   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
   for (const BluetoothUUID& uuid : device->GetServiceDataUUIDs()) {
     mojom::BluetoothAdvertisingDataPtr service_data_element =
         mojom::BluetoothAdvertisingData::New();
     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);
     DCHECK(data != nullptr);
 
     std::vector<uint8_t> data_copy = *data;
     service_data->data.Swap(&data_copy);
 
     service_data_element->set_service_data(std::move(service_data));
     advertising_data.push_back(std::move(service_data_element));
@@ skipping 117 matching lines @@
   sdp_bluetooth_instance->OnGetSdpRecords(
       status, std::move(remote_addr), target_uuid,
       mojo::Array<mojom::BluetoothSdpRecordPtr>::New(0));
 }
 
 bool ArcBluetoothBridge::CalledOnValidThread() {
   return thread_checker_.CalledOnValidThread();
 }
 
 }  // namespace arc