Adds plumbing for always requiring handshake confirmation in QUIC.

Adds plumbing for always requiring handshake confirmation in QUIC, for use in Finch field trials.

BUG=410436
Committed: https://crrev.com/2b966f217ba9f38cb4e625bdabbae0044dd28523
Cr-Commit-Position: refs/heads/master@{#293010}

[Jana, 2014-08-29 21:36:59]

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

[Jana, 2014-08-29 21:42:10]

Apologies -- fixed logic. Use this patch set for the review.

[Jana, 2014-08-29 23:27:50]

Added test and cleaned up code a bit. Also used formatting suggestions from cl
format.

[Ryan Hamilton, 2014-09-02 19:28:22]

Looks great! Just a few small comments.

https://codereview.chromium.org/524463004/diff/40001/chrome/browser/io_thread...
File chrome/browser/io_thread_unittest.cc (right):

https://codereview.chromium.org/524463004/diff/40001/chrome/browser/io_thread...
chrome/browser/io_thread_unittest.cc:330:
EXPECT_FALSE(params.quic_always_require_handshake_confirmation);
I would move this line to the test EnableQuicFromFieldTrialGroup which
effectively tests the defaults.

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
File net/quic/quic_stream_factory.cc (right):

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
net/quic/quic_stream_factory.cc:605: require_confirmation_ = false;
You could consider changing this to be:

  if (!always_require_confirmation && rv == OK) 
    require_confirmation_ = false;

Then your getter could still simply return require_confirmation_

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
File net/quic/quic_stream_factory.h (right):

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
net/quic/quic_stream_factory.h:158: bool require_confirmation() const {
Hm. This method is named in hacker_case which typically means that it's a simple
getter of the similarly named member variable (require_confirmation_). Having it
return the || or two different members is a bit confusing.

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
File net/quic/quic_stream_factory_test.cc (right):

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
net/quic/quic_stream_factory_test.cc:113: true,
As long as you're here, can you add comments for each of these three literals?

[Jana, 2014-09-02 20:58:16]

Thanks, Ryan. Comments addressed, and responses inline.

https://codereview.chromium.org/524463004/diff/40001/chrome/browser/io_thread...
File chrome/browser/io_thread_unittest.cc (right):

https://codereview.chromium.org/524463004/diff/40001/chrome/browser/io_thread...
chrome/browser/io_thread_unittest.cc:330:
EXPECT_FALSE(params.quic_always_require_handshake_confirmation);
On 2014/09/02 19:28:22, Ryan Hamilton wrote:
> I would move this line to the test EnableQuicFromFieldTrialGroup which
> effectively tests the defaults.

Done.

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
File net/quic/quic_stream_factory.cc (right):

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
net/quic/quic_stream_factory.cc:605: require_confirmation_ = false;
On 2014/09/02 19:28:22, Ryan Hamilton wrote:
> You could consider changing this to be:
> 
>   if (!always_require_confirmation && rv == OK) 
>     require_confirmation_ = false;
> 
> Then your getter could still simply return require_confirmation_

Thanks for the suggestion -- I've made changes that capture what you want, I
think. PTAL.

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
File net/quic/quic_stream_factory_test.cc (right):

https://codereview.chromium.org/524463004/diff/40001/net/quic/quic_stream_fac...
net/quic/quic_stream_factory_test.cc:113: true,
On 2014/09/02 19:28:22, Ryan Hamilton wrote:
> As long as you're here, can you add comments for each of these three literals?

Done.

[Ryan Hamilton, 2014-09-02 21:05:49]

The CQ bit was checked by rch@chromium.org

[Ryan Hamilton, 2014-09-02 21:05:50]

lgtm

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

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jri@chromium.org/524463004/60001

[commit-bot: I haz the power, 2014-09-02 22:26:45]

Committed patchset #4 (id:60001) as 5349b8278082a4d9103b9d6e91b703acc6d3709a

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

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