Land Recent QUIC Changes

Land Recent QUIC Changes

Handle versioning by closing the connection on version mismatch for now.

Merge internal change: 43606997

Number of cleanups from landing recent crypto changes.

Merge internal change: 43606111

Added delta_time_largest_observed to ReceivedPacketInfo to calculate accurate RTT.

Merge internal change: 43582099

Implement server-side QUIC key expansion. The derived keys are still not being used yet.

TODO: Code is in crypto_test_utils needs to be enabled.

Merge internal change: 43570937

Added AbandoningPacket to congestion control to avoid issue with FEC.

Merge internal change: 43570099

Wait infinite (aka wait for next ack) is not handled correctly.

Merge internal change: 43558636

Enable faster stats for QUIC.

Merge internal change: 43557310

Implement QUIC key expansion on the client side.  The keys are not being used yet.

Merge internal change: 43515237

Add missing quic_stats files.

Track some connection stats.

Merge internal change: 43506869

Fix bug in WriteQueuedPackets

Merge internal change: 43499600

Small comment change in crypto_handshake's ProcessServerHello method.

Merge internal change: 43448804

R=rch@chromium.org
BUG=

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

[Ryan Hamilton, 2013-03-12 23:12:21]

lgtm

Rockin!  I've reviewed all the subsidiary patches and this has the right
description.

[commit-bot: I haz the power, 2013-03-13 22:59:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/12806002/39001

[commit-bot: I haz the power, 2013-03-13 22:59:30]

Presubmit check for 12806002-39001 failed and returned exit status 1.

INFO:root:Found 77 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/crypto/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/net/PRESUBMIT.py

** Presubmit Messages **
You might be calling functions intended only for testing from
production code.  It is OK to ignore this warning if you know what
you are doing, as the heuristics used to detect the situation are
not perfect.  The commit queue will not block on this warning.
Email joi@chromium.org if you have questions.
  net/quic/crypto/crypto_handshake.cc:566
    scoped_ptr<QuicServerConfigProtobuf> config(ConfigForTesting(

If this change has an associated bug, add BUG=[bug number].

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    crypto/hkdf.cc

Presubmit checks took 4.8s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[ramant (doing other things), 2013-03-13 23:12:42]

Hi Wan-Teh,
  Added a test for key abd iv bytes size to be greater than 0 before we build
string piece and assign it to a std:string (this crashes on windows). Will
upstream the hkdf.cc change.

  rch has reviewed all quic changes. Would appreciate, if you could review
crypto/hkdf.cc change.

thanks

[wtc, 2013-03-14 00:38:08]

Patch set 10 LGTM. I only reviewed crypto/hkdf.cc.

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc
File crypto/hkdf.cc (right):

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc#new...
crypto/hkdf.cc:75: // On Windows, when output's size is zero, dereference of
0'th element results

output's => the size of output_

because "output_'s" looks strange.

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc#new...
crypto/hkdf.cc:76: // in a crash. C++11 solves this problem by passing
std::vector's data.

This should read:
  C++11 solves this problem by adding a data() getter method
  to std::vector.

In fact, can you try if we can replace
  &output_[j]
by
  output_.data() + j
?

Perhaps the data() method is already available in MSVC, gcc,
and clang.

You can do this experiment after committing this CL.

[ramant (doing other things), 2013-03-14 03:05:15]

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc
File crypto/hkdf.cc (right):

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc#new...
crypto/hkdf.cc:75: // On Windows, when output's size is zero, dereference of
0'th element results
On 2013/03/14 00:38:08, wtc wrote:
> 
> output's => the size of output_
> 
> because "output_'s" looks strange.

Deleted this comment

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc#new...
crypto/hkdf.cc:76: // in a crash. C++11 solves this problem by passing
std::vector's data.
On 2013/03/14 00:38:08, wtc wrote:
> 
> This should read:
>   C++11 solves this problem by adding a data() getter method
>   to std::vector.
> 
> In fact, can you try if we can replace
>   &output_[j]
> by
>   output_.data() + j
> ?
> 
> Perhaps the data() method is already available in MSVC, gcc,
> and clang.
> 
> You can do this experiment after committing this CL.

Changed the code to use output_.data() + j. Thanks for the fix.
Done.

[commit-bot: I haz the power, 2013-03-14 03:11:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/12806002/48001

[ramant (doing other things), 2013-03-14 03:26:20]

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc
File crypto/hkdf.cc (right):

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc#new...
crypto/hkdf.cc:75: // On Windows, when output's size is zero, dereference of
0'th element results
On 2013/03/14 03:05:15, ramant wrote:
> On 2013/03/14 00:38:08, wtc wrote:
> > 
> > output's => the size of output_
> > 
> > because "output_'s" looks strange.
> 
> Deleted this comment

Added the comment.

https://chromiumcodereview.appspot.com/12806002/diff/39002/crypto/hkdf.cc#new...
crypto/hkdf.cc:76: // in a crash. C++11 solves this problem by passing
std::vector's data.
On 2013/03/14 03:05:15, ramant wrote:
> On 2013/03/14 00:38:08, wtc wrote:
> > 
> > This should read:
> >   C++11 solves this problem by adding a data() getter method
> >   to std::vector.
> > 
> > In fact, can you try if we can replace
> >   &output_[j]
> > by
> >   output_.data() + j
> > ?
> > 
> > Perhaps the data() method is already available in MSVC, gcc,
> > and clang.
> > 
> > You can do this experiment after committing this CL.
> 
> Changed the code to use output_.data() + j. Thanks for the fix.
> Done.

Added the comment. output_.data() + j is not supported on Android.

Added your comments

[commit-bot: I haz the power, 2013-03-14 03:31:30]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-03-14 03:38:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/12806002/58003

[commit-bot: I haz the power, 2013-03-14 05:10:48]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-03-14 05:55:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/12806002/58003

[commit-bot: I haz the power, 2013-03-14 16:25:35]

Change committed as 188096