Implement finding BLE connections in chrome://proximity-auth.

Implement finding BLE connections in chrome://proximity-auth.

This CL also refactors the classic Bluetooth connection finder to use
SeekDeviceByAddress in order to share the ConnectionFinder interface in
ProximityAuthWebUIHandler.

BUG=517641
TEST=manual and unit tests

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

[Tim Song, 2015-08-07 21:16:51]

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

[Tim Song, 2015-08-07 21:19:38]

Patchset #1 (id:1) has been deleted

[sacomoto, 2015-08-10 19:40:29]

LGTM with nits.

Thanks, for integrating the BLE flow in chrome://proximity-auth.

I should have the iOS crypto done by end of the week, so this would be very
helpful for testing.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc
(right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc:117: 
nit: move it inside the if instead of removing it.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h
(right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h:101: //
Used to invokes |connection_callback_| asynchronously, decoupling the
nit: s/invokes/invoke/

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/bluetooth_connection_finder.cc (right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder.cc:90: // it is not
already known to the adapter.
nit: Maybe add: "This is a faster alternative to
|BluetoothAdapter::StartDiscoverySession|."

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/bluetooth_connection_finder.h (right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder.h:85: // Used to invokes
|connection_callback_| asynchronously, decoupling the
nit: s/invokes/invoke/

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/bluetooth_connection_finder_unittest.cc (right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder_unittest.cc:139:
.WillByDefault(Return(bluetooth_device_.get()));
nit: add a comment that by default the device is know and |SeekByDeviceAddress|
is not called.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder_unittest.cc:231: // be
nit: kill the extra line.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder_unittest.cc:340: // After
seeking is successful, the normal flow should resume.
optional: you could refactor this in a helper method.

[sacomoto, 2015-08-10 19:47:34]

By the way, I think we should avoid duplicate code for timeout (in the TODO) of
the BluetoothConnectionFinder and BluetoothLowEnergyConnectionFinder by having
it on ConnectionFinder.

On 2015/08/10 19:40:29, sacomoto wrote:
> LGTM with nits.
> 
> Thanks, for integrating the BLE flow in chrome://proximity-auth.
> 
> I should have the iOS crypto done by end of the week, so this would be very
> helpful for testing.
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> File components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc
> (right):
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc:117: 
> nit: move it inside the if instead of removing it.
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> File components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h
> (right):
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h:101: //
> Used to invokes |connection_callback_| asynchronously, decoupling the
> nit: s/invokes/invoke/
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> File components/proximity_auth/bluetooth_connection_finder.cc (right):
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> components/proximity_auth/bluetooth_connection_finder.cc:90: // it is not
> already known to the adapter.
> nit: Maybe add: "This is a faster alternative to
> |BluetoothAdapter::StartDiscoverySession|."
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> File components/proximity_auth/bluetooth_connection_finder.h (right):
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> components/proximity_auth/bluetooth_connection_finder.h:85: // Used to invokes
> |connection_callback_| asynchronously, decoupling the
> nit: s/invokes/invoke/
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> File components/proximity_auth/bluetooth_connection_finder_unittest.cc
(right):
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> components/proximity_auth/bluetooth_connection_finder_unittest.cc:139:
> .WillByDefault(Return(bluetooth_device_.get()));
> nit: add a comment that by default the device is know and
|SeekByDeviceAddress|
> is not called.
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> components/proximity_auth/bluetooth_connection_finder_unittest.cc:231: // be
> nit: kill the extra line.
> 
>
https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
> components/proximity_auth/bluetooth_connection_finder_unittest.cc:340: //
After
> seeking is successful, the normal flow should resume.
> optional: you could refactor this in a helper method.

[Tim Song, 2015-08-10 22:17:55]

The normal interface of ConnectionFinder assumes that after Find() is called, it
will continue to find the connection indefinitely. I don't think we should
change it just for the WebUI.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc
(right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/ble/bluetooth_low_energy_connection_finder.cc:117: 
On 2015/08/10 19:40:28, sacomoto wrote:
> nit: move it inside the if instead of removing it.

Done.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h
(right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/ble/bluetooth_low_energy_connection_finder.h:101: //
Used to invokes |connection_callback_| asynchronously, decoupling the
On 2015/08/10 19:40:28, sacomoto wrote:
> nit: s/invokes/invoke/

Done.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/bluetooth_connection_finder.cc (right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder.cc:90: // it is not
already known to the adapter.
On 2015/08/10 19:40:28, sacomoto wrote:
> nit: Maybe add: "This is a faster alternative to
> |BluetoothAdapter::StartDiscoverySession|."

This isn't actually an alternative because the phone isn't making itself
discoverable.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/bluetooth_connection_finder.h (right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder.h:85: // Used to invokes
|connection_callback_| asynchronously, decoupling the
On 2015/08/10 19:40:28, sacomoto wrote:
> nit: s/invokes/invoke/

Done.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
File components/proximity_auth/bluetooth_connection_finder_unittest.cc (right):

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder_unittest.cc:139:
.WillByDefault(Return(bluetooth_device_.get()));
On 2015/08/10 19:40:28, sacomoto wrote:
> nit: add a comment that by default the device is know and
|SeekByDeviceAddress|
> is not called.

Done.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder_unittest.cc:231: // be
On 2015/08/10 19:40:28, sacomoto wrote:
> nit: kill the extra line.

Done.

https://codereview.chromium.org/1277483007/diff/20001/components/proximity_au...
components/proximity_auth/bluetooth_connection_finder_unittest.cc:340: // After
seeking is successful, the normal flow should resume.
On 2015/08/10 19:40:28, sacomoto wrote:
> optional: you could refactor this in a helper method.

Done.

[Tim Song, 2015-08-10 23:25:10]

The CQ bit was checked by tengs@chromium.org

[Tim Song, 2015-08-10 23:25:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
sacomoto@chromium.org
Link to the patchset: https://codereview.chromium.org/1277483007/#ps40001
(title: "refactor unit test")

[commit-bot: I haz the power, 2015-08-10 23:26:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1277483007/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1277483007/40001

[commit-bot: I haz the power, 2015-08-11 00:21:08]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-08-11 00:21:48]

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