diff options
author | sacomoto <sacomoto@chromium.org> | 2015-07-06 08:21:51 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-07-06 15:22:34 +0000 |
commit | a1518bdf22dbaeb7386bad47571012b3b0dc60a3 (patch) | |
tree | ed66a262dc4b9336e590aebe00b4b3c1dceb0f96 /components/proximity_auth | |
parent | e7ec0943debc8862080e8bf84653ebdde90c95fc (diff) | |
download | chromium_src-a1518bdf22dbaeb7386bad47571012b3b0dc60a3.zip chromium_src-a1518bdf22dbaeb7386bad47571012b3b0dc60a3.tar.gz chromium_src-a1518bdf22dbaeb7386bad47571012b3b0dc60a3.tar.bz2 |
Fixing a reconnection bug in ProximityAuthBleSystem
BluetoothLowEnergyConnectionFinder was unable to reconnect to a BLE
device immediatelly after (< 1s) after successfully disconnecting from
it.
This was caused by a race condition with |BluetoothDevice::Disconnect()|.
Basically, the last call to |BluetoothDevice::Disconnect()| was not
completed before the next |BluetoothDevice::CreateGattConnection()|
was done. So, |BluetoothDevice::Disconnect()| would complete after, and
interrupt the later call to |BluetoothDevice::CreateGattConnection()|.
The use of |BluetoothDevice::Disconnect()| was incorrect in this case,
as it effectively kills all connections to the given device. We should
use |BluetoothGattConnection::Disconnect()| instead, as it only kills
a given |BluetoothGattConnection| instance. This change solves the
problem.
BUG=506633
Review URL: https://codereview.chromium.org/1216413005
Cr-Commit-Position: refs/heads/master@{#337397}
Diffstat (limited to 'components/proximity_auth')
6 files changed, 38 insertions, 35 deletions
diff --git a/components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.cc b/components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.cc index 838ee6e..60d15c5 100644 --- a/components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.cc +++ b/components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.cc @@ -85,15 +85,22 @@ void BluetoothLowEnergyCharacteristicsFinder::GattDiscoveryCompleteForService( void BluetoothLowEnergyCharacteristicsFinder::ScanRemoteCharacteristics( BluetoothDevice* device, const BluetoothUUID& service_uuid) { + PA_LOG(INFO) << "Scanning remote characteristics."; if (device) { std::vector<BluetoothGattService*> services = device->GetGattServices(); for (const auto& service : services) { if (service->GetUUID() == service_uuid) { + PA_LOG(INFO) << "Service " << service_uuid.canonical_value() + << " found."; // Right service found, now scaning its characteristics. std::vector<device::BluetoothGattCharacteristic*> characteristics = service->GetCharacteristics(); - for (const auto& characteristic : characteristics) + for (const auto& characteristic : characteristics) { + PA_LOG(INFO) << "Char: " + << characteristic->GetUUID().canonical_value() + << " found."; HandleCharacteristicUpdate(characteristic); + } break; } } diff --git a/components/proximity_auth/ble/bluetooth_low_energy_connection.cc b/components/proximity_auth/ble/bluetooth_low_energy_connection.cc index 42c7655..11b4a2f 100644 --- a/components/proximity_auth/ble/bluetooth_low_energy_connection.cc +++ b/components/proximity_auth/ble/bluetooth_low_energy_connection.cc @@ -53,7 +53,7 @@ BluetoothLowEnergyConnection::BluetoothLowEnergyConnection( remote_service_({remote_service_uuid, ""}), to_peripheral_char_({to_peripheral_char_uuid, ""}), from_peripheral_char_({from_peripheral_char_uuid, ""}), - connection_(gatt_connection.Pass()), + gatt_connection_(gatt_connection.Pass()), sub_status_(SubStatus::DISCONNECTED), receiving_bytes_(false), write_remote_characteristic_pending_(false), @@ -76,8 +76,8 @@ BluetoothLowEnergyConnection::~BluetoothLowEnergyConnection() { } void BluetoothLowEnergyConnection::Connect() { - if (connection_ && connection_->IsConnected()) { - OnGattConnectionCreated(connection_.Pass()); + if (gatt_connection_ && gatt_connection_->IsConnected()) { + OnGattConnectionCreated(gatt_connection_.Pass()); return; } @@ -97,17 +97,12 @@ void BluetoothLowEnergyConnection::Disconnect() { ClearWriteRequestsQueue(); StopNotifySession(); SetSubStatus(SubStatus::DISCONNECTED); - if (connection_) { - connection_.reset(); - BluetoothDevice* device = GetRemoteDevice(); - if (device) { - PA_LOG(INFO) << "Disconnect from device " << device->GetAddress(); - device->Disconnect( - base::Bind(&BluetoothLowEnergyConnection::OnDisconnected, - weak_ptr_factory_.GetWeakPtr()), - base::Bind(&BluetoothLowEnergyConnection::OnDisconnectError, - weak_ptr_factory_.GetWeakPtr())); - } + if (gatt_connection_) { + PA_LOG(INFO) << "Disconnect from device " + << gatt_connection_->GetDeviceAddress(); + + // Destroying BluetoothGattConnection also disconnects it. + gatt_connection_.reset(); } } } @@ -273,7 +268,7 @@ void BluetoothLowEnergyConnection::OnCreateGattConnectionError( void BluetoothLowEnergyConnection::OnGattConnectionCreated( scoped_ptr<device::BluetoothGattConnection> gatt_connection) { - connection_ = gatt_connection.Pass(); + gatt_connection_ = gatt_connection.Pass(); SetSubStatus(SubStatus::WAITING_CHARACTERISTICS); characteristic_finder_.reset(CreateCharacteristicsFinder( base::Bind(&BluetoothLowEnergyConnection::OnCharacteristicsFound, diff --git a/components/proximity_auth/ble/bluetooth_low_energy_connection.h b/components/proximity_auth/ble/bluetooth_low_energy_connection.h index 648b199..9a57608 100644 --- a/components/proximity_auth/ble/bluetooth_low_energy_connection.h +++ b/components/proximity_auth/ble/bluetooth_low_energy_connection.h @@ -244,7 +244,7 @@ class BluetoothLowEnergyConnection : public Connection, // The Bluetooth adapter over which the Bluetooth connection will be made. scoped_refptr<device::BluetoothAdapter> adapter_; - // Remote service the |connection_| was established with. + // Remote service the |gatt_connection_| was established with. RemoteAttribute remote_service_; // Characteristic used to send data to the remote device. @@ -254,7 +254,7 @@ class BluetoothLowEnergyConnection : public Connection, RemoteAttribute from_peripheral_char_; // The GATT connection with the remote device. - scoped_ptr<device::BluetoothGattConnection> connection_; + scoped_ptr<device::BluetoothGattConnection> gatt_connection_; // The characteristics finder for remote device. scoped_ptr<BluetoothLowEnergyCharacteristicsFinder> characteristic_finder_; diff --git a/components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc b/components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc index c2e0991..37276e8 100644 --- a/components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc +++ b/components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc @@ -310,14 +310,9 @@ void BluetoothLowEnergyConnectionFinder::CreateGattConnection( void BluetoothLowEnergyConnectionFinder::CloseGattConnection( scoped_ptr<device::BluetoothGattConnection> gatt_connection) { DCHECK(gatt_connection); - std::string device_address = gatt_connection->GetDeviceAddress(); + + // Destroying the BluetoothGattConnection also disconnects it. gatt_connection.reset(); - BluetoothDevice* device = adapter_->GetDevice(device_address); - if (device) { - PA_LOG(INFO) << "Disconnect from device " << device->GetAddress(); - device->Disconnect(base::Bind(&base::DoNothing), - base::Bind(&base::DoNothing)); - } } scoped_ptr<Connection> BluetoothLowEnergyConnectionFinder::CreateConnection( @@ -358,9 +353,8 @@ void BluetoothLowEnergyConnectionFinder::RestartDiscoverySessionWhenReady() { // |device::BluetoothDiscoverySession::Stop()|. // The second condition is satisfied when |OnDiscoveryStopped| is called and // |discovery_session_| is reset. - BluetoothDevice* device = GetDevice(remote_device_.bluetooth_address); - if ((!device || !device->IsConnected()) && !discovery_session_) { - DCHECK(!gatt_connection_); + if ((!gatt_connection_ || !gatt_connection_->IsConnected()) && + !discovery_session_) { connection_.reset(); connected_ = false; StartDiscoverySession(); diff --git a/components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h b/components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h index e639246..7d0310e 100644 --- a/components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h +++ b/components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h @@ -41,10 +41,6 @@ class BluetoothLowEnergyConnectionFinder // Finds a connection to the remote device. Only the first one is functional. void Find(const ConnectionCallback& connection_callback) override; - // Closes the connection and forgets the device. - void CloseGattConnection( - scoped_ptr<device::BluetoothGattConnection> gatt_connection); - // proximity_auth::ConnectionObserver: void OnConnectionStatusChanged(Connection* connection, Connection::Status old_status, @@ -61,6 +57,10 @@ class BluetoothLowEnergyConnectionFinder device::BluetoothDevice* device) override; protected: + // Closes the GATT connection. Virtual for testing. + virtual void CloseGattConnection( + scoped_ptr<device::BluetoothGattConnection> gatt_connection); + // Creates a proximity_auth::Connection based on |gatt_connection|. Exposed // for testing. virtual scoped_ptr<Connection> CreateConnection( diff --git a/components/proximity_auth/ble/bluetooth_low_energy_connection_finder_unittest.cc b/components/proximity_auth/ble/bluetooth_low_energy_connection_finder_unittest.cc index 75981cc..86be60e 100644 --- a/components/proximity_auth/ble/bluetooth_low_energy_connection_finder_unittest.cc +++ b/components/proximity_auth/ble/bluetooth_low_energy_connection_finder_unittest.cc @@ -92,12 +92,21 @@ class MockBluetoothLowEnergyConnectionFinder return connection_alias; } + MOCK_METHOD0(CloseGattConnectionProxy, void(void)); + protected: scoped_ptr<Connection> CreateConnection( scoped_ptr<device::BluetoothGattConnection> gatt_connection) override { return make_scoped_ptr(CreateConnectionProxy()); } + void CloseGattConnection( + scoped_ptr<device::BluetoothGattConnection> gatt_connection) override { + BluetoothLowEnergyConnectionFinder::CloseGattConnection( + gatt_connection.Pass()); + CloseGattConnectionProxy(); + } + private: DISALLOW_COPY_AND_ASSIGN(MockBluetoothLowEnergyConnectionFinder); }; @@ -324,9 +333,7 @@ TEST_F(ProximityAuthBluetoothLowEnergyConnectionFinderTest, run_loop.RunUntilIdle(); // The second device should be forgetten. - EXPECT_CALL(*adapter_, GetDevice(std::string(kOtherBluetoothAddress))) - .WillOnce(Return(&other_device)); - EXPECT_CALL(other_device, Disconnect(_, _)); + EXPECT_CALL(connection_finder, CloseGattConnectionProxy()); other_gatt_connection_callback.Run( make_scoped_ptr(new NiceMock<device::MockBluetoothGattConnection>( kOtherBluetoothAddress))); |