Adding unit tests for BLE connection.

In order making testing more convenient, this CL does the following
refactoring:

- Creates a new virtual CreateCharacteristicsFinder in
BluetoothLowEnergyConnection;
- Starts the connection on BluetoothLowEnergyConnection after a call to
Connect() (this was already suggested by tengs@ in a previous CL);
- Creates a new protected constructor in BluetoothCharacteristicsFinder;
- Mocks for BluetoothCharacteristicsFinder and
BluetoothLowEnergyConnection;

The unit tests covers the Connect() and Disconnect() execution paths. A
second part will cover receiving bytes and Send().

This CL also fixes some small bugs on BluetoothLowEnergyConnection.

BUG=

Review URL: https://codereview.chromium.org/1144333007

Cr-Commit-Position: refs/heads/master@{#333051}

components/components_tests.gyp

@@ -435,6 +435,7 @@
     'proximity_auth_unittest_sources': [
       'proximity_auth/ble/bluetooth_low_energy_characteristics_finder_unittest.cc',
       'proximity_auth/ble/bluetooth_low_energy_connection_finder_unittest.cc',
+      'proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc',
       'proximity_auth/ble/proximity_auth_ble_system_unittest.cc',
       'proximity_auth/bluetooth_connection_finder_unittest.cc',
       'proximity_auth/bluetooth_connection_unittest.cc',

components/proximity_auth/ble/BUILD.gn

@@ -32,6 +32,7 @@ source_set("unit_tests") {
   sources = [
     "bluetooth_low_energy_characteristics_finder_unittest.cc",
     "bluetooth_low_energy_connection_finder_unittest.cc",
+    "bluetooth_low_energy_connection_unittest.cc",
     "proximity_auth_ble_system_unittest.cc",
   ]
 

components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.cc

@@ -44,6 +44,10 @@ BluetoothLowEnergyCharacteristicsFinder::
   // TODO(sacomoto): implement a timeout for characteristic discovery.
 }
 
+BluetoothLowEnergyCharacteristicsFinder::
+    BluetoothLowEnergyCharacteristicsFinder() {
+}
+
 BluetoothLowEnergyCharacteristicsFinder::
     ~BluetoothLowEnergyCharacteristicsFinder() {
   ResetCallbacks();

components/proximity_auth/ble/bluetooth_low_energy_characteristics_finder.h

@@ -68,6 +68,9 @@ class BluetoothLowEnergyCharacteristicsFinder
       device::BluetoothAdapter* adapter,
       device::BluetoothGattCharacteristic* characteristic) override;
 
+  // For testing. Used to mock this class.
+  BluetoothLowEnergyCharacteristicsFinder();
+
  private:
   // Handles the discovery of a new characteristic.
   void HandleCharacteristicUpdate(

components/proximity_auth/ble/bluetooth_low_energy_connection.cc

@@ -45,6 +45,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()),
       sub_status_(SubStatus::DISCONNECTED),
       receiving_bytes_(false),
       write_remote_characteristic_pending_(false),
@@ -55,9 +56,6 @@ BluetoothLowEnergyConnection::BluetoothLowEnergyConnection(
 
   start_time_ = base::TimeTicks::Now();
   adapter_->AddObserver(this);
-
-  if (gatt_connection && gatt_connection->IsConnected())
-    OnGattConnectionCreated(gatt_connection.Pass());
 }
 
 BluetoothLowEnergyConnection::~BluetoothLowEnergyConnection() {
@@ -69,6 +67,11 @@ BluetoothLowEnergyConnection::~BluetoothLowEnergyConnection() {
 }
 
 void BluetoothLowEnergyConnection::Connect() {
+  if (connection_ && connection_->IsConnected()) {
+    OnGattConnectionCreated(connection_.Pass());
+    return;
+  }
+
   BluetoothDevice* remote_device = GetRemoteDevice();
   if (remote_device) {
     SetSubStatus(SubStatus::WAITING_GATT_CONNECTION);
@@ -84,15 +87,17 @@ void BluetoothLowEnergyConnection::Connect() {
 // connect to BLE devices advertising the SmartLock service (assuming this
 // device has no other connection).
 void BluetoothLowEnergyConnection::Disconnect() {
-  ClearWriteRequestsQueue();
-  StopNotifySession();
-  SetSubStatus(SubStatus::DISCONNECTED);
-  if (connection_) {
-    connection_.reset();
-    BluetoothDevice* device = GetRemoteDevice();
-    if (device) {
-      VLOG(1) << "Forget device " << device->GetAddress();
-      device->Forget(base::Bind(&base::DoNothing));
+  if (sub_status_ != SubStatus::DISCONNECTED) {
+    ClearWriteRequestsQueue();
+    StopNotifySession();
+    SetSubStatus(SubStatus::DISCONNECTED);
+    if (connection_) {
+      connection_.reset();
+      BluetoothDevice* device = GetRemoteDevice();
+      if (device) {
+        VLOG(1) << "Forget device " << device->GetAddress();
+        device->Forget(base::Bind(&base::DoNothing));
+      }
     }
   }
 }
@@ -213,16 +218,22 @@ void BluetoothLowEnergyConnection::OnGattConnectionCreated(
     scoped_ptr<device::BluetoothGattConnection> gatt_connection) {
   connection_ = gatt_connection.Pass();
   SetSubStatus(SubStatus::WAITING_CHARACTERISTICS);
+  characteristic_finder_.reset(CreateCharacteristicsFinder(
+      base::Bind(&BluetoothLowEnergyConnection::OnCharacteristicsFound,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&BluetoothLowEnergyConnection::OnCharacteristicsFinderError,
+                 weak_ptr_factory_.GetWeakPtr())));
+}
 
-  characteristic_finder_ =
-      make_scoped_ptr(new BluetoothLowEnergyCharacteristicsFinder(
-          adapter_, GetRemoteDevice(), remote_service_, to_peripheral_char_,
-          from_peripheral_char_,
-          base::Bind(&BluetoothLowEnergyConnection::OnCharacteristicsFound,
-                     weak_ptr_factory_.GetWeakPtr()),
-          base::Bind(
-              &BluetoothLowEnergyConnection::OnCharacteristicsFinderError,
-              weak_ptr_factory_.GetWeakPtr())));
+BluetoothLowEnergyCharacteristicsFinder*
+BluetoothLowEnergyConnection::CreateCharacteristicsFinder(
+    const BluetoothLowEnergyCharacteristicsFinder::SuccessCallback&
+        success_callback,
+    const BluetoothLowEnergyCharacteristicsFinder::ErrorCallback&
+        error_callback) {
+  return new BluetoothLowEnergyCharacteristicsFinder(
+      adapter_, GetRemoteDevice(), remote_service_, to_peripheral_char_,
+      from_peripheral_char_, success_callback, error_callback);
 }
 
 void BluetoothLowEnergyConnection::OnCharacteristicsFound(
@@ -355,7 +366,7 @@ void BluetoothLowEnergyConnection::OnWriteRemoteCharacteristicError(
 
   // Increases the number of failed attempts and retry.
   DCHECK(!write_requests_queue_.empty());
-  if (write_requests_queue_.front().number_of_failed_attempts++ >=
+  if (++write_requests_queue_.front().number_of_failed_attempts >=
       max_number_of_write_attempts_) {
     Disconnect();
     return;

components/proximity_auth/ble/bluetooth_low_energy_connection.h

@@ -59,11 +59,25 @@ class BluetoothLowEnergyConnection : public Connection,
     kDisconnectSignal = 3,
   };
 
+  // The sub-state of a proximity_auth::BluetoothLowEnergyConnection class
+  // extends the IN_PROGRESS state of proximity_auth::Connection::Status.
+  enum class SubStatus {
+    DISCONNECTED,
+    WAITING_GATT_CONNECTION,
+    GATT_CONNECTION_ESTABLISHED,
+    WAITING_CHARACTERISTICS,
+    CHARACTERISTICS_FOUND,
+    WAITING_NOTIFY_SESSION,
+    NOTIFY_SESSION_READY,
+    WAITING_RESPONSE_SIGNAL,
+    CONNECTED,
+  };
+
   // Constructs a Bluetooth low energy connection to the service with
   // |remote_service_| on the |remote_device|. The |adapter| must be already
   // initaalized and ready. The GATT connection may alreaady be established and
-  // pass through |gatt_connection|. If |gatt_connection| is NULL, a subsequent
-  // call to Connect() must be made.
+  // pass through |gatt_connection|. A subsequent call to Connect() must be
+  // made.
   BluetoothLowEnergyConnection(
       const RemoteDevice& remote_device,
       scoped_refptr<device::BluetoothAdapter> adapter,
@@ -80,23 +94,17 @@ class BluetoothLowEnergyConnection : public Connection,
   void Disconnect() override;
 
  protected:
-  // The sub-state of a proximity_auth::BluetoothLowEnergyConnection class
-  // extends the IN_PROGRESS state of proximity_auth::Connection::Status.
-  enum class SubStatus {
-    DISCONNECTED,
-    WAITING_GATT_CONNECTION,
-    GATT_CONNECTION_ESTABLISHED,
-    WAITING_CHARACTERISTICS,
-    CHARACTERISTICS_FOUND,
-    WAITING_NOTIFY_SESSION,
-    NOTIFY_SESSION_READY,
-    WAITING_RESPONSE_SIGNAL,
-    CONNECTED,
-  };
-
+  // Exposed for testing.
   void SetSubStatus(SubStatus status);
   SubStatus sub_status() { return sub_status_; }
 
+  // Virtual for testing.
+  virtual BluetoothLowEnergyCharacteristicsFinder* CreateCharacteristicsFinder(
+      const BluetoothLowEnergyCharacteristicsFinder::SuccessCallback&
+          success_callback,
+      const BluetoothLowEnergyCharacteristicsFinder::ErrorCallback&
+          error_callback);
+
   // proximity_auth::Connection
   void SendMessageImpl(scoped_ptr<WireMessage> message) override;
   scoped_ptr<WireMessage> DeserializeWireMessage(

components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc

@@ -223,6 +223,7 @@ void BluetoothLowEnergyConnectionFinder::OnGattConnectionCreated(
 
   connection_ = CreateConnection(gatt_connection.Pass());
   connection_->AddObserver(this);
+  connection_->Connect();
 
   StopDiscoverySession();
 }