bluetooth: Replace old advertised uuids

device/bluetooth/bluetooth_adapter_android.cc

 // Copyright 2015 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 "device/bluetooth/bluetooth_adapter_android.h"
 
 #include <memory>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
@@ skipping 163 matching lines @@
   num_discovery_sessions_ = 0;
   MarkDiscoverySessionsAsInactive();
 }
 
 void BluetoothAdapterAndroid::CreateOrUpdateDeviceOnScan(
     JNIEnv* env,
     const JavaParamRef<jobject>& caller,
     const JavaParamRef<jstring>& address,
     const JavaParamRef<jobject>&
         bluetooth_device_wrapper,  // Java Type: bluetoothDeviceWrapper
-    const JavaParamRef<jobject>&
+    const JavaParamRef<jobjectArray>&
         advertised_uuids) {  // Java Type: List<ParcelUuid>
   std::string device_address = ConvertJavaStringToUTF8(env, address);
   DevicesMap::const_iterator iter = devices_.find(device_address);
 
   if (iter == devices_.end()) {
     // New device.
     BluetoothDeviceAndroid* device_android =
         BluetoothDeviceAndroid::Create(this, bluetooth_device_wrapper);
     device_android->UpdateAdvertisedUUIDs(advertised_uuids);
     device_android->UpdateTimestamp();
     devices_.add(device_address,
                  std::unique_ptr<BluetoothDevice>(device_android));
     FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
                       DeviceAdded(this, device_android));
   } else {
     // Existing device.
     BluetoothDeviceAndroid* device_android =
         static_cast<BluetoothDeviceAndroid*>(iter->second);
     device_android->UpdateTimestamp();
-    if (device_android->UpdateAdvertisedUUIDs(advertised_uuids)) {
-      FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
-                        DeviceChanged(this, device_android));
-    }
+    device_android->UpdateAdvertisedUUIDs(advertised_uuids);
+
+    FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,

[scheib, 2016/08/05 18:49:31]

Only notify if UUIDs are different.

[ortuno, 2016/08/05 23:00:16]

This specific change has little to do with the advertised UUIDs. This is
changing DeviceChanged to be called for *every* new advertisement. This is what
will allow the chooser to know if the device is actually around. For example:

1. Turn on Playbulb
2. requestDevice()
3. The chooser will:
   1. Add any connected devices that match
   2. Start a scan
   3. Add devices as DeviceAdded and DeviceChanged are called.
4. See Playbulb.

So far so good.

1. Turn off Playbulb
2. requestDevice()
3. The chooser will do the same steps as above but the device won't get added to
the chooser because DeviceChanged was never called so the device can't possibly
be around.
4. Users are happy because they don't see the device they know is no longer
around.

If we were to only notify when the UUIDs change we would no longer be able to
perform step 3 and the Playbulb will still show up in the chooser even though
it's not on.

Besides the reason stated above. CreateOrUpdateDeviceOnScan will also eventually
update rssi. rssi changes for every single packet so we would basically notify
for every packet regardless of whether services have changed or not.

[scheib, 2016/08/06 00:21:41]

OK, thanks for explanation -- Document on adapter::Observer that DeviceChanged
is expected for all advertisements - even for stable values then? Or, make
another notification?

[ortuno, 2016/08/08 22:56:39]

Added a comment to explicitly mention that DeviceChanged is essentially called
for every advertisement packet because the RSSI is always changing. Regarding
adding a new notification: Our hands are tied by BlueZ here. I thought through a
couple of options to implement given the BlueZ restrictions:

Say we wanted to introduce a really useful function that tells us everything
that's included in an adv packet. There are two reasons why this wouldn't work.
(1) We don't have access to each packet so we wouldn't know what's in each
packet so we would tell the client the wrong information. (2) BlueZ doesn't
update all properties at once, it notifies of each change so for example a
device that is alternating names and service data:

 1. Notify of Name change.
 2. Notify of RSSI change.
 3. Notify of Service Data change.

We would have to implement some sort of bounce mechanism so that we can group
the changes for each advertisement.

Say we wanted to introduce a simple notification for when adv's are received:

1. AdvReceived called when RSSI changes -> We have no guarantee of the order in
which properties are updated so clients can't know if the information is up to
date when the get AdvReceived. For example

   1. Device advertises "0x01" as adv data and we get RSSI: -100
   2. BlueZ calls PropertyChanged with RSSI and we call AdvReceived.
   3. Client sees AdvReceived and tries to get the advertisement data but sees
no data :(
   4. BlueZ calls PropertyChanged with Adv Data "0x01".

2. AdvReceived called when RSSI, TxPower, Device Name, or any of the other adv
related properties changes -> We would call AdvReceived for each of these that
change which would confuse clients.

   1. Device advertises "0x01" as adv data and we get RSSI: -100
   2. Bluez calls PropertyChanged with RSSI and we call AdvReceived.
   3. BlueZ calls PropertyChanged with Adv Data and we call AdvReceived.
   4. BlueZ calls PropertyChanged with Name and we call AdvReceived.
   ... and so on for each property that changed in the adv packet.

After thinking through this I think the best solution would be for BlueZ to just
have an Advertisement Packet property that's just raw bytes. Then we can parse
it an update the properties all at once.

Anyway, I think any of those options needs a bigger refactoring. We can open an
issue if we feel that is important.

+                      DeviceChanged(this, device_android));
   }
 }
 
 BluetoothAdapterAndroid::BluetoothAdapterAndroid() : weak_ptr_factory_(this) {
 }
 
 BluetoothAdapterAndroid::~BluetoothAdapterAndroid() {
   Java_ChromeBluetoothAdapter_onBluetoothAdapterAndroidDestruction(
       AttachCurrentThread(), j_adapter_.obj());
 }
@@ skipping 84 matching lines @@
   // TODO(scheib): Support filters crbug.com/490401
   NOTIMPLEMENTED();
   error_callback.Run(UMABluetoothDiscoverySessionOutcome::NOT_IMPLEMENTED);
 }
 
 void BluetoothAdapterAndroid::RemovePairingDelegateInternal(
     device::BluetoothDevice::PairingDelegate* pairing_delegate) {
 }
 
 }  // namespace device