Remove outdated devices from Android device chooser

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"
+#include "base/bind.h"
+#include "base/location.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "device/bluetooth/android/wrappers.h"
 #include "device/bluetooth/bluetooth_advertisement.h"
 #include "device/bluetooth/bluetooth_device_android.h"
 #include "device/bluetooth/bluetooth_discovery_session_outcome.h"
 #include "jni/ChromeBluetoothAdapter_jni.h"
 
 using base::android::AttachCurrentThread;
 using base::android::ConvertJavaStringToUTF8;
 
+namespace {
+// The poll interval in ms when there is no active discovery. This
+// matches the max allowed advertisting interval for connectable
+// devices.
+enum { kPassivePollInterval = 11000 };
+// The poll interval in ms when there is an active discovery.
+enum { kActivePollInterval = 1000 };
+}
+
 namespace device {
 
 // static
 base::WeakPtr<BluetoothAdapter> BluetoothAdapter::CreateAdapter(
     const InitCallback& init_callback) {
   return BluetoothAdapterAndroid::Create(
       BluetoothAdapterWrapper_CreateWithDefaultAdapter().obj());
 }
 
 // static
 base::WeakPtr<BluetoothAdapterAndroid> BluetoothAdapterAndroid::Create(
     jobject bluetooth_adapter_wrapper) {  // Java Type: bluetoothAdapterWrapper
   BluetoothAdapterAndroid* adapter = new BluetoothAdapterAndroid();
 
   adapter->j_adapter_.Reset(Java_ChromeBluetoothAdapter_create(
       AttachCurrentThread(), reinterpret_cast<intptr_t>(adapter),
       bluetooth_adapter_wrapper));
 
+  adapter->ui_task_runner_ = base::ThreadTaskRunnerHandle::Get();
+
   return adapter->weak_ptr_factory_.GetWeakPtr();
 }
 
 // static
 bool BluetoothAdapterAndroid::RegisterJNI(JNIEnv* env) {
   return RegisterNativesImpl(env);  // Generated in BluetoothAdapter_jni.h
 }
 
 std::string BluetoothAdapterAndroid::GetAddress() const {
   return ConvertJavaStringToUTF8(Java_ChromeBluetoothAdapter_getAddress(
@@ skipping 98 matching lines @@
 void BluetoothAdapterAndroid::OnAdapterStateChanged(
     JNIEnv* env,
     const JavaParamRef<jobject>& caller,
     const bool powered) {
   NotifyAdapterPoweredChanged(powered);
 }
 
 void BluetoothAdapterAndroid::OnScanFailed(
     JNIEnv* env,
     const JavaParamRef<jobject>& caller) {
+  numDiscoverySessions = 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>&
         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));
     }
   }
 }
 
 BluetoothAdapterAndroid::BluetoothAdapterAndroid() : weak_ptr_factory_(this) {
 }
 
 BluetoothAdapterAndroid::~BluetoothAdapterAndroid() {
   Java_ChromeBluetoothAdapter_onBluetoothAdapterAndroidDestruction(
       AttachCurrentThread(), j_adapter_.obj());
 }
 
