[GCM] API update to allow only a single sender in registration

CLOSING the issue. We decided to stick with multiple senders. This CL might come in handy once and if GCM moves toward a registration ID per sender. Given that that decision has not been made, we are not taking this change.

[GCM] API update to allow only a single sender in registration

In anticipation of the coming changes and in the interest of not deprecating the API shortly after it is removed we reduce the number of senders the applicaiton is allowed to register for to 1.

The registrationID is going to be valid for a single sender only.

Relevant updates are propagated everywhere through the code and the tests are updated in the change as well.

In anticipation of allowing multiple senders that do not share registration id, reading and writing the registration ids and senders is updated as well.

BUG=316683

[fgorski, 2014-02-28 18:50:25]

PTAL:

tim (if zea is out) or zea:
gcm/

tim:
invalidations/

jian:
services/gcm + tests + gcm (non-owner)

kalman:
gcm.json

[not at google - send to devlin, 2014-02-28 22:36:48]

json file lgtm

https://codereview.chromium.org/183923006/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/gcm.json (right):

https://codereview.chromium.org/183923006/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/gcm.json:19: "description": "Registers the
application with GCM. The registration ID will be returned by the
<code>callback</code>.",
The comment you removed explained what happened when the call is made multiple
times. Could you add something like that here?

https://codereview.chromium.org/183923006/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/gcm.json:25: "description": "A list of server IDs
that are allowed to send messages to the application. It should contain at least
one and no more than 100 sender IDs."
This description needs an overhaul.

[jianli, 2014-03-03 21:29:30]

https://codereview.chromium.org/183923006/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/gcm/gcm_api.cc (right):

