Introduce CryptAuthGCMManager, which handles GCM interactions for CryptAuth.

Introduce CryptAuthGCMManager, which handles GCM interactions for CryptAuth.

BUG=420315
TEST=unit test

Committed: https://crrev.com/953ae68f686aa0132cfa35c7f2fca27ea043f73f
Cr-Commit-Position: refs/heads/master@{#338427}

[Tim Song, 2015-07-09 01:53:28]

Patchset #1 (id:1) has been deleted

[Tim Song, 2015-07-09 01:55:09]

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

[Tim Song, 2015-07-09 01:55:09]

+fgorski@ for gcm_driver DEPS
+sacomoto@ for general review

[fgorski, 2015-07-09 18:43:11]

DEPS addition for gcm_driver lgtm

[sacomoto, 2015-07-10 10:54:14]

LTGM with nits.

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
File components/proximity_auth/cryptauth/cryptauth_gcm_manager.h (right):

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
components/proximity_auth/cryptauth/cryptauth_gcm_manager.h:47: // function will
be called with registration completes.
nit: s/with/when/

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
components/proximity_auth/cryptauth/cryptauth_gcm_manager.h:47: // function will
be called with registration completes.
This comment is not very clear. Do you mean the |OnRegistrationCompleted()|
callback declared in the implementation?

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
File components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl.cc (right):

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl.cc:68:
weak_ptr_factory_.GetWeakPtr()));
See comment in cryptauth_gcm_manager_impl_unittest.cc.

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
File components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl_unittest.cc
(right):

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl_unittest.cc:132:
}
The current behavior when |CryptAuthGCMManagerImpl::RegisterWithGCM()| fails,
and there is already an existing registration is to fail anyway. Is this
intended? Or in this case it should report success?

[sacomoto, 2015-07-10 10:54:45]

I mean LGTM.

On 2015/07/10 10:54:14, sacomoto wrote:
> LTGM with nits.
> 
>
https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
> File components/proximity_auth/cryptauth/cryptauth_gcm_manager.h (right):
> 
>
https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
> components/proximity_auth/cryptauth/cryptauth_gcm_manager.h:47: // function
will
> be called with registration completes.
> nit: s/with/when/
> 
>
https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
> components/proximity_auth/cryptauth/cryptauth_gcm_manager.h:47: // function
will
> be called with registration completes.
> This comment is not very clear. Do you mean the |OnRegistrationCompleted()|
> callback declared in the implementation?
> 
>
https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
> File components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl.cc
(right):
> 
>
https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
> components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl.cc:68:
> weak_ptr_factory_.GetWeakPtr()));
> See comment in cryptauth_gcm_manager_impl_unittest.cc.
> 
>
https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
> File
components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
>
components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl_unittest.cc:132:
> }
> The current behavior when |CryptAuthGCMManagerImpl::RegisterWithGCM()| fails,
> and there is already an existing registration is to fail anyway. Is this
> intended? Or in this case it should report success?

[Tim Song, 2015-07-10 20:52:04]

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
File components/proximity_auth/cryptauth/cryptauth_gcm_manager.h (right):

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
components/proximity_auth/cryptauth/cryptauth_gcm_manager.h:47: // function will
be called with registration completes.
On 2015/07/10 10:54:14, sacomoto wrote:
> This comment is not very clear. Do you mean the |OnRegistrationCompleted()|
> callback declared in the implementation?

Oops, I meant the Observer::OnGCMRegistrationResult() function.

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
components/proximity_auth/cryptauth/cryptauth_gcm_manager.h:47: // function will
be called with registration completes.
On 2015/07/10 10:54:14, sacomoto wrote:
> nit: s/with/when/

Done.

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
File components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl.cc (right):

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl.cc:68:
weak_ptr_factory_.GetWeakPtr()));
On 2015/07/10 10:54:14, sacomoto wrote:
> See comment in cryptauth_gcm_manager_impl_unittest.cc.

Acknowledged.

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
File components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl_unittest.cc
(right):

https://codereview.chromium.org/1228763002/diff/20001/components/proximity_au...
components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl_unittest.cc:132:
}
On 2015/07/10 10:54:14, sacomoto wrote:
> The current behavior when |CryptAuthGCMManagerImpl::RegisterWithGCM()| fails,
> and there is already an existing registration is to fail anyway. Is this
> intended? Or in this case it should report success? 

This is the intended result. It should still report failure for the last
registration that was requested.

I added a test case for this situation.

[Tim Song, 2015-07-11 02:15:09]

The CQ bit was checked by tengs@chromium.org

[Tim Song, 2015-07-11 02:15:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
sacomoto@chromium.org, fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/1228763002/#ps40001
(title: "fixes")

[commit-bot: I haz the power, 2015-07-11 02:15:25]

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

[commit-bot: I haz the power, 2015-07-11 02:20:10]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-07-11 02:21:20]

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