+void BluetoothAdapterAndroid::PurgeTimedOutDevices() {

[ortuno, 2016/06/15 17:20:16]

I'm wondering if we should post a delayed task to RemoveTimedOutDevices after a
device disconnects. What do you think?

[perja, 2016/06/17 13:39:27]

Not sure I see why. It will be kept or purged during the next discover session
like other devices.

+  if (IsDiscovering()) {
+    RemoveTimedOutDevices();
+    ui_task_runner_->PostDelayedTask(
+        FROM_HERE, base::Bind(&BluetoothAdapterAndroid::PurgeTimedOutDevices,
+                              weak_ptr_factory_.GetWeakPtr()),
+        base::TimeDelta::FromMilliseconds(kActivePollInterval));
+  } else {
+    ui_task_runner_->PostDelayedTask(
+        FROM_HERE, base::Bind(&BluetoothAdapterAndroid::RemoveTimedOutDevices,
+                              weak_ptr_factory_.GetWeakPtr()),
+        base::TimeDelta::FromMilliseconds(kPassivePollInterval));
+  }
+}
+
 void BluetoothAdapterAndroid::AddDiscoverySession(
     BluetoothDiscoveryFilter* discovery_filter,
     const base::Closure& callback,
     const DiscoverySessionErrorCallback& error_callback) {
   // TODO(scheib): Support filters crbug.com/490401
-  if (Java_ChromeBluetoothAdapter_addDiscoverySession(AttachCurrentThread(),
-                                                      j_adapter_.obj())) {
+  if (AddDiscoverySessionInternal()) {
     callback.Run();
   } else {
     // TODO(scheib): Eventually wire the SCAN_FAILED result through to here.
     error_callback.Run(UMABluetoothDiscoverySessionOutcome::UNKNOWN);
   }
 }
 
+bool BluetoothAdapterAndroid::AddDiscoverySessionInternal() {

[ortuno, 2016/06/15 17:20:16]

optional: I think this could be moved to AddDiscoverySession. Same with
StopDiscoverySessionInternal.

[perja, 2016/06/17 13:39:27]

Done.

+  if (!IsPowered()) {
+    VLOG(1) << "AddDiscoverySession: Fails: !isPowered";
+    return false;
+  }
+
+  numDiscoverySessions++;
+  VLOG(1) << "AddDiscoverySession: Now " << unsigned(numDiscoverySessions)
+          << " sessions.";
+  if (numDiscoverySessions > 1) {
+    return true;
+  }
+
+  if (!Java_ChromeBluetoothAdapter_addDiscoverySession(AttachCurrentThread(),
+                                                       j_adapter_.obj())) {
+    numDiscoverySessions--;
+    return false;
+  } else {
+    PurgeTimedOutDevices();

[ortuno, 2016/06/15 17:20:16]

This results in mAdapter.isDiscovering() being called right after
scanner.startScan(), which is kind of an async function. Are we sure
isDiscovering will return true? If not, we might want to post a delayed task to
give it time to settle.

[perja, 2016/06/17 13:39:27]

Yeah, that sounds like a good idea. I guess we can't be sure that isDiscovering
will return true right away. Also I tested with the ACTION_DISCOVERY_STARTED
broadcast on Android, but this is only fired for classic BT discovery.

+  }
+  return true;
+}
+
 void BluetoothAdapterAndroid::RemoveDiscoverySession(
     BluetoothDiscoveryFilter* discovery_filter,
     const base::Closure& callback,
     const DiscoverySessionErrorCallback& error_callback) {
-  if (Java_ChromeBluetoothAdapter_removeDiscoverySession(AttachCurrentThread(),
-                                                         j_adapter_.obj())) {
+  if (RemoveDiscoverySessionInternal()) {
     callback.Run();
   } else {
     // TODO(scheib): Eventually wire the SCAN_FAILED result through to here.
     error_callback.Run(UMABluetoothDiscoverySessionOutcome::UNKNOWN);
   }
 }
 
+bool BluetoothAdapterAndroid::RemoveDiscoverySessionInternal() {
+  if (numDiscoverySessions == 0) {
+    DCHECK(false);
+    VLOG(1) << "RemoveDiscoverySession: No scan in progress.";
+    return false;
+  }
+
+  --numDiscoverySessions;
+
+  if (numDiscoverySessions == 0) {
+    VLOG(1) << "RemoveDiscoverySession: Now 0 sessions. Stopping scan.";
+    return Java_ChromeBluetoothAdapter_removeDiscoverySession(
+        AttachCurrentThread(), j_adapter_.obj());
+  }
+
+  VLOG(1) << "RemoveDiscoverySession: Now " << unsigned(numDiscoverySessions)
+          << " sessions.";
+  return true;
+}
+
 void BluetoothAdapterAndroid::SetDiscoveryFilter(
     std::unique_ptr<BluetoothDiscoveryFilter> discovery_filter,
     const base::Closure& callback,
     const DiscoverySessionErrorCallback& error_callback) {
   // TODO(scheib): Support filters crbug.com/490401
   NOTIMPLEMENTED();
   error_callback.Run(UMABluetoothDiscoverySessionOutcome::NOT_IMPLEMENTED);
 }
 
 void BluetoothAdapterAndroid::RemovePairingDelegateInternal(
     device::BluetoothDevice::PairingDelegate* pairing_delegate) {
 }
 
 }  // namespace device