Add support to GCMKeyStore for multiple keys per app_id

Add support to GCMKeyStore for multiple keys per app_id

Specifically, allow one key per (app_id, authorized_entity) pair. This
will only be used for Instance ID tokens; legacy GCM registrations will
store an empty string for the authorized_entity (rather than storing the
sender ID) for backwards compatibility. The clunky name
instance_id_authorized_entity is designed to remind people of this.

Part of a series of patches:
1. https://codereview.chromium.org/1832833002 adds InstanceIDWithSubtype
2. https://codereview.chromium.org/1830983002 adds JNI bindings
3. https://codereview.chromium.org/1829023002 adds fake and test
4. https://codereview.chromium.org/1899753002 fixes strict mode violations
5. https://codereview.chromium.org/1854093002 enables InstanceID by default
6. this patch
7. https://codereview.chromium.org/1923953002 integrates IIDs with crypto
8. https://codereview.chromium.org/1851423003 switches Push to InstanceIDs

Also depends on https://codereview.chromium.org/1948133003

BUG=589461
Committed: https://crrev.com/36ae5806242390404f2492c8a5c39041cd374fd6
Cr-Commit-Position: refs/heads/master@{#392617}

[johnme, 2016-05-06 12:23:15]

johnme@chromium.org changed reviewers:
+ peter@chromium.org

[johnme, 2016-05-06 12:23:16]

Forked off from https://codereview.chromium.org/1923953002/#msg5

[johnme, 2016-05-09 13:13:33]

Description was changed from

==========
Add support to GCMKeyStore for multiple keys per app_id

Specifically, allow one key per (app_id, authorized_entity) pair. This
will only be used for Instance ID tokens; legacy GCM registrations will
store an empty string for the authorized_entity (rather than storing the
sender ID), in order to avoid conflicting with keys stored for Instance
IDs. The clunky name instance_id_authorized_entity is designed to remind
people of this.

Part of a series of patches:
1. https://codereview.chromium.org/1832833002 adds InstanceIDWithSubtype
2. https://codereview.chromium.org/1830983002 adds JNI bindings
3. https://codereview.chromium.org/1829023002 adds fake and test
4. https://codereview.chromium.org/1899753002 fixes strict mode violations
5. https://codereview.chromium.org/1854093002 enables InstanceID by default
6. this patch
7. https://codereview.chromium.org/1923953002 integrates IIDs with crypto
8. https://codereview.chromium.org/1851423003 switches Push to InstanceIDs

Also depends on https://codereview.chromium.org/1948133003

BUG=589461
==========

to

==========
Add support to GCMKeyStore for multiple keys per app_id

Specifically, allow one key per (app_id, authorized_entity) pair. This
will only be used for Instance ID tokens; legacy GCM registrations will
store an empty string for the authorized_entity (rather than storing the
sender ID) for backwards compatibility. The clunky name
instance_id_authorized_entity is designed to remind people of this.

Part of a series of patches:
1. https://codereview.chromium.org/1832833002 adds InstanceIDWithSubtype
2. https://codereview.chromium.org/1830983002 adds JNI bindings
3. https://codereview.chromium.org/1829023002 adds fake and test
4. https://codereview.chromium.org/1899753002 fixes strict mode violations
5. https://codereview.chromium.org/1854093002 enables InstanceID by default
6. this patch
7. https://codereview.chromium.org/1923953002 integrates IIDs with crypto
8. https://codereview.chromium.org/1851423003 switches Push to InstanceIDs

Also depends on https://codereview.chromium.org/1948133003

BUG=589461
==========

[Peter Beverloo, 2016-05-09 14:10:10]

Thank you John! The code mostly lg with a few questions and suggestions. I like
the DCHECK test, too.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_encryption_provider.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider.cc:163: // (key store owner
= app_id + ',' + sender_id) or a legacy GCM registration
Mentioning the key store owner and its format has no added value here; also
please try to avoid mentioning "we". What about:

  // The message that is to be decrypted could be associated with either a v3
  // or v4 (InstanceID) registration, which store encryption keys differently,
  // but the used version is not known at this point.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_encryption_provider.h (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider.h:87: //                   
              or "" for legacy GCM registrations.
