Revert "fido: stop scanning for BLE advertisements before connecting"

This reverts commit c10ef92a.

Reason for revert: Causes GATT connection failures on Windows
(crbug.com/1106597)

This is a partial revert. I'm not including updates to the comments in
device/bluetooth/bluetooth_discovery_session.h because they were
unrelated to the actual change and are still correct despite the revert.
(See the PS1/PS2 diff for the re-revert.)

Original change's description:
> fido: stop scanning for BLE advertisements before connecting
>
> Anecdotally, GATT connection reliability on Windows is higher if the
> Bluetooth adapter is not scanning while the connection attempt occurs.
> Hence, make FidoCableDiscovery invoke BluetoothDiscoverySession::Stop()
> before connecting to a device. Obviously, there is a possibility of
> scans occurring for other reasons. But that's unlikely to happen and we
> can't do anything about it.
>
> Note that stopping the scan probably means a single FidoCableDiscovery
> instance can at most connect a single device. However, caBLEv1
> currently only really supports a single authenticator anyway.
>
> Change-Id: Ibaac6bc1563e4429dad0df9570f77c594ed1469c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2211281
> Commit-Queue: Martin Kreichgauer <martinkr@google.com>
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Reviewed-by: Adam Langley <agl@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#781094}

Bug: 1106597
Change-Id: I1e4fcf5b6be1d26a4d7634ad1fe96df1d07b176a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2303683
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789705}

device/fido/cable/fido_cable_discovery.cc

@@ -185,7 +185,6 @@ FidoCableDiscovery::~FidoCableDiscovery() {
   for (auto advertisement : advertisements_) {
     advertisement.second->Unregister(base::DoNothing(), base::DoNothing());
   }
-  StopBluetoothDiscoverySession();
 
   if (adapter_)
     adapter_->RemoveObserver(this);
@@ -290,6 +289,11 @@ void FidoCableDiscovery::OnSetPowered() {
                                 weak_factory_.GetWeakPtr()));
 }
 
+void FidoCableDiscovery::SetDiscoverySession(
+    std::unique_ptr<BluetoothDiscoverySession> discovery_session) {
+  discovery_session_ = std::move(discovery_session);
+}
+
 void FidoCableDiscovery::DeviceAdded(BluetoothAdapter* adapter,
                                      BluetoothDevice* device) {
   if (!IsCableDevice(device))
@@ -384,7 +388,6 @@ void FidoCableDiscovery::FidoCableDeviceTimeout(FidoCableDevice* device) {
 }
 
 void FidoCableDiscovery::StartCableDiscovery() {
-  DCHECK(!discovery_session_);
   adapter()->StartDiscoverySessionWithFilter(
       std::make_unique<BluetoothDiscoveryFilter>(
           BluetoothTransport::BLUETOOTH_TRANSPORT_LE),
@@ -397,14 +400,12 @@ void FidoCableDiscovery::StartCableDiscovery() {
 }
 
 void FidoCableDiscovery::OnStartDiscoverySession(
-    std::unique_ptr<BluetoothDiscoverySession> discovery_session) {
+    std::unique_ptr<BluetoothDiscoverySession> session) {
   FIDO_LOG(DEBUG) << "Discovery session started.";
-  discovery_session_ = std::move(discovery_session);
-
   if (has_v1_discovery_data_) {
     RecordCableV1DiscoveryEventOnce(CableV1DiscoveryEvent::kScanningStarted);
   }
-
+  SetDiscoverySession(std::move(session));
   // Advertising is delayed by 500ms to ensure that any UI has a chance to
   // appear as we don't want to start broadcasting without the user being
   // aware.
@@ -446,15 +447,6 @@ void FidoCableDiscovery::StartAdvertisement() {
   }
 }
 
-void FidoCableDiscovery::StopBluetoothDiscoverySession() {
-  FIDO_LOG(DEBUG) << "Stopping Bluetooth discovery";
-  // BluetoothDiscoverySession::Stop() is considered deprecated, deleting the
-  // instance suffices. Note that general, other parts of Chrome or other
-  // processes on the system may still perform BLE scans, so this is only a
-  // best-effort attempt.
-  discovery_session_.reset();
-}
-
 void FidoCableDiscovery::StopAdvertisements(base::OnceClosure callback) {
   // Destructing a BluetoothAdvertisement invokes its Unregister() method, but
   // there may be references to the advertisement outside this
@@ -477,7 +469,6 @@ void FidoCableDiscovery::StopAdvertisements(base::OnceClosure callback) {
         cb.Run();
       },
       barrier_closure);
-
   for (auto advertisement : advertisements_) {
     advertisement.second->Unregister(barrier_closure, error_closure);
   }
@@ -544,10 +535,6 @@ void FidoCableDiscovery::CableDeviceFound(BluetoothAdapter* adapter,
   active_handshakes_.emplace_back(std::move(cable_device),
                                   std::move(*handshake_handler));
 
-  // Stop discovery and advertisements, then establish a GATT connection and
-  // begin the handshake. On some platforms (Windows), supposedly connecting is
-  // more reliable if there is no discovery running in parallel.
-  StopBluetoothDiscoverySession();
   StopAdvertisements(
       base::BindOnce(&FidoCableDiscovery::ConductEncryptionHandshake,
                      weak_factory_.GetWeakPtr(), handshake_handler_ptr,
@@ -840,7 +827,6 @@ bool FidoCableDiscovery::MaybeStop() {
     NOTREACHED();
   }
   StopAdvertisements(base::DoNothing());
-  StopBluetoothDiscoverySession();
   return true;
 }
 

device/fido/cable/fido_cable_discovery.h

@@ -114,18 +114,19 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoCableDiscovery
   void OnGetAdapter(scoped_refptr<BluetoothAdapter> adapter);
   void OnSetPowered();
 
+  void SetDiscoverySession(
+      std::unique_ptr<BluetoothDiscoverySession> discovery_session);
+
   BluetoothAdapter* adapter() { return adapter_.get(); }
 
+  // Attempt to stop all on-going advertisements in best-effort basis.
+  // Once all the callbacks for Unregister() function is received, invoke
+  // |callback|.
   void StopAdvertisements(base::OnceClosure callback);
   void OnAdvertisementsStopped(base::OnceClosure callback);
-
-  void StopBluetoothDiscoverySession();
-
   void CableDeviceFound(BluetoothAdapter* adapter, BluetoothDevice* device);
-
   void ConductEncryptionHandshake(FidoCableHandshakeHandler* handshake_handler,
                                   CableDiscoveryData::Version cable_version);
-
   void ValidateAuthenticatorHandshakeMessage(
       CableDiscoveryData::Version cable_version,
       FidoCableHandshakeHandler* handshake_handler,