[fido] Avoid re-entrancy when handling BLE connection changes

This patch implements a quick fix for issue 950204 by introducing a
PostTask() when destroying a BluetoothGattConnection object. Since this
can happen while handling the response to GATT requests or when the GATT
connection status changes this introduces re-entrancy which is not
safely handled by the BluetoothDevice object. This change provides a
workaround until that safety can be guaranteed.

Bug: 950204
Change-Id: I6dcce598113ede6e0ae1ed36cebf85937d72c392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1584174Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Auto-Submit: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#654627}

device/fido/ble/fido_ble_connection.cc

@@ -185,6 +185,11 @@ FidoBleConnection::FidoBleConnection(
 }
 
 FidoBleConnection::~FidoBleConnection() {
+  // Workaround for crbug.com/950204, avoids re-entrancy bugs triggered by
+  // closing a GATT connection while handling changes to the GATT connection
+  // state.
+  base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
+                                                  std::move(connection_));
   adapter_->RemoveObserver(this);
 }
 

device/fido/ble/fido_ble_connection_unittest.cc

@@ -123,6 +123,12 @@ class FidoBleConnectionTest : public ::testing::Test {
     BluetoothAdapterFactory::SetAdapterForTesting(adapter_);
   }
 
+  void TearDown() override {
+    // Workaround for the workaround for crbug.com/950204 to allow the
+    // MockBluetoothGattConnection to be destroyed before the test exits.
+    scoped_task_environment_.RunUntilIdle();
+  }
+
   BluetoothAdapter* adapter() { return adapter_.get(); }
   MockBluetoothDevice* device() { return fido_device_; }