Remove outdated devices from Android device chooser

device/bluetooth/bluetooth_adapter_android.cc

Index: device/bluetooth/bluetooth_adapter_android.cc
diff --git a/device/bluetooth/bluetooth_adapter_android.cc b/device/bluetooth/bluetooth_adapter_android.cc
index c642e03a005a21ed06fa744274e98da7af34edb0..70446f2173b7f38f1b6afc82fa2138655a7bcb68 100644
--- a/device/bluetooth/bluetooth_adapter_android.cc
+++ b/device/bluetooth/bluetooth_adapter_android.cc
@@ -8,6 +8,8 @@
 
 #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"
@@ -20,6 +22,15 @@
 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
@@ -38,6 +49,8 @@ base::WeakPtr<BluetoothAdapterAndroid> BluetoothAdapterAndroid::Create(
       AttachCurrentThread(), reinterpret_cast<intptr_t>(adapter),
       bluetooth_adapter_wrapper));
 
+  adapter->ui_task_runner_ = base::ThreadTaskRunnerHandle::Get();
+
   return adapter->weak_ptr_factory_.GetWeakPtr();
 }
 
@@ -156,6 +169,7 @@ void BluetoothAdapterAndroid::OnAdapterStateChanged(
 void BluetoothAdapterAndroid::OnScanFailed(
     JNIEnv* env,
     const JavaParamRef<jobject>& caller) {
+  numDiscoverySessions = 0;
   MarkDiscoverySessionsAsInactive();
 }
 
@@ -175,6 +189,7 @@ void BluetoothAdapterAndroid::CreateOrUpdateDeviceOnScan(
     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_,
@@ -183,6 +198,7 @@ void BluetoothAdapterAndroid::CreateOrUpdateDeviceOnScan(
     // 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));
@@ -198,13 +214,27 @@ BluetoothAdapterAndroid::~BluetoothAdapterAndroid() {
       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.
@@ -212,12 +242,34 @@ void BluetoothAdapterAndroid::AddDiscoverySession(
   }
 }
 
+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.
@@ -225,6 +277,26 @@ void BluetoothAdapterAndroid::RemoveDiscoverySession(
   }
 }
 
+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,