nit: I would like to find a better way of phrasing for "legacy GCM
registrations" throughout this CL, because it is a concept that will continue to
be supported and we have no concrete plans to deprecate.

Sprinkling the term "legacy" over the code-base makes it feel dated,
unmaintained, for no good reason. We can do better :-).

A few ideas:
- "non-InstanceID registrations"
- "v3 registrations" (without acknowledging the server-side Web Push exceptions)

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider.h:87: //                   
              or "" for legacy GCM registrations.
nit: Here and elsewhere, please be consistent with the existing code in how you
document the new parameter— convention in the component is to describe them
using a brief sentence.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider.h:89: const std::string&
instance_id_authorized_entity,
nit: s/instance_id_authorized_entity/authorized_entity/g

The latter is an established concept that is understood in scope of this
component. There's no need for the additional verbosity, and this would reduce
wrapping in a number of places.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:265:
TEST_F(GCMEncryptionProviderTest, VerifiesKeyRemovalGCMRegistration) {
While I'm all for tests, this doesn't actually test the logic in the
GCMEncryptionProvider and has a significant amount of overlap with
GCMKeyStoreTest.RemoveKeysWildcardAuthorizedEntity. Could we merge?

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:283: // Should
get encryption info created above (although these calls are async
Instead of the comment in parenthesis, can we just place a RunUntilIdle() in
between for clarity?

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:298:
EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_gcm, true));
This should be an ASSERT_NO_FATAL_FAILURE. There is little point in continuing
if any of the asserts in the CheckHasKey() methods failed.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store.cc:39: const char
kDatabaseUMAClientName[] = "GCMKeyStore";
Please move this (and kAuthSecretBytes) to the top of the anonymous namespace.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store.h (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store.h:135:
std::unordered_map<std::string,
Why can't we use a multimap from |app_id| to a tuple of (|authorized_entity|,
|key_pair|, |auth_secret|)?

It seems to me like that could help remove the outer_iter/inner_iter code from
the GetKeys() and RemoveKeys() paths. This operation is so infrequent, and
operates on (generally) such small amounts of data, that losing the O(1)ish
performance of the unordered_maps doesn't really matter here, and we can
optimize for maintainability instead.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store_unittest.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store_unittest.cc:57: if (pair_out)
nit: Could the new tests just pass dummy strings, avoiding these clauses?

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/proto/gcm_encryption_data.proto (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/proto/gcm_encryption_data.proto:41: // Next tag: 4
5

[johnme, 2016-05-09 18:15:55]

Addressed review comments - thanks!

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_encryption_provider.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider.cc:163: // (key store owner
= app_id + ',' + sender_id) or a legacy GCM registration
On 2016/05/09 14:10:09, Peter Beverloo wrote:
> Mentioning the key store owner and its format has no added value here; also
> please try to avoid mentioning "we". What about:
> 
>   // The message that is to be decrypted could be associated with either a v3
>   // or v4 (InstanceID) registration, which store encryption keys differently,
>   // but the used version is not known at this point.

I changed this to: "
  // Use |fallback_to_empty_authorized_entity|, since this message might have
  // been sent to either an InstanceID token or a non-InstanceID registration."

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_encryption_provider.h (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider.h:87: //                   
              or "" for legacy GCM registrations.
On 2016/05/09 14:10:09, Peter Beverloo wrote:
> nit: I would like to find a better way of phrasing for "legacy GCM
> registrations" throughout this CL, because it is a concept that will continue
to
> be supported and we have no concrete plans to deprecate.
> 
> Sprinkling the term "legacy" over the code-base makes it feel dated,
> unmaintained, for no good reason. We can do better :-).
> 
> A few ideas:
> - "non-InstanceID registrations"
> - "v3 registrations" (without acknowledging the server-side Web Push
exceptions)

Done ("non-InstanceID", since the v3/v4 split requires historical context that's
not documented in the code).

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider.h:87: //                   
              or "" for legacy GCM registrations.
On 2016/05/09 14:10:09, Peter Beverloo wrote:
> nit: Here and elsewhere, please be consistent with the existing code in how
you
> document the new parameter— convention in the component is to describe them
> using a brief sentence.

