arc: bluetooth: Separate code path of classic discovery and LE scan

Uses a separate discovery session for LE scan requested by ARC.
Avoids triggering the DiscoveryStateChanged event for LE scan since
Android doesn't use it for LE scan. Note that We still use a timer for
LE scan since we cannot set scan parameters and filters now, while
this is not expected by Android.

BUG=b:145257789
BUG=b:152463320
TEST=Automatic cts tests passed;
TEST=Manually test with the app mentioned in the bug thread, the
behavior becomes normal.

Change-Id: I9b9d24827896678b24592260eefd48e7aa2b89c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2010606Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Commit-Queue: Jie Jiang <jiejiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760371}

chrome/browser/chromeos/arc/bluetooth/arc_bluetooth_bridge.cc

@@ -58,10 +58,10 @@ using device::BluetoothAdvertisement;
 using device::BluetoothDevice;
 using device::BluetoothDiscoveryFilter;
 using device::BluetoothDiscoverySession;
-using device::BluetoothGattConnection;
-using device::BluetoothGattNotifySession;
 using device::BluetoothGattCharacteristic;
+using device::BluetoothGattConnection;
 using device::BluetoothGattDescriptor;
+using device::BluetoothGattNotifySession;
 using device::BluetoothGattService;
 using device::BluetoothLocalGattCharacteristic;
 using device::BluetoothLocalGattDescriptor;
@@ -1120,27 +1120,28 @@ void ArcBluetoothBridge::SetAdapterProperty(
 
 void ArcBluetoothBridge::StartDiscovery() {
   discovery_queue_.Push(base::BindOnce(&ArcBluetoothBridge::StartDiscoveryImpl,
-                                       weak_factory_.GetWeakPtr(), false));
+                                       weak_factory_.GetWeakPtr()));
 }
 
