Adding unit tests for BLE connection.

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=

Committed: https://crrev.com/e5da7f28b633b5f05c3b99aa019d7da6b4ff1a0b
Cr-Commit-Position: refs/heads/master@{#333051}

[sacomoto, 2015-06-04 17:47:30]

sacomoto@chromium.org changed reviewers:
+ msarda@chromium.org, tengs@chromium.org

[sacomoto, 2015-06-04 17:49:01]

PTAL.

[msarda, 2015-06-05 09:40:19]

LGTM with some nits.

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
File components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc
(right):

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc:77: 
// Exposing inherited protected methods for testing.

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc:79: 
// Exposing inherited protected fields for testing.

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc:89:
ProximityAuthBluetoothLowEnergyConnectionTest()
This may be a personal preference, but I prefer having test SetUp and TearDown
do the setup required for the test. Could you move part of this to a SetUp
function?

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc:300:
device::MockBluetoothGattConnection* gatt_connection_alias_;
I think this is useless (only used twice). Please remove it and replace its
usage with gatt_connection_.get().

[sacomoto, 2015-06-05 11:04:04]

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
File components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc
(right):

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc:77: 
On 2015/06/05 09:40:19, msarda wrote:
> // Exposing inherited protected methods for testing.

Done.

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc:79: 
On 2015/06/05 09:40:19, msarda wrote:
> // Exposing inherited protected fields for testing.

Done.

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc:89:
ProximityAuthBluetoothLowEnergyConnectionTest()
On 2015/06/05 09:40:19, msarda wrote:
> This may be a personal preference, but I prefer having test SetUp and TearDown
> do the setup required for the test. Could you move part of this to a SetUp
> function?

Done.

https://codereview.chromium.org/1144333007/diff/1/components/proximity_auth/b...
components/proximity_auth/ble/bluetooth_low_energy_connection_unittest.cc:300:
device::MockBluetoothGattConnection* gatt_connection_alias_;
On 2015/06/05 09:40:19, msarda wrote:
> I think this is useless (only used twice). Please remove it and replace its
> usage with gatt_connection_.get().

We cannot remove it. We pass the ownership of |gatt_connection_| when creating
the BluetoothLowEnergyConnection object in CreateConnection(). But then we need
it again in ConnectWithoutExistingGattConnection() and |gatt_connection_| is
already NULL.

[sacomoto, 2015-06-05 14:55:04]

The CQ bit was checked by sacomoto@chromium.org

[sacomoto, 2015-06-05 14:55:04]

The patchset sent to the CQ was uploaded after l-g-t-m from msarda@chromium.org
Link to the patchset: https://codereview.chromium.org/1144333007/#ps60001
(title: "Fixing compilation on OS X and Windows 2")

[commit-bot: I haz the power, 2015-06-05 14:55:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1144333007/60001

[commit-bot: I haz the power, 2015-06-05 15:37:30]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-06-05 15:38:38]

Patchset 4 (id:??) landed as
https://crrev.com/e5da7f28b633b5f05c3b99aa019d7da6b4ff1a0b
Cr-Commit-Position: refs/heads/master@{#333051}