Done.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider.h:89: const std::string&
instance_id_authorized_entity,
On 2016/05/09 14:10:09, Peter Beverloo wrote:
> nit: s/instance_id_authorized_entity/authorized_entity/g
> 
> The latter is an established concept that is understood in scope of this
> component. There's no need for the additional verbosity, and this would reduce
> wrapping in a number of places.

Done (thank goodness for git cl format!).

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:265:
TEST_F(GCMEncryptionProviderTest, VerifiesKeyRemovalGCMRegistration) {
On 2016/05/09 14:10:10, Peter Beverloo wrote:
> While I'm all for tests, this doesn't actually test the logic in the
> GCMEncryptionProvider and has a significant amount of overlap with
> GCMKeyStoreTest.RemoveKeysWildcardAuthorizedEntity. Could we merge?

This (and its twin, VerifiesKeyRemovalInstanceIDToken) is the only test coverage
of GCMEncryptionProvider::RemoveEncryptionInfo, which is the public API to the
crypto component. It's also the only test coverage of calling GetEncryptionInfo
for a key that already exists (as opposed to calling it in order to create a new
key). So it seems worth keeping, though I agree that the duplication is
unfortunate.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:283: // Should
get encryption info created above (although these calls are async
On 2016/05/09 14:10:09, Peter Beverloo wrote:
> Instead of the comment in parenthesis, can we just place a RunUntilIdle() in
> between for clarity?

Sure; instead I expanded GCMKeyStoreTest.CreateGetAndRemoveKeysSynchronously to
cover synchronous gets.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:298:
EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_gcm, true));
On 2016/05/09 14:10:10, Peter Beverloo wrote:
> This should be an ASSERT_NO_FATAL_FAILURE. There is little point in continuing
> if any of the asserts in the CheckHasKey() methods failed.

