Always expect certificates in QuicChromiumClientSession::CanPool

Always expect certificates in QuicChromiumClientSession::CanPool

Committed: https://crrev.com/6faa4d41d61c005a9474f419b95bd00002e292b7
Cr-Commit-Position: refs/heads/master@{#367640}

[Ryan Hamilton, 2015-12-30 21:29:37]

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

[Zhongyi Shi, 2015-12-30 22:55:31]

Thanks for doing this! Only one small comment in quic_network_transaction.

https://codereview.chromium.org/1552783003/diff/1/net/quic/quic_network_trans...
File net/quic/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1552783003/diff/1/net/quic/quic_network_trans...
net/quic/quic_network_transaction_unittest.cc:212:
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details_);
Instead of adding two ProofVerifyDetailsChromium here, is that better to leave
as it is and add ProofVerifyDetailsChromium in the tests when needed?

Or it might be better than we never insert ProofVerifyDetailsChromium in the
cxtor but do that in the test when needed? Some of our test don't require cert
at all.

[Ryan Hamilton, 2015-12-30 23:21:12]

https://codereview.chromium.org/1552783003/diff/1/net/quic/quic_network_trans...
File net/quic/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1552783003/diff/1/net/quic/quic_network_trans...
net/quic/quic_network_transaction_unittest.cc:212:
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details_);
On 2015/12/30 22:55:31, Zhongyi Shi wrote:
> Instead of adding two ProofVerifyDetailsChromium here, is that better to leave
> as it is and add ProofVerifyDetailsChromium in the tests when needed?

Done. It's just one test.

> Or it might be better than we never insert ProofVerifyDetailsChromium in the
> cxtor but do that in the test when needed? Some of our test don't require cert
> at all. 

I thought about that, but it seemed like it was more hassle than it was worth as
it 
would just add noise to all the tests.

[Zhongyi Shi, 2015-12-30 23:36:31]

On 2015/12/30 23:21:12, Ryan Hamilton wrote:
>
https://codereview.chromium.org/1552783003/diff/1/net/quic/quic_network_trans...
> File net/quic/quic_network_transaction_unittest.cc (right):
> 
>
https://codereview.chromium.org/1552783003/diff/1/net/quic/quic_network_trans...
> net/quic/quic_network_transaction_unittest.cc:212:
> crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details_);
> On 2015/12/30 22:55:31, Zhongyi Shi wrote:
> > Instead of adding two ProofVerifyDetailsChromium here, is that better to
leave
> > as it is and add ProofVerifyDetailsChromium in the tests when needed?
> 
> Done. It's just one test.
> 
> > Or it might be better than we never insert ProofVerifyDetailsChromium in the
> > cxtor but do that in the test when needed? Some of our test don't require
cert
> > at all. 
> 
> I thought about that, but it seemed like it was more hassle than it was worth
as
> it 
> would just add noise to all the tests.

lgtm

[Ryan Hamilton, 2016-01-05 00:19:56]

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

[Ryan Hamilton, 2016-01-05 00:19:56]

raman: can you approve?

[ramant (doing other things), 2016-01-05 00:54:15]

lgtm

[Ryan Hamilton, 2016-01-05 01:20:47]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2016-01-05 01:21:12]

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

[commit-bot: I haz the power, 2016-01-05 02:53:59]

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

[commit-bot: I haz the power, 2016-01-05 02:53:59]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Zhongyi Shi, 2016-01-05 19:39:33]

The CQ bit was checked by zhongyi@chromium.org

[commit-bot: I haz the power, 2016-01-05 19:40:48]

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

[commit-bot: I haz the power, 2016-01-05 20:48:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-05 20:50:54]

Description was changed from

==========
Always expect certificates in QuicChromiumClientSession::CanPool
==========

to

==========
Always expect certificates in QuicChromiumClientSession::CanPool

Committed: https://crrev.com/6faa4d41d61c005a9474f419b95bd00002e292b7
Cr-Commit-Position: refs/heads/master@{#367640}
==========

[commit-bot: I haz the power, 2016-01-05 20:50:54]

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