Introduce QuicStreamFactoryTest::DefaultProofVerifyDetails().

Introduce QuicStreamFactoryTest::DefaultProofVerifyDetails().

Introduce QuicStreamFactoryTest::DefaultProofVerifyDetails(), a helper function
to set up a ProofVerifyDetailsChromium object with a default cert loaded.

BUG=474217
Committed: https://crrev.com/f8bf0721c2c4ab8a6204832c1971bcfec46ef801
Cr-Commit-Position: refs/heads/master@{#330589}

[Bence, 2015-05-19 14:52:15]

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

[Bence, 2015-05-19 14:52:16]

Ryan: PTAL.  Thanks.

[Ryan Hamilton, 2015-05-19 19:16:56]

LGTM, but I think I would just remove the comment.

https://codereview.chromium.org/1142953002/diff/1/net/quic/quic_stream_factor...
File net/quic/quic_stream_factory_test.cc (right):

https://codereview.chromium.org/1142953002/diff/1/net/quic/quic_stream_factor...
net/quic/quic_stream_factory_test.cc:308: // Note that
MockCryptoClientStreamFactory::AddProofVerifyDetails() does not
I don't understand what this comment means.  What is a "named object"?

[Bence, 2015-05-19 19:41:29]

The CQ bit was checked by bnc@chromium.org

[Bence, 2015-05-19 19:41:56]

The CQ bit was unchecked by bnc@chromium.org

[Bence, 2015-05-19 19:43:37]

The CQ bit was checked by bnc@chromium.org

[Bence, 2015-05-19 19:43:38]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1142953002/#ps20001
(title: "Remove comment.")

[commit-bot: I haz the power, 2015-05-19 19:44:26]

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

[Bence, 2015-05-19 19:45:33]

Thanks.

https://codereview.chromium.org/1142953002/diff/1/net/quic/quic_stream_factor...
File net/quic/quic_stream_factory_test.cc (right):

https://codereview.chromium.org/1142953002/diff/1/net/quic/quic_stream_factor...
net/quic/quic_stream_factory_test.cc:308: // Note that
MockCryptoClientStreamFactory::AddProofVerifyDetails() does not
On 2015/05/19 19:16:56, Ryan Hamilton wrote:
> I don't understand what this comment means.  What is a "named object"?

I'm landing without the comment as you suggested, but I'd like to add this in
the next CL.  Please help me with the wording here.  I'm trying to say that
crypto_client_stream_factory_.AddProofVerifyDetails(DefaultProofVerifyDetails())
would not work, because the argument is a temporary that gets destroyed right
after the call finished.  Instead, the test need to have an object in local
scope that remains valid while the test runs.

Or is it better to proactively change the implementation to prevent
use-after-free error?

[Ryan Hamilton, 2015-05-19 19:55:34]

https://codereview.chromium.org/1142953002/diff/1/net/quic/quic_stream_factor...
File net/quic/quic_stream_factory_test.cc (right):

https://codereview.chromium.org/1142953002/diff/1/net/quic/quic_stream_factor...
net/quic/quic_stream_factory_test.cc:308: // Note that
MockCryptoClientStreamFactory::AddProofVerifyDetails() does not
On 2015/05/19 19:45:33, Bence wrote:
> On 2015/05/19 19:16:56, Ryan Hamilton wrote:
> > I don't understand what this comment means.  What is a "named object"?
> 
> I'm landing without the comment as you suggested, but I'd like to add this in
> the next CL.  Please help me with the wording here.  I'm trying to say that
>
crypto_client_stream_factory_.AddProofVerifyDetails(DefaultProofVerifyDetails())
> would not work, because the argument is a temporary that gets destroyed right
> after the call finished.  

Will that actually compile? AddProofVerifyDetails takes a ProofVerifyDetails*
but DefaultProofVerifyDetails() returns a ProofVerifyDetails (pointer vs object)

> Instead, the test need to have an object in local
> scope that remains valid while the test runs.
> 
> Or is it better to proactively change the implementation to prevent
> use-after-free error?

[Bence, 2015-05-19 19:59:06]

On 2015/05/19 19:55:34, Ryan Hamilton wrote:
>
https://codereview.chromium.org/1142953002/diff/1/net/quic/quic_stream_factor...
> File net/quic/quic_stream_factory_test.cc (right):
> 
>
https://codereview.chromium.org/1142953002/diff/1/net/quic/quic_stream_factor...
> net/quic/quic_stream_factory_test.cc:308: // Note that
> MockCryptoClientStreamFactory::AddProofVerifyDetails() does not
> On 2015/05/19 19:45:33, Bence wrote:
> > On 2015/05/19 19:16:56, Ryan Hamilton wrote:
> > > I don't understand what this comment means.  What is a "named object"?
> > 
> > I'm landing without the comment as you suggested, but I'd like to add this
in
> > the next CL.  Please help me with the wording here.  I'm trying to say that
> >
>
crypto_client_stream_factory_.AddProofVerifyDetails(DefaultProofVerifyDetails())
> > would not work, because the argument is a temporary that gets destroyed
right
> > after the call finished.  
> 
> Will that actually compile? AddProofVerifyDetails takes a ProofVerifyDetails*
> but DefaultProofVerifyDetails() returns a ProofVerifyDetails (pointer vs
object)

Oops, you are right.  Never mind.

[commit-bot: I haz the power, 2015-05-19 20:04:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-19 20:05:01]

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