Break ChannelIDSigner into two classes: ChannelIDKey and ChannelIDSource.

ChannelIDSource looks up a channel ID key for a given hostname, and
ChannelIDKey does the signing.

Right now ChannelIDSource::GetChannelIDKey is still a synchronous function.
It will become asynchronous in a future CL. The assumption is that channel
ID keys may be stored on a disk, so the lookup may require disk access, but
once a channel ID key is loaded into memory, it can sign data without
blocking.

Corresponds to internal CL 68051215.

R=wtc@chromium.org
BUG=

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

[wtc, 2014-05-28 01:21:09]

Review comments on patch set 1:

https://codereview.chromium.org/300223007/diff/1/net/quic/test_tools/crypto_t...
File net/quic/test_tools/crypto_test_utils_openssl.cc (right):

https://codereview.chromium.org/300223007/diff/1/net/quic/test_tools/crypto_t...
net/quic/test_tools/crypto_test_utils_openssl.cc:107:
crypto::ScopedOpenSSL<EC_KEY, EC_KEY_free> ecdsa_key_;

The bug is here. It seems to be a copy-and-paste error.

This should be "EVP_PKEY, EVP_PKEY_free". Note the "PKEY". See line 98 of the
original code.

[wtc, 2014-05-28 22:04:29]

https://codereview.chromium.org/300223007/diff/2/net/quic/test_tools/crypto_t...
File net/quic/test_tools/crypto_test_utils_openssl.cc (right):

https://codereview.chromium.org/300223007/diff/2/net/quic/test_tools/crypto_t...
net/quic/test_tools/crypto_test_utils_openssl.cc:33: class TestChannelIDKey :
public ChannelIDKey {

net/quic/test_tools/crypto_test_utils_nss.cc also needs to be modified. The
changes are not too difficult if you understand the changes I made to this file.

You can give it a try. If you get stuck, I have the changes at
https://codereview.chromium.org/303933002/.

[wtc, 2014-05-29 21:59:14]

Patch set 3 LGTM.

Please fix the comment nits, upload a new patch set, and click the Commit box.

https://codereview.chromium.org/300223007/diff/30001/net/quic/test_tools/cryp...
File net/quic/test_tools/crypto_test_utils_nss.cc (right):

https://codereview.chromium.org/300223007/diff/30001/net/quic/test_tools/cryp...
net/quic/test_tools/crypto_test_utils_nss.cc:25: // implement real ChannelIDKey.

Nit: This comment is difficult to understand. I suggest we just keep the second
sentence: "Convert Sign() to be asynchronous using completion callback."

https://codereview.chromium.org/300223007/diff/30001/net/quic/test_tools/cryp...
net/quic/test_tools/crypto_test_utils_nss.cc:81: const SECKEYPublicKey*
public_key = ecdsa_keypair_->public_key();

Nit: since the comment block refers to "public_key", it is better to move this
declaration of public_key before the comment block.

[dmziegler, 2014-05-29 22:25:10]

The CQ bit was checked by dmziegler@google.com

[dmziegler, 2014-05-29 22:25:20]

The CQ bit was unchecked by dmziegler@google.com

[dmziegler, 2014-05-29 22:25:21]

The CQ bit was checked by dmziegler@google.com

[commit-bot: I haz the power, 2014-05-29 22:27:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmziegler@google.com/300223007/50001

[dmziegler, 2014-05-29 22:58:47]

On 2014/05/29 22:27:33, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-status.appspot.com/cq/dmziegler@google.com/300223007/50001

The android_dbg build is failing with three of these error messages:
../../net/quic/test_tools/crypto_test_utils_openssl.cc:41:42: error: override
controls (override/final) only available with -std=c++11 or -std=gnu++11
[-Werror]
                     string* out_signature) override {

Presumably, the patch needs to use |OVERRIDE| rather than |override|?

[wtc, 2014-05-29 23:29:44]

https://codereview.chromium.org/300223007/diff/50001/net/quic/test_tools/cryp...
File net/quic/test_tools/crypto_test_utils_openssl.cc (right):

https://codereview.chromium.org/300223007/diff/50001/net/quic/test_tools/cryp...
net/quic/test_tools/crypto_test_utils_openssl.cc:41: string* out_signature)
override {

Yes, please change "override" back to "OVERRIDE".

[wtc, 2014-05-29 23:33:09]

https://codereview.chromium.org/300223007/diff/50001/net/quic/test_tools/cryp...
File net/quic/test_tools/crypto_test_utils_openssl.cc (right):

https://codereview.chromium.org/300223007/diff/50001/net/quic/test_tools/cryp...
net/quic/test_tools/crypto_test_utils_openssl.cc:41: string* out_signature)
override {

Note that there are two other occurrences of "override" in this file. They also
need to be changed.

[dmziegler, 2014-05-30 00:53:01]

The CQ bit was checked by dmziegler@google.com

[commit-bot: I haz the power, 2014-05-30 00:53:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmziegler@google.com/300223007/70001

[commit-bot: I haz the power, 2014-05-30 07:11:28]

Change committed as 273748