https://codereview.chromium.org/183923006/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/gcm/gcm_api.cc:132: params->sender_id,
Do we want to validate if the sender ID is empty here?

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
File chrome/browser/services/gcm/gcm_client_mock.h (right):

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_client_mock.h:67: static std::string
GetRegistrationIdFromSenderIds(
Remove the trailing "s" from the name.

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
File chrome/browser/services/gcm/gcm_profile_service.cc (right):

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service.cc:999:
registration_info_dict->SetString(kSenderKey,
Do we want to make it be compatible with future multiple registration support?

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
File chrome/browser/services/gcm/gcm_profile_service_unittest.cc (right):

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:774:
consumer()->Register(kTestingAppId, "sedner1@error");
nit: typo for sender

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:781:
TEST_F(GCMProfileServiceSingleProfileTest, RegisterAgainWithSameSenderIDs) {
Remove the trailing "s" from the test case name.

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:802:
RegisterAgainWithDifferentSenderIDs) {
ditto.

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:811: // Make sender
IDs different.
IDs => ID

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/engine/regis...
File google_apis/gcm/engine/registration_request.h (right):

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/engine/regis...
google_apis/gcm/engine/registration_request.h:8: #include <map>
nit: this also seems not to be used at all

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/engine/regis...
google_apis/gcm/engine/registration_request.h:9: #include <vector>
nit: this can be removed

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/gcm_client_i...
File google_apis/gcm/gcm_client_impl_unittest.cc (right):

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl_unittest.cc:367: std::vector<std::string>
senders;
nit: senders should be removed

[fgorski, 2014-03-03 23:14:16]

Jian, Tim, PTAL

https://codereview.chromium.org/183923006/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/gcm/gcm_api.cc (right):

https://codereview.chromium.org/183923006/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/gcm/gcm_api.cc:132: params->sender_id,
On 2014/03/03 21:29:31, jianli wrote:
> Do we want to validate if the sender ID is empty here?

This is already done by the gcm.json, and covered by a test.

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
File chrome/browser/services/gcm/gcm_client_mock.h (right):

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_client_mock.h:67: static std::string
GetRegistrationIdFromSenderIds(
On 2014/03/03 21:29:31, jianli wrote:
> Remove the trailing "s" from the name.

Done.

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
File chrome/browser/services/gcm/gcm_profile_service.cc (right):

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service.cc:999:
registration_info_dict->SetString(kSenderKey,
On 2014/03/03 21:29:31, jianli wrote:
> Do we want to make it be compatible with future multiple registration support?

Done. Modified the write and read code to simply store a dictionary of 
sender_id -> registration_id mappings. The reading will assume multiple entries,
but exit after reading one.

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
File chrome/browser/services/gcm/gcm_profile_service_unittest.cc (right):

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:774:
consumer()->Register(kTestingAppId, "sedner1@error");
On 2014/03/03 21:29:31, jianli wrote:
> nit: typo for sender

Done. Brilliant catch!

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:781:
TEST_F(GCMProfileServiceSingleProfileTest, RegisterAgainWithSameSenderIDs) {
On 2014/03/03 21:29:31, jianli wrote:
> Remove the trailing "s" from the test case name.

Done.

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:802:
RegisterAgainWithDifferentSenderIDs) {
On 2014/03/03 21:29:31, jianli wrote:
> ditto.

Done.

https://codereview.chromium.org/183923006/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:811: // Make sender
IDs different.
On 2014/03/03 21:29:31, jianli wrote:
> IDs => ID

Done.

https://codereview.chromium.org/183923006/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/gcm.json (right):

https://codereview.chromium.org/183923006/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/gcm.json:19: "description": "Registers the
application with GCM. The registration ID will be returned by the
<code>callback</code>.",
On 2014/02/28 22:36:48, kalman wrote:
> The comment you removed explained what happened when the call is made multiple
> times. Could you add something like that here?

Done. I've brought the statement back adjusted for the "arity" of sender Id.
I'll update the documentation further once we land cloudMessagingV2
doc.(together with other updates based on API review)

https://codereview.chromium.org/183923006/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/gcm.json:25: "description": "A list of server IDs
that are allowed to send messages to the application. It should contain at least
one and no more than 100 sender IDs."
On 2014/02/28 22:36:48, kalman wrote:
> This description needs an overhaul.

Done.

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/engine/regis...
File google_apis/gcm/engine/registration_request.h (right):

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/engine/regis...
google_apis/gcm/engine/registration_request.h:8: #include <map>
On 2014/03/03 21:29:31, jianli wrote:
> nit: this also seems not to be used at all

Done.

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/engine/regis...
google_apis/gcm/engine/registration_request.h:9: #include <vector>
On 2014/03/03 21:29:31, jianli wrote:
> nit: this can be removed

Done.

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/gcm_client_i...
File google_apis/gcm/gcm_client_impl_unittest.cc (right):

https://codereview.chromium.org/183923006/diff/1/google_apis/gcm/gcm_client_i...
google_apis/gcm/gcm_client_impl_unittest.cc:367: std::vector<std::string>
senders;
On 2014/03/03 21:29:31, jianli wrote:
> nit: senders should be removed

Done.

[fgorski, 2014-03-04 00:42:10]

rlarocque@chromium.org: Please review changes in invalidations
dimich@chromium.org: Please review changes in google_apis/gcm

Adding alternative reviewers for invalidations and google_apis

Tim, feel free to still do the drive by.

[Dmitry Titov, 2014-03-04 00:51:21]

> dimich@chromium.org: Please review changes in google_apis/gcm

LGTM. gcm/ changes are pretty mechanical here.

[jianli, 2014-03-04 01:13:53]

lgtm

https://codereview.chromium.org/183923006/diff/10001/chrome/browser/services/...
File chrome/browser/services/gcm/gcm_profile_service.cc (right):

https://codereview.chromium.org/183923006/diff/10001/chrome/browser/services/...
chrome/browser/services/gcm/gcm_profile_service.cc:1053: // TODO(fgorski):
Switch the return statements when multiple sendres are
Probably you only need to add one TODO comment at line 1042:
  // TODO(...): Add multi-sender reading support.