bluetooth: Refactor how we update based on Advertising Data

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 1388 matching lines @@
   }
   if (type == mojom::BluetoothPropertyType::ALL ||
       type == mojom::BluetoothPropertyType::BDADDR) {
     mojom::BluetoothPropertyPtr btp = mojom::BluetoothProperty::New();
     btp->set_bdaddr(mojom::BluetoothAddress::From(device->GetAddress()));
     properties.push_back(std::move(btp));
   }
   if (type == mojom::BluetoothPropertyType::ALL ||
       type == mojom::BluetoothPropertyType::UUIDS) {
     mojom::BluetoothPropertyPtr btp = mojom::BluetoothProperty::New();
-    std::vector<BluetoothUUID> uuids = device->GetUUIDs();
+    BluetoothDevice::UUIDSet uuids = device->GetUUIDs();
     mojo::Array<mojom::BluetoothUUIDPtr> uuid_results =
-        mojo::Array<mojom::BluetoothUUIDPtr>::New(0);
+        mojo::Array<mojom::BluetoothUUIDPtr>::New(uuids.size());
 
-    for (auto& uuid : uuids) {
-      uuid_results.push_back(mojom::BluetoothUUID::From(uuid));
+    size_t i = 0;
+    for (const auto& uuid : uuids) {
+      uuid_results[i] = mojom::BluetoothUUID::From(uuid);

[Jeffrey Yasskin, 2016/08/18 16:04:32]

Too bad they don't have tests. You need to i++ in this loop.

[ortuno, 2016/08/19 20:50:32]

Done.

     }
 
     btp->set_uuids(std::move(uuid_results));
     properties.push_back(std::move(btp));
   }
   if (type == mojom::BluetoothPropertyType::ALL ||
       type == mojom::BluetoothPropertyType::CLASS_OF_DEVICE) {
     mojom::BluetoothPropertyPtr btp = mojom::BluetoothProperty::New();
     btp->set_device_class(device->GetBluetoothClass());
     properties.push_back(std::move(btp));
@@ skipping 110 matching lines @@
   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::UUIDList uuid_list = device->GetUUIDs();
-  if (uuid_list.size() > 0) {
+    BluetoothDevice::UUIDSet uuid_set = device->GetUUIDs();

[Jeffrey Yasskin, 2016/08/18 16:04:32]

This indentation's wrong.

[ortuno, 2016/08/19 20:50:32]

Done.

+  if (uuid_set.size() > 0) {
     mojom::BluetoothAdvertisingDataPtr service_uuids =
         mojom::BluetoothAdvertisingData::New();
-    service_uuids->set_service_uuids(
-        mojo::Array<mojom::BluetoothUUIDPtr>::From(uuid_list));
+    mojo::Array<mojom::BluetoothUUIDPtr> advertised_uuids(uuid_set.size());
+    size_t i = 0;
+    for (const auto& uuid : uuid_set) {
+      advertised_uuids[i] = mojom::BluetoothUUID::From(uuid);

[Jeffrey Yasskin, 2016/08/18 16:04:32]

Another missing i++.

[ortuno, 2016/08/19 20:50:32]

Done.

+    }
+    service_uuids->set_service_uuids(std::move(advertised_uuids));
     advertising_data.push_back(std::move(service_uuids));
   }
 
   // Service data
   for (auto& uuid : device->GetServiceDataUUIDs()) {

[Jeffrey Yasskin, 2016/08/18 16:04:32]

Not from this change, but can this be a 'const auto&', or even a 'const
BluetoothUUID&'?

[ortuno, 2016/08/19 20:50:32]

Done.

-    base::BinaryValue* data = device->GetServiceData(uuid);
-    if (data->GetSize() == 0)
-      continue;
-    std::string data_str;
-    if (!data->GetAsString(&data_str))
-      continue;
-
     mojom::BluetoothAdvertisingDataPtr service_data_element =
         mojom::BluetoothAdvertisingData::New();
     mojom::BluetoothServiceDataPtr service_data =
         mojom::BluetoothServiceData::New();
 
     std::string uuid_str = uuid.canonical_value();
     // Convert xxxxyyyy-xxxx-xxxx-xxxx-xxxxxxxxxxxx to int16 yyyy
     service_data->uuid_16bit = std::stoi(uuid_str.substr(4, 4), nullptr, 16);
-    for (auto& c : data_str) {
-      service_data->data.push_back(c);
+
+    const std::vector<uint8_t>* data = device->GetServiceDataForUUID(uuid);
+    DCHECK(data != nullptr);
+
+    for (const uint8_t& byte : *data) {

[Jeffrey Yasskin, 2016/08/18 16:04:32]

I think you can use "service_data->data.assign(data->begin(), data->end())" here
instead of the loop.

[ortuno, 2016/08/19 20:50:32]

Close. service_data->data is a mojo::Array and that doesn't have an assign
method. But I copied the data to another vector and used mojo::Array::Swap.

+      service_data->data.push_back(byte);
     }
     service_data_element->set_service_data(std::move(service_data));
     advertising_data.push_back(std::move(service_data_element));
   }
 
   return advertising_data;
 }
 
 void ArcBluetoothBridge::SendCachedDevicesFound() const {
   // Send devices that have already been discovered, but aren't connected.
@@ skipping 79 matching lines @@
   LOG(WARNING) << "Bluetooth instance is too old (version " << version
                << ") need version " << version_need;
   return false;
 }
 
 bool ArcBluetoothBridge::CalledOnValidThread() {
   return thread_checker_.CalledOnValidThread();
 }
 
 }  // namespace arc