[GCM] Only allow the senders that the application is currently registered for

Only allow the senders that the application is currently registered for to send messages to it. 

* GCMProfileService adds a check that the app is currently registered for the sender_id of the sender that sent the message 
* IncomingMessage is extended to add sender_id 
* GCMClientImpl adds the sender_id to the incoming message in the handling code. 
* Adding a test verifying that messages are not delivered when not registered for senders 
* fixing the GCM Profile Service tests that were previously not registering for senders before delivering messages 
* Also: moving the DCHECKS in the GCMClientImpl::HandleIncomingMessage higher up to run them as soon as the checks values are available. 

This CL obsoletes https://codereview.chromium.org/185933003/

BUG=344665
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255483

[fgorski, 2014-03-05 00:41:21]

PTAL (this is almost the same patch remade for multi-sender registration)

[jianli, 2014-03-05 01:33:02]

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

https://codereview.chromium.org/185133008/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service.cc:919: for
(std::vector<std::string>::const_iterator iter =
nit: Please add a comment like:
  // Drop the message that is from unregistered sender.

Also, it seems that you can use std::find() to simplify the code.

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

https://codereview.chromium.org/185133008/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:543: void
RegisterAllSenders(GCMProfileServiceTestConsumer* consumer,
This should be defined in GCMProfileServiceTestConsumer such that you don't need
to pass consumer argument.

https://codereview.chromium.org/185133008/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:1056:
MessageNotReceivedWhenSenderNotRegistered) {
nit: MessageNotReceivedFromNotRegisteredSender

https://codereview.chromium.org/185133008/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:1445: // Signing our
cleared all registrations, so we need to register again.
nit: out

[Dmitry Titov, 2014-03-05 01:48:00]

Nothing to add to Jian's notes, LGTM.

[fgorski, 2014-03-05 19:30:22]

All fixed. PTAL

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

https://codereview.chromium.org/185133008/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service.cc:919: for
(std::vector<std::string>::const_iterator iter =
On 2014/03/05 01:33:02, jianli wrote:
> nit: Please add a comment like:
>   // Drop the message that is from unregistered sender.
> 
> Also, it seems that you can use std::find() to simplify the code.

Done.

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

https://codereview.chromium.org/185133008/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:543: void
RegisterAllSenders(GCMProfileServiceTestConsumer* consumer,
On 2014/03/05 01:33:02, jianli wrote:
> This should be defined in GCMProfileServiceTestConsumer such that you don't
need
> to pass consumer argument.

Done. I extracted string to vector conversion to a different function, as having
a second method didn't look right --- one of them was Waiting UntilCompleted,
and I would have to update the method naming to reflect parameter types.

https://codereview.chromium.org/185133008/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:1056:
MessageNotReceivedWhenSenderNotRegistered) {
On 2014/03/05 01:33:02, jianli wrote:
> nit: MessageNotReceivedFromNotRegisteredSender

Done.

https://codereview.chromium.org/185133008/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service_unittest.cc:1445: // Signing our
cleared all registrations, so we need to register again.
On 2014/03/05 01:33:02, jianli wrote:
> nit: out

Done.

[jianli, 2014-03-05 20:46:30]

lgtm

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

https://codereview.chromium.org/185133008/diff/10001/chrome/browser/services/...
chrome/browser/services/gcm/gcm_profile_service.cc:917: // Dropping the message
when application does not have a registration entry
nit: move the comment above the previous line.

[fgorski, 2014-03-05 20:53:19]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-03-05 20:55:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fgorski@chromium.org/185133008/30001

[commit-bot: I haz the power, 2014-03-05 21:09:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-05 21:09:02]

Retried try job too often on win_rel for step(s) app_list_unittests,
ash_unittests, aura_unittests, base_unittests, browser_tests,
cacheinvalidation_unittests, cc_unittests, check_deps, check_deps2git,
chrome_elf_unittests, chromedriver_unittests, components_unittests,
compositor_unittests, content_browsertests, content_unittests, crypto_unittests,
events_unittests, gfx_unittests, google_apis_unittests, gpu_unittests,
installer_util_unittests, interactive_ui_tests, ipc_tests, jingle_unittests,
media_unittests, mini_installer_test, nacl_integration, net_unittests,
ppapi_unittests, printing_unittests, remoting_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests, telemetry_perf_unittests,
telemetry_unittests, unit_tests, views_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[fgorski, 2014-03-05 22:42:39]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-03-05 22:44:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fgorski@chromium.org/185133008/30001

[commit-bot: I haz the power, 2014-03-05 23:38:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-05 23:38:03]

Retried try job too often on win_rel for step(s) net_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[fgorski, 2014-03-06 00:00:24]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-03-06 00:23:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fgorski@chromium.org/185133008/30001

[commit-bot: I haz the power, 2014-03-06 00:49:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 00:49:28]

Try jobs failed on following builders: mac_chromium_rel

[fgorski, 2014-03-06 17:14:19]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-03-06 17:14:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fgorski@chromium.org/185133008/30001

[commit-bot: I haz the power, 2014-03-06 19:07:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 19:07:58]

Retried try job too often on win_rel for step(s) browser_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[fgorski, 2014-03-06 19:10:53]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-03-06 19:14:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fgorski@chromium.org/185133008/30001

[commit-bot: I haz the power, 2014-03-06 20:59:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fgorski@chromium.org/185133008/30001

[commit-bot: I haz the power, 2014-03-06 21:53:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fgorski@chromium.org/185133008/30001

[commit-bot: I haz the power, 2014-03-07 00:58:42]

Change committed as 255483