Fix a QUIC bug where previously undecryptable packets were not decrypted

Fix a QUIC bug where previously undecryptable packets were not decrypted
before sending out an ack when the encryption level changed.

Merge internal change: 60051502

Added debugging code to dump QuicFrame and Headers.

R=rch@chromium.org

[ramant (doing other things), 2014-01-25 05:00:31]

Patch set# 2 has a change to make quic_http_stream unittests to pass. Will send
the frames quic_packet_creator created and the call stack in a private email.

I am not 100% sure of this change.

thanks
raman

[Ryan Hamilton, 2014-01-27 19:08:02]

https://codereview.chromium.org/144063012/diff/70001/net/quic/quic_http_strea...
File net/quic/quic_http_stream_test.cc (right):

https://codereview.chromium.org/144063012/diff/70001/net/quic/quic_http_strea...
net/quic/quic_http_stream_test.cc:615: // AddWrite(ConstructRstStreamPacket(2));
Do you understand why the RST_STREAM packet is not sent?

[ramant (doing other things), 2014-01-29 00:40:19]

Hi Ryan,
  As you have suggested, added some debugging code. Would appreciate if you
could take another look at this patch.

thanks
raman

https://codereview.chromium.org/144063012/diff/70001/net/quic/quic_http_strea...
File net/quic/quic_http_stream_test.cc (right):

https://codereview.chromium.org/144063012/diff/70001/net/quic/quic_http_strea...
net/quic/quic_http_stream_test.cc:615: // AddWrite(ConstructRstStreamPacket(2));
On 2014/01/27 19:08:02, Ryan Hamilton wrote:
> Do you understand why the RST_STREAM packet is not sent?

Thanks for your help. Undid this change. Replaced it with sending ACK and RST
frames in one packet.

[Ryan Hamilton, 2014-01-29 00:52:51]

Thanks so much for getting to the bottom of this!  Just a couple of nits.

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_framer.cc
File net/quic/quic_framer.cc (right):

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_framer.cc...
net/quic/quic_framer.cc:622: DVLOG(1) << "Header: " << header;
nit: "Appending header: "

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_http_stre...
File net/quic/quic_http_stream_test.cc (right):

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_http_stre...
net/quic/quic_http_stream_test.cc:267: QuicPacketSequenceNumber sequence_number,
nit: Since the final 2 args are always 1, I'd remove them from this method so it
only takes sequence_number.  If we ever need to use different values, we can add
them back.

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_packet_cr...
File net/quic/quic_packet_creator.cc (right):

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_packet_cr...
net/quic/quic_packet_creator.cc:425: if (frame.type == ACK_FRAME) {
nit:I think I would use a switch statement here.  You might want to consider
adding an operator<< method to QuicFrame which actually does this.  Then you
could remove the #ifdef:

DVLOG(1) << "Adding: " << frame;

[ramant (doing other things), 2014-01-29 02:45:38]

Made all the changes. PTAL. thanks.

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_framer.cc
File net/quic/quic_framer.cc (right):

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_framer.cc...
net/quic/quic_framer.cc:622: DVLOG(1) << "Header: " << header;
On 2014/01/29 00:52:52, Ryan Hamilton wrote:
> nit: "Appending header: "

Done.

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_http_stre...
File net/quic/quic_http_stream_test.cc (right):

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_http_stre...
net/quic/quic_http_stream_test.cc:267: QuicPacketSequenceNumber sequence_number,
On 2014/01/29 00:52:52, Ryan Hamilton wrote:
> nit: Since the final 2 args are always 1, I'd remove them from this method so
it
> only takes sequence_number.  If we ever need to use different values, we can
add
> them back.

Done.

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_packet_cr...
File net/quic/quic_packet_creator.cc (right):

https://codereview.chromium.org/144063012/diff/170001/net/quic/quic_packet_cr...
net/quic/quic_packet_creator.cc:425: if (frame.type == ACK_FRAME) {
On 2014/01/29 00:52:52, Ryan Hamilton wrote:
> nit:I think I would use a switch statement here.  You might want to consider
> adding an operator<< method to QuicFrame which actually does this.  Then you
> could remove the #ifdef:
> 
> DVLOG(1) << "Adding: " << frame;

Done.

[Ryan Hamilton, 2014-01-29 03:07:55]

lgtm

THANKS!