Done (ditto below, since it's always the first failure that matters here).

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store.cc:39: const char
kDatabaseUMAClientName[] = "GCMKeyStore";
On 2016/05/09 14:10:10, Peter Beverloo wrote:
> Please move this (and kAuthSecretBytes) to the top of the anonymous namespace.

Done.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store.h (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store.h:135:
std::unordered_map<std::string,
On 2016/05/09 14:10:10, Peter Beverloo wrote:
> Why can't we use a multimap from |app_id| to a tuple of (|authorized_entity|,
> |key_pair|, |auth_secret|)?
> 
> It seems to me like that could help remove the outer_iter/inner_iter code from
> the GetKeys() and RemoveKeys() paths. This operation is so infrequent, and
> operates on (generally) such small amounts of data, that losing the O(1)ish
> performance of the unordered_maps doesn't really matter here, and we can
> optimize for maintainability instead.

I considered that, but I'm not convinced a multimap would be more maintainable:

- GetKeysAfterInitialize would be at least as complex, since it would need to
loop though the equal_range of keys, then loop through a second time for
fallback (whereas fallback is currently a one-liner).

- CreateKeysAfterInitialize would be more complex, since it would no longer be
possible to express conditions like !key_data_[app_id].count(authorized_entity)
in the DCHECK (except perhaps using lambda functions, but that wouldn't improve
legibility).

- RemoveKeysAfterInitialize would be about equally complex (looping through
equal_range instead of looping through inner map).

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store_unittest.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store_unittest.cc:57: if (pair_out)
On 2016/05/09 14:10:10, Peter Beverloo wrote:
> nit: Could the new tests just pass dummy strings, avoiding these clauses?

Done.

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/proto/gcm_encryption_data.proto (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/proto/gcm_encryption_data.proto:41: // Next tag: 4
On 2016/05/09 14:10:10, Peter Beverloo wrote:
> 5

Done.

[Peter Beverloo, 2016-05-10 12:38:35]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-10 12:38:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1953273002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1953273002/60001

[wildcur20, 2016-05-10 12:43:31]

wildcur20@gmail.com changed reviewers:
+ wildcur20@gmail.com

[johnme, 2016-05-10 12:49:11]

johnme@chromium.org changed reviewers:
+ rch@chromium.org

[johnme, 2016-05-10 12:49:11]

rch@chromium.org: Please review GN/GYP changes in net/ that move the files
required for EXPECT_DFATAL to net test_support, so they can be depended on more
cleanly.

[Peter Beverloo, 2016-05-10 12:52:39]

lgtm

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store_unittest.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store_unittest.cc:57: if (pair_out)
On 2016/05/09 18:15:55, johnme wrote:
> On 2016/05/09 14:10:10, Peter Beverloo wrote:
> > nit: Could the new tests just pass dummy strings, avoiding these clauses?
> 
> Done.

No? The statements + calls w/ nullptr still exist.

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc (right):

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:70: if
(p256dh_out)
no need for the if-statements - all callers pass values

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:81: if
(pair_out)
dito

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store.cc (right):

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store.cc:82: auto outer_iter =
key_data_.find(app_id);
const&

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store.cc:151:
encryption_data.set_authorized_entity(authorized_entity);
nit: no curly braces

[commit-bot: I haz the power, 2016-05-10 13:15:47]

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

[commit-bot: I haz the power, 2016-05-10 13:15:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ryan Hamilton, 2016-05-10 13:20:03]

net/ lgtm

[johnme, 2016-05-10 13:24:45]

Addressed nits - thanks!

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store_unittest.cc (right):

https://codereview.chromium.org/1953273002/diff/40001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store_unittest.cc:57: if (pair_out)
On 2016/05/10 12:52:38, Peter Beverloo wrote:
> On 2016/05/09 18:15:55, johnme wrote:
> > On 2016/05/09 14:10:10, Peter Beverloo wrote:
> > > nit: Could the new tests just pass dummy strings, avoiding these clauses?
> > 
> > Done.
> 
> No? The statements + calls w/ nullptr still exist.

Done (weird, I swear I typed this into my editor, but the patch only contains
the dummy strings in CannotShareAppIdFromInstanceIDToGCM, not my edits to
CannotShareAppIdFromGCMToInstanceID and GotKeys).

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc (right):

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:70: if
(p256dh_out)
On 2016/05/10 12:52:38, Peter Beverloo wrote:
> no need for the if-statements - all callers pass values

Done.

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_encryption_provider_unittest.cc:81: if
(pair_out)
On 2016/05/10 12:52:38, Peter Beverloo wrote:
> dito

Done.

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
File components/gcm_driver/crypto/gcm_key_store.cc (right):

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store.cc:82: auto outer_iter =
key_data_.find(app_id);
On 2016/05/10 12:52:38, Peter Beverloo wrote:
> const&

No point, since find returns an iterator by value.

https://codereview.chromium.org/1953273002/diff/60001/components/gcm_driver/c...
components/gcm_driver/crypto/gcm_key_store.cc:151:
encryption_data.set_authorized_entity(authorized_entity);
On 2016/05/10 12:52:39, Peter Beverloo wrote:
> nit: no curly braces

Done.

[johnme, 2016-05-10 14:31:24]

The CQ bit was checked by johnme@chromium.org

[johnme, 2016-05-10 14:31:24]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1953273002/#ps100001
(title: "Only EXPECT_DFATAL when LOG_DCHECK == LOG_DFATAL")

[commit-bot: I haz the power, 2016-05-10 14:31:34]

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

[commit-bot: I haz the power, 2016-05-10 15:46:38]

Description was changed from

==========
Add support to GCMKeyStore for multiple keys per app_id

Specifically, allow one key per (app_id, authorized_entity) pair. This
will only be used for Instance ID tokens; legacy GCM registrations will
store an empty string for the authorized_entity (rather than storing the
sender ID) for backwards compatibility. The clunky name
instance_id_authorized_entity is designed to remind people of this.

Part of a series of patches:
1. https://codereview.chromium.org/1832833002 adds InstanceIDWithSubtype
2. https://codereview.chromium.org/1830983002 adds JNI bindings
3. https://codereview.chromium.org/1829023002 adds fake and test
4. https://codereview.chromium.org/1899753002 fixes strict mode violations
5. https://codereview.chromium.org/1854093002 enables InstanceID by default
6. this patch
7. https://codereview.chromium.org/1923953002 integrates IIDs with crypto
8. https://codereview.chromium.org/1851423003 switches Push to InstanceIDs

Also depends on https://codereview.chromium.org/1948133003

BUG=589461
==========

to

==========
Add support to GCMKeyStore for multiple keys per app_id

Specifically, allow one key per (app_id, authorized_entity) pair. This
will only be used for Instance ID tokens; legacy GCM registrations will
store an empty string for the authorized_entity (rather than storing the
sender ID) for backwards compatibility. The clunky name
instance_id_authorized_entity is designed to remind people of this.

Part of a series of patches:
1. https://codereview.chromium.org/1832833002 adds InstanceIDWithSubtype
2. https://codereview.chromium.org/1830983002 adds JNI bindings
3. https://codereview.chromium.org/1829023002 adds fake and test
4. https://codereview.chromium.org/1899753002 fixes strict mode violations
5. https://codereview.chromium.org/1854093002 enables InstanceID by default
6. this patch
7. https://codereview.chromium.org/1923953002 integrates IIDs with crypto
8. https://codereview.chromium.org/1851423003 switches Push to InstanceIDs

Also depends on https://codereview.chromium.org/1948133003

BUG=589461
==========

[commit-bot: I haz the power, 2016-05-10 15:46:39]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-10 15:47:47]

Description was changed from

==========
Add support to GCMKeyStore for multiple keys per app_id

Specifically, allow one key per (app_id, authorized_entity) pair. This
will only be used for Instance ID tokens; legacy GCM registrations will
store an empty string for the authorized_entity (rather than storing the
sender ID) for backwards compatibility. The clunky name
instance_id_authorized_entity is designed to remind people of this.

Part of a series of patches:
1. https://codereview.chromium.org/1832833002 adds InstanceIDWithSubtype
2. https://codereview.chromium.org/1830983002 adds JNI bindings
3. https://codereview.chromium.org/1829023002 adds fake and test
4. https://codereview.chromium.org/1899753002 fixes strict mode violations
5. https://codereview.chromium.org/1854093002 enables InstanceID by default
6. this patch
7. https://codereview.chromium.org/1923953002 integrates IIDs with crypto
8. https://codereview.chromium.org/1851423003 switches Push to InstanceIDs

Also depends on https://codereview.chromium.org/1948133003

BUG=589461
==========

to

==========
Add support to GCMKeyStore for multiple keys per app_id

Specifically, allow one key per (app_id, authorized_entity) pair. This
will only be used for Instance ID tokens; legacy GCM registrations will
store an empty string for the authorized_entity (rather than storing the
sender ID) for backwards compatibility. The clunky name
instance_id_authorized_entity is designed to remind people of this.

Part of a series of patches:
1. https://codereview.chromium.org/1832833002 adds InstanceIDWithSubtype
2. https://codereview.chromium.org/1830983002 adds JNI bindings
3. https://codereview.chromium.org/1829023002 adds fake and test
4. https://codereview.chromium.org/1899753002 fixes strict mode violations
5. https://codereview.chromium.org/1854093002 enables InstanceID by default
6. this patch
7. https://codereview.chromium.org/1923953002 integrates IIDs with crypto
8. https://codereview.chromium.org/1851423003 switches Push to InstanceIDs

Also depends on https://codereview.chromium.org/1948133003

BUG=589461

Committed: https://crrev.com/36ae5806242390404f2492c8a5c39041cd374fd6
Cr-Commit-Position: refs/heads/master@{#392617}
==========

[commit-bot: I haz the power, 2016-05-10 15:47:48]

Patchset 6 (id:??) landed as
https://crrev.com/36ae5806242390404f2492c8a5c39041cd374fd6
Cr-Commit-Position: refs/heads/master@{#392617}

[Ryan Sleevi, 2016-09-13 18:15:53]

rsleevi@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Sleevi, 2016-09-13 18:15:54]

I realize this is a four-month after-the fact drive-by, but we shouldn't be
exporting the DFATAL stuff of gtest_util from //net as part of //net's
interface.

While gtest_util has now also grown to accomodate aspects that //net consumers
do apparently need (such as GMock matchers for //net concepts), the notion of
DFATAL & friends are something that we reasonably should be discussing as either
part of //base or duplicated in //components, but I don't think the goal should
be that //net provides API surface for GTest-utilities.

Since this code appears to be the only (outside-net) consumer, would it be
possible to revisit this decision so that we can remove those bits from //net?