-void ArcBluetoothBridge::StartDiscoveryImpl(bool le_scan) {
+void ArcBluetoothBridge::StartDiscoveryImpl() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  DCHECK(bluetooth_adapter_);
+  if (!bluetooth_adapter_) {
+    LOG(DFATAL) << "Bluetooth adapter does not exist.";
+    return;
+  }
 
   if (discovery_session_) {
     LOG(ERROR) << "Discovery session already running; Reset timeout.";
-    discovery_off_timer_.Start(FROM_HERE, kDiscoveryTimeout,
-                               base::Bind(&ArcBluetoothBridge::CancelDiscovery,
-                                          weak_factory_.GetWeakPtr()));
+    discovery_off_timer_.Start(
+        FROM_HERE, kDiscoveryTimeout,
+        base::BindOnce(&ArcBluetoothBridge::CancelDiscovery,
+                       weak_factory_.GetWeakPtr()));
     discovered_devices_.clear();
     discovery_queue_.Pop();
     return;
   }
 
-  bluetooth_adapter_->StartDiscoverySessionWithFilter(
-      le_scan ? std::make_unique<BluetoothDiscoveryFilter>(
-                    device::BLUETOOTH_TRANSPORT_LE)
-              : nullptr,
+  bluetooth_adapter_->StartDiscoverySession(
       base::Bind(&ArcBluetoothBridge::OnDiscoveryStarted,
                  weak_factory_.GetWeakPtr()),
       base::Bind(&ArcBluetoothBridge::OnDiscoveryError,
@@ -1152,9 +1153,35 @@ void ArcBluetoothBridge::CancelDiscovery() {
                                        weak_factory_.GetWeakPtr()));
 }
 
+void ArcBluetoothBridge::StartLEScanImpl() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  if (!bluetooth_adapter_) {
+    LOG(DFATAL) << "Bluetooth adapter does not exist.";
+    return;
+  }
+
+  if (le_scan_session_) {
+    LOG(ERROR) << "Discovery session for LE scan already running.";
+    le_scan_off_timer_.Start(
+        FROM_HERE, kDiscoveryTimeout,
+        base::BindOnce(&ArcBluetoothBridge::StopLEScanByTimer,
+                       weak_factory_.GetWeakPtr()));
+    discovery_queue_.Pop();
+    return;
+  }
+
+  bluetooth_adapter_->StartDiscoverySessionWithFilter(
+      std::make_unique<BluetoothDiscoveryFilter>(
+          device::BLUETOOTH_TRANSPORT_LE),
+      base::Bind(&ArcBluetoothBridge::OnLEScanStarted,
+                 weak_factory_.GetWeakPtr()),
+      base::Bind(&ArcBluetoothBridge::OnLEScanError,
+                 weak_factory_.GetWeakPtr()));
+}
+
 void ArcBluetoothBridge::CancelDiscoveryImpl() {
   discovery_off_timer_.Stop();
-  discovery_session_.reset();
+  discovery_session_ = nullptr;
   auto* bluetooth_instance = ARC_GET_INSTANCE_FOR_METHOD(
       arc_bridge_service_->bluetooth(), OnDiscoveryStateChanged);
   if (bluetooth_instance != nullptr) {
@@ -1164,6 +1191,12 @@ void ArcBluetoothBridge::CancelDiscoveryImpl() {
   discovery_queue_.Pop();
 }
 
+void ArcBluetoothBridge::StopLEScanImpl() {
+  le_scan_off_timer_.Stop();
+  le_scan_session_ = nullptr;
+  discovery_queue_.Pop();
+}
+
 void ArcBluetoothBridge::OnPoweredOn(
     ArcBluetoothBridge::AdapterStateCallback callback,
     bool save_user_pref) const {
@@ -1205,9 +1238,10 @@ void ArcBluetoothBridge::OnDiscoveryStarted(
   // We need to set timer to turn device discovery off because of the difference
   // between Android API (do device discovery once) and Chrome API (do device
   // discovery until user turns it off).
-  discovery_off_timer_.Start(FROM_HERE, kDiscoveryTimeout,
-                             base::Bind(&ArcBluetoothBridge::CancelDiscovery,
-                                        weak_factory_.GetWeakPtr()));
+  discovery_off_timer_.Start(
+      FROM_HERE, kDiscoveryTimeout,
+      base::BindOnce(&ArcBluetoothBridge::CancelDiscovery,
+                     weak_factory_.GetWeakPtr()));
   discovery_session_ = std::move(session);
   discovered_devices_.clear();
 
@@ -1220,6 +1254,24 @@ void ArcBluetoothBridge::OnDiscoveryStarted(
   discovery_queue_.Pop();
 }
 
+void ArcBluetoothBridge::OnLEScanStarted(
+    std::unique_ptr<BluetoothDiscoverySession> session) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  // TODO(b/152463320): Android expects to stop the LE scan by itself but not by
+  // a timer automatically. We set this timer here due to the potential
+  // complains about the power consumption since we cannot set scan parameters
+  // and filters now.
+  le_scan_off_timer_.Start(
+      FROM_HERE, kDiscoveryTimeout,
+      base::BindOnce(&ArcBluetoothBridge::StopLEScanByTimer,
+                     weak_factory_.GetWeakPtr()));
+  le_scan_session_ = std::move(session);
+
+  // Android doesn't need a callback for discovery started event for a LE scan.
+  discovery_queue_.Pop();
+}
+
 void ArcBluetoothBridge::CreateBond(mojom::BluetoothAddressPtr addr,
                                     int32_t transport) {
   std::string addr_str = addr->To<std::string>();
@@ -1290,12 +1342,22 @@ void ArcBluetoothBridge::GetConnectionState(
 }
 
 void ArcBluetoothBridge::StartLEScan() {
-  discovery_queue_.Push(base::BindOnce(&ArcBluetoothBridge::StartDiscoveryImpl,
-                                       weak_factory_.GetWeakPtr(), true));
+  discovery_queue_.Push(base::BindOnce(&ArcBluetoothBridge::StartLEScanImpl,
+                                       weak_factory_.GetWeakPtr()));
 }
 
 void ArcBluetoothBridge::StopLEScan() {
-  CancelDiscovery();
+  discovery_queue_.Push(base::BindOnce(&ArcBluetoothBridge::StopLEScanImpl,
+                                       weak_factory_.GetWeakPtr()));
+}
+
+void ArcBluetoothBridge::StopLEScanByTimer() {
+  // If the scan is stopped by the timer, it is possible that the following scan
+  // client in Android cannot start the scan successfully but that client will
+  // not get an error.
+  LOG(WARNING) << "The discovery session for LE scan is stopped by the timer";
+  discovery_queue_.Push(base::BindOnce(&ArcBluetoothBridge::StopLEScanImpl,
+                                       weak_factory_.GetWeakPtr()));
 }
 
 void ArcBluetoothBridge::OnGattConnectStateChanged(
@@ -2283,6 +2345,11 @@ void ArcBluetoothBridge::OnDiscoveryError() {
   discovery_queue_.Pop();
 }
 
+void ArcBluetoothBridge::OnLEScanError() {
+  LOG(WARNING) << "failed to start LE scan";
+  discovery_queue_.Pop();
+}
+
 void ArcBluetoothBridge::OnPairing(mojom::BluetoothAddressPtr addr) const {
   auto* bluetooth_instance = ARC_GET_INSTANCE_FOR_METHOD(
       arc_bridge_service_->bluetooth(), OnBondStateChanged);

chrome/browser/chromeos/arc/bluetooth/arc_bluetooth_bridge.h

@@ -346,8 +346,15 @@ class ArcBluetoothBridge
       int32_t adv_handle,
       ReleaseAdvertisementHandleCallback callback);
 
-  void StartDiscoveryImpl(bool le_scan);
+  // StartDiscovery() is used for scanning both BR/EDR and LE devices, while
+  // StartLEScan() is only for LE devices.
+  void StartDiscoveryImpl();
   void CancelDiscoveryImpl();
+  void StartLEScanImpl();
+  void StopLEScanImpl();
+
+  // The callback function triggered by le_scan_off_timer_.
+  void StopLEScanByTimer();
 
   // Power state change on Bluetooth adapter.
   enum class AdapterPowerState { TURN_OFF, TURN_ON };
@@ -359,6 +366,9 @@ class ArcBluetoothBridge
   void OnDiscoveryStarted(
       std::unique_ptr<device::BluetoothDiscoverySession> session);
   void OnDiscoveryError();
+  void OnLEScanStarted(
+      std::unique_ptr<device::BluetoothDiscoverySession> session);
+  void OnLEScanError();
   void OnPairing(mojom::BluetoothAddressPtr addr) const;
   void OnPairedDone(mojom::BluetoothAddressPtr addr) const;
   void OnPairedError(
@@ -572,8 +582,16 @@ class ArcBluetoothBridge
 
   scoped_refptr<bluez::BluetoothAdapterBlueZ> bluetooth_adapter_;
   scoped_refptr<device::BluetoothAdvertisement> advertisment_;
+  // Discovery session created by StartDiscovery().
   std::unique_ptr<device::BluetoothDiscoverySession> discovery_session_;
-  // Discovered devices in the current discovery session.
+  // Discovery session created by StartLEScan().
+  std::unique_ptr<device::BluetoothDiscoverySession> le_scan_session_;
+  // Discovered devices in the current discovery session started by
+  // StartDiscovery(). We don't need to keep track of this for StartLEScan()
+  // since Android don't have a callback for new found devices in LE scan. When
+  // a new advertisement of an LE device comes, DeviceAdertismentReceived() will
+  // be called and we pass the result to Android via OnLEDeviceFound(), and then
+  // it will notify the LE scanner in Android.
   std::set<std::string> discovered_devices_;
   std::unordered_map<std::string,
                      std::unique_ptr<device::BluetoothGattNotifySession>>
@@ -605,8 +623,12 @@ class ArcBluetoothBridge
   };
   std::unordered_map<std::string, GattConnection> gatt_connections_;
 
-  // Timer to turn discovery off.
+  // Timer to turn off discovery_session_.
   base::OneShotTimer discovery_off_timer_;
+  // Timer to turn off le_scan_session_.
+  // TODO(b/152463320): Remove this timer after the platform side supports
+  // setting scan parameters and filters.
+  base::OneShotTimer le_scan_off_timer_;
   // Timer to turn adapter discoverable off.
   base::OneShotTimer discoverable_off_timer_;
   // Adapter discoverable timeout value.
@@ -636,6 +658,8 @@ class ArcBluetoothBridge
   std::map<int32_t, scoped_refptr<device::BluetoothAdvertisement>>
       advertisements_;
   ArcBluetoothTaskQueue advertisement_queue_;
+  // This queue will hold requests from both Start/CancelDiscovery() and
+  // Start/StopLEScan().
   ArcBluetoothTaskQueue discovery_queue_;
 
   // Rfcomm sockets that live in Chrome.