Add RemoteDeviceLoader to create RemoteDevice from CryptAuth data.

Add RemoteDeviceLoader to create RemoteDevice from CryptAuth data.

BUG=527558
TEST=unit test

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

[Tim Song, 2015-09-24 02:33:19]

Patchset #1 (id:1) has been deleted

[Tim Song, 2015-09-24 02:33:29]

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

[sacomoto, 2015-09-24 12:41:59]

LTGM with nits.

This is not for this CL, but every time we add a new field to RemoteDevice we
have to change a bunch of tests, and in most cases we only need to create a
default remote device. 

Maybe we could have a CreateDefaultRemoteDevice() function somewhere, like in a 
proximity_auth_test_util.cc.

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

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device.h:19: std::string sign_in_challenge;
nit: You could already add the flag for BLE devices here.

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

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device_loader.cc:32: if
(!remaining_unlock_keys_.size()) {
nit: remaining_unlock_keys_.empty()

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device_loader.cc:60: PA_LOG(INFO) << "Derived
psk for " << unlock_key.friendly_device_name()
nit: s/psk/PSK/.

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

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device_loader_unittest.cc:97:
EXPECT_FALSE(remote_devices_[0].persistent_symmetric_key.empty());
nit: Can't you compare with an expected value?

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device_loader_unittest.cc:120:
EXPECT_FALSE(remote_devices_[i].persistent_symmetric_key.empty());
nit: ditto

[sacomoto, 2015-09-24 12:42:59]

On 2015/09/24 12:41:59, sacomoto wrote:
> LTGM with nits.
> 
> This is not for this CL, but every time we add a new field to RemoteDevice we
> have to change a bunch of tests, and in most cases we only need to create a
> default remote device. 
> 
> Maybe we could have a CreateDefaultRemoteDevice() function somewhere, like in
a 
> proximity_auth_test_util.cc.
> 
>
https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
> File components/proximity_auth/remote_device.h (right):
> 
>
https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
> components/proximity_auth/remote_device.h:19: std::string sign_in_challenge;
> nit: You could already add the flag for BLE devices here.
> 
>
https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
> File components/proximity_auth/remote_device_loader.cc (right):
> 
>
https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
> components/proximity_auth/remote_device_loader.cc:32: if
> (!remaining_unlock_keys_.size()) {
> nit: remaining_unlock_keys_.empty()
> 
>
https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
> components/proximity_auth/remote_device_loader.cc:60: PA_LOG(INFO) << "Derived
> psk for " << unlock_key.friendly_device_name()
> nit: s/psk/PSK/.
> 
>
https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
> File components/proximity_auth/remote_device_loader_unittest.cc (right):
> 
>
https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
> components/proximity_auth/remote_device_loader_unittest.cc:97:
> EXPECT_FALSE(remote_devices_[0].persistent_symmetric_key.empty());
> nit: Can't you compare with an expected value?
> 
>
https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
> components/proximity_auth/remote_device_loader_unittest.cc:120:
> EXPECT_FALSE(remote_devices_[i].persistent_symmetric_key.empty());
> nit: ditto

Ops, LGTM.

[Tim Song, 2015-09-26 02:08:14]

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

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device.h:19: std::string sign_in_challenge;
On 2015/09/24 12:41:59, sacomoto wrote:
> nit: You could already add the flag for BLE devices here.

Done.

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

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device_loader.cc:32: if
(!remaining_unlock_keys_.size()) {
On 2015/09/24 12:41:59, sacomoto wrote:
> nit: remaining_unlock_keys_.empty()

Done.

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device_loader.cc:60: PA_LOG(INFO) << "Derived
psk for " << unlock_key.friendly_device_name()
On 2015/09/24 12:41:59, sacomoto wrote:
> nit: s/psk/PSK/.

Done.

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

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device_loader_unittest.cc:97:
EXPECT_FALSE(remote_devices_[0].persistent_symmetric_key.empty());
On 2015/09/24 12:41:59, sacomoto wrote:
> nit: Can't you compare with an expected value?

The FakeSecureMessageDelegate would essentially generate some random string,
which I don't think is too meaningful to compare.

https://codereview.chromium.org/1356943004/diff/20001/components/proximity_au...
components/proximity_auth/remote_device_loader_unittest.cc:120:
EXPECT_FALSE(remote_devices_[i].persistent_symmetric_key.empty());
On 2015/09/24 12:41:59, sacomoto wrote:
> nit: ditto

Acknowledged.

[Tim Song, 2015-09-28 17:15:15]

The CQ bit was checked by tengs@chromium.org

[Tim Song, 2015-09-28 17:15:15]

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/1356943004/#ps40001
(title: "move RemoteDevice creation in tests to util function")

[commit-bot: I haz the power, 2015-09-28 17:16:40]

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

[commit-bot: I haz the power, 2015-09-28 19:18:29]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-09-28 19:19:47]

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