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) {
+  // 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
+  if (!isUuid16(uuid))
+    LOG(WARNING) << "Illegal conversion to UUID16 " << uuid.canonical_value();
   return std::stoi(uuid.canonical_value().substr(4, 4), nullptr, 16);
 }
 
 mojo::Array<arc::mojom::BluetoothPropertyPtr> GetDiscoveryTimeoutProperty(
     uint32_t timeout) {
   arc::mojom::BluetoothPropertyPtr property =
       arc::mojom::BluetoothProperty::New();
   property->set_discovery_timeout(timeout);
   mojo::Array<arc::mojom::BluetoothPropertyPtr> properties;
   properties.push_back(std::move(property));
@@ skipping 1639 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

[Luis Héctor Chávez, 2016/09/28 16:11:48]

This does not seem to be true anymore? Or at least needs some rewording.

[puthik_chromium, 2016/09/29 21:06:50]

Done.

 // UUID 128 bits.
 // 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();
   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(uuid_set.size());
+      size_t i = 0;
+      for (const auto& uuid : uuid_set) {
+        uuid16s[i] = GetUUID16(uuid);
+        i++;

[Luis Héctor Chávez, 2016/09/28 16:11:48]

Ugh, more of this pattern :(

Can you try using this change? https://codereview.chromium.org/2380643003 if it
works, I'll send it for review.

[puthik_chromium, 2016/09/29 02:06:08]

It works.
Thanks for doing this.

+      }
+      service_uuids->set_service_uuids_16(std::move(uuid16s));
+    } else {
+      mojo::Array<BluetoothUUID> uuids(uuid_set.size());
+      size_t i = 0;
+      for (const auto& uuid : uuid_set) {
+        uuids[i] = uuid;
+        i++;
+      }
+      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();
 
     service_data->uuid_16bit = GetUUID16(uuid);

[Luis Héctor Chávez, 2016/09/28 16:11:48]

In which cases |uuid| won't be an uuid16? Will it happen often and cause a lot
of logspam? Can we somehow prevent it from happening by e.g. filtering these
cases?

[puthik_chromium, 2016/09/29 21:06:50]

I think it is rare not to have uuid 16 bit for service data.
But I will remove the log to prevent log spam.

 
     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 116 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