bluetooth: Refactor how we update based on Advertising Data

device/bluetooth/bluetooth_adapter_mac.mm

Index: device/bluetooth/bluetooth_adapter_mac.mm
diff --git a/device/bluetooth/bluetooth_adapter_mac.mm b/device/bluetooth/bluetooth_adapter_mac.mm
index dca907ffdeac4f616fb7be80f9b5869985590224..3a9a66d250e11e00f8b3b062a2aae125951da9cc 100644
--- a/device/bluetooth/bluetooth_adapter_mac.mm
+++ b/device/bluetooth/bluetooth_adapter_mac.mm
@@ -330,8 +330,12 @@ void BluetoothAdapterMac::RemoveDiscoverySession(
     }
   }
   if (transport & BLUETOOTH_TRANSPORT_LE) {
-    if (IsLowEnergyAvailable())
+    if (IsLowEnergyAvailable()) {
       low_energy_discovery_manager_->StopDiscovery();
+      for (const auto& device_id_object_pair : devices_) {
+        device_id_object_pair.second->ClearAdvertisementData();
+      }
+    }
   }
 
   DVLOG(1) << "Discovery stopped";
@@ -481,43 +485,75 @@ void BluetoothAdapterMac::LowEnergyDeviceUpdated(
     int rssi) {
   BluetoothLowEnergyDeviceMac* device_mac =
       GetBluetoothLowEnergyDeviceMac(peripheral);
-  // if has no entry in the map, create new device and insert into |devices_|,
+  // If has no entry in the map, create new device and insert into |devices_|,
   // otherwise update the existing device.
-  if (!device_mac) {
+  bool is_new_device = device_mac == nullptr;

[Jeffrey Yasskin, 2016/08/18 19:56:30]

Consider making this const, so it's really clear it doesn't change over the
course of the function.

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

Done.

+  if (is_new_device) {
     VLOG(1) << "LowEnergyDeviceUpdated new device";
     // A new device has been found.
-    device_mac = new BluetoothLowEnergyDeviceMac(this, peripheral,
-                                                 advertisement_data, rssi);
+    device_mac = new BluetoothLowEnergyDeviceMac(this, peripheral);
+  } else {
+    // Check that there are no collisions.
+    std::string stored_device_id = device_mac->GetIdentifier();
+    std::string updated_device_id =
+        BluetoothLowEnergyDeviceMac::GetPeripheralIdentifier(peripheral);
+    if (stored_device_id != updated_device_id) {
+      VLOG(1)
+          << "LowEnergyDeviceUpdated stored_device_id != updated_device_id: "
+          << std::endl
+          << "  " << stored_device_id << std::endl
+          << "  " << updated_device_id;
+      // Collision, two identifiers map to the same hash address.  With a 48 bit
+      // hash the probability of this occuring with 10,000 devices
+      // simultaneously present is 1e-6 (see
+      // https://en.wikipedia.org/wiki/Birthday_problem#Probability_table).  We
+      // ignore the second device by returning.
+      return;
+    }
+  }
+
+  DCHECK(device_mac);
+
+  // Get Advertised UUIDs
+  std::vector<std::string> advertised_uuids;
+  NSArray* service_uuids =
+      [advertisement_data objectForKey:CBAdvertisementDataServiceUUIDsKey];
+  for (CBUUID* uuid in service_uuids) {
+    advertised_uuids.push_back([[uuid UUIDString] UTF8String]);
+  }
+  NSArray* overflow_service_uuids = [advertisement_data
+      objectForKey:CBAdvertisementDataOverflowServiceUUIDsKey];
+  for (CBUUID* uuid in overflow_service_uuids) {
+    advertised_uuids.push_back([[uuid UUIDString] UTF8String]);
+  }
+
+  // Get Service Data.
+  std::unordered_map<std::string, std::vector<uint8_t>> service_data_map;

[Jeffrey Yasskin, 2016/08/18 19:56:30]

Would it make sense to use a BluetoothUUID as the key of this map?

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

Done.

+  NSDictionary* service_data =
+      [advertisement_data objectForKey:CBAdvertisementDataServiceDataKey];
+  for (CBUUID* uuid in service_data) {
+    NSData* data = [service_data objectForKey:uuid];
+    const uint8_t* bytes = static_cast<const uint8_t*>([data bytes]);
+    size_t length = [data length];
+    service_data_map.emplace(
+        std::piecewise_construct,

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

I wouldn't bother with the complexity of piecewise_construct here. Strings and
vectors are cheap to move, so you don't need to avoid constructing them outside
of the constructor. So, .emplace([[uuid UUIDString] UTF8String],
std::vector<uint8_t>(bytes, bytes + length)) should be fine.

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

Done.

+        std::forward_as_tuple([[uuid UUIDString] UTF8String]),
+        std::forward_as_tuple(bytes, bytes + length));
+  }
+
+  device_mac->UpdateAdvertisementData(std::move(advertised_uuids),
+                                      std::move(service_data_map));
+
+  if (is_new_device) {
     std::string device_address =
         BluetoothLowEnergyDeviceMac::GetPeripheralHashAddress(peripheral);
     devices_.add(device_address, std::unique_ptr<BluetoothDevice>(device_mac));
     FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
                       DeviceAdded(this, device_mac));
-    return;
-  }
-
-  std::string stored_device_id = device_mac->GetIdentifier();
-  std::string updated_device_id =
-      BluetoothLowEnergyDeviceMac::GetPeripheralIdentifier(peripheral);
-  if (stored_device_id != updated_device_id) {
-    VLOG(1) << "LowEnergyDeviceUpdated stored_device_id != updated_device_id: "
-            << std::endl
-            << "  " << stored_device_id << std::endl
-            << "  " << updated_device_id;
-    // Collision, two identifiers map to the same hash address.  With a 48 bit
-    // hash the probability of this occuring with 10,000 devices
-    // simultaneously present is 1e-6 (see
-    // https://en.wikipedia.org/wiki/Birthday_problem#Probability_table).  We
-    // ignore the second device by returning.
-    return;
+  } else {
+    FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
+                      DeviceChanged(this, device_mac));
   }
-
-  // A device has an update.
-  VLOG(2) << "LowEnergyDeviceUpdated";
-  device_mac->Update(advertisement_data, rssi);
-
-  FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
-                    DeviceChanged(this, device_mac));
 }
 
 // TODO(krstnmnlsn): Implement. crbug.com/511025