SPDY & HPACK: Land recent internal changes (through 65328503)

SPDY & HPACK: Land recent internal changes (through 65328503)

Refactor RST_STREAM status code handling. Also handle SPDY4/HTTP2 status codes. 
Fix an off-by-one DCHECK for frame lengths equal to the frame limit.

This lands server change 63285101 by raullenchai and 63409085 by hkhalil. 
https://codereview.chromium.org/243643002/


Refactor GOAWAY status code handling, also handle SPDY4/HTTP2 status codes.

This lands server change 63424889 by hkhalil.
https://codereview.chromium.org/243613003/


SPDY tests: Additional headers validation in framer tests. 

Server change also included logic to lower-case headers and error on responses with incorrect casing. This wasn't merged, as Chromium instead has handling and tests on this in SpdyStream. 

This lands server change 63898388 by mlavan.
https://codereview.chromium.org/244833003/


SPDY: Replace SerializeDataFrameHeader.

Replaced with SerializeDataFrameHeaderWithPaddingLengthField, which serializes
DATA frame header and, when necessary, padding length fields. Tests are added.

This lands server change 64328823 by raullenchai.
https://codereview.chromium.org/244853004/


Modify SerializeHeaders, SerializePushPromise and SpdyFrameBuilder so that 
HEADERS and PUSH_PROMISE frame payloads can be serialized into CONTINUATION 
frames as needed. 

This lands server change 64331353 by mlavan.
https://codereview.chromium.org/246013002/


SPDY: RST_STREAM and GO_AWAY status checking.

Call OnError() from ProcessRstStreamFramePayload() and
ProcessGoAwayFramePayload() when handling an invalid status code.

This lands server change 64372977 by mlavan.
https://codereview.chromium.org/247503002/


SPDY: Fix flaky padding edge case.

The root cause of the flakiness is: during the processing of data frame, the
visitor should be informed if the FIN flag is set and there is no more data in
this frame. One corner case that has not been covered is when
remaining_data_length_ = remaining_padding_payload_length_ = 0, which does
happen when the data payload is empty (e.g., GET request) and the padding len
is exactly 1.

This lands server change 64513441 by raullenchai.
https://codereview.chromium.org/246933003/


SPDY: Use SpdyMajorVersion rather than ints for SPDY version number. 

Version comparisons are canonicalized on using the lowest applicable version 
number. Also add new SPDY5 enum (just the literal; no implementation). 
Fix a bug in cl/64786464 that could cause SPDY version to be 
mis-parsed in the framer. 

This lands server change 64786464 by mlavan and 64899106 by mlavan.
https://codereview.chromium.org/247243003/


Remove net:: when referencing types from code locations already within the net:: namespace. 
Remove obsolete TODO. 

This lands server change 64909661 by hkhalil and 64909678 by hkhalil.
https://codereview.chromium.org/246123005/


SPDY: Catch another edge case in version validation.

Specifically, where the version number is invalid, but happens to match the
enum number of a valid version. Also consolidate version checks in the framer
so that related logic is easier to reason about.

This lands server change 64912010 by mlavan.
https://codereview.chromium.org/247583002/


HPACK: Refactor and expand HpackHeaderTable indexing.

HpackHeaderTable now maintains indices for querying static and dynamic
entries by name and name & value. Indicies are ordered on entry name,
value, then table index. A full-table index is maintained, as well as a
reference-set index.

HpackEntry no longer tracks reference-set status (that's HpackHeaderTable's
job), and the "touch" mechanism has been replaced with a similar "state".
HpackHeaderTable & HpackEntry also now cooperate to provide HpackEntry with
sufficient bookmarking to compute it's own index.

HpackEncodingContext has been eliminated. HpackHeaderTable manages its
former responsibilities (primarly managing the static table).

Together, these changes allow an encoder to:
* Build the encoding delta via a linear walk through (just) the reference set
(assuming headers are already sorted, ala std::map).
* Map representations to entries in O(log(N)) time, preferring the
lowest-index entry.
* Determine a mapped entry's index in O(1) time.
* Add and evict representations to the dynamic table in O(log(N)) time.
* Efficiently enumerate all headers having a specific name. Not required now,
but may be in the future.

This lands server change 65185410 by jgraettinger.
https://codereview.chromium.org/246383003/


HPACK: Refactor and simplify HpackOutputStream

HpackOutputStream is no longer responsible for emitting entire opcodes. That
will be the encoder's job. Instead, it simply provides primitives for emitting
prefixes, varints, and byte arrays. The longer-term intent is that
HpackOutputStream will be merged into SpdyFrameBuilder and eliminated
altogether.

HpackOutputStream also no longer asserts a maximum literal length. Rationale
is that we already have a theoretical too-long header at this point: we might
as well emit it and allow the remote decoder to possibly accept it. At worst
we break the connection, which was going to happen anyway.

This lands server change 65186763 by jgraettinger.
https://codereview.chromium.org/247793002/


HPACK: Add HpackHuffmanTable::EncodedSize() 

EncodedSize() will be used by the HPACK encoder to select identity vs Huffman 
coding, without having to run the full Huffman coder. Also update tests to 
compare against EncodedSize() by default. 
HPACK unittest: Fixing memory error in Huffman test fixture. 

This lands server change 65187740 by jgraettinger and 65328503 by jgraettinger.
https://codereview.chromium.org/247893002/


HPACK: Implement delta encoding.

HpackEncoder determines each header set's delta with the reference set, and
makes use of the full HPACK encoding grammar to emit a minimal compression
for the set.

A "encode without compression" method has also been added which bypasses the
dynamic table & Huffman coding. Selection of encoding method is controlled by
SpdyFramer::enable_compression_.

Encoding context updates are not yet used. That's a future optimization.

HpackRoundTripTest has been added to verify that HpackEncoder & HpackDecoder
successfully round-trip sequences of canned and dynamically generated header
sets.

This lands server change 65188922 by jgraettinger.
https://codereview.chromium.org/247283003/


BUG=345769, 339578
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266904

[Ryan Hamilton, 2014-04-23 22:33:23]

lgtm

[Johnny, 2014-04-24 16:36:11]

Alexei or Jim, would one of you review for histograms.xml?

The only change is from this CL, which you're both on:
https://codereview.chromium.org/243643002/

Thanks

[Alexei Svitkine (slow), 2014-04-24 17:06:42]

I'll let Jim review since this is touching the network coding which he's
more familiar with.


On Thu, Apr 24, 2014 at 12:36 PM, <jgraettinger@chromium.org> wrote:

> Alexei or Jim, would one of you review for histograms.xml?
>
> The only change is from this CL, which you're both on:
> https://codereview.chromium.org/243643002/
>
> Thanks
>
> https://codereview.chromium.org/246073007/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jar (doing other things), 2014-04-28 19:27:07]

lgtm

[Johnny, 2014-04-28 19:28:48]

The CQ bit was checked by jgraettinger@chromium.org

[commit-bot: I haz the power, 2014-04-28 19:29:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jgraettinger@chromium.org/246073007/10...

[commit-bot: I haz the power, 2014-04-28 22:40:59]

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

[commit-bot: I haz the power, 2014-04-28 22:40:59]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[Johnny, 2014-04-29 02:27:13]

The CQ bit was checked by jgraettinger@chromium.org

[commit-bot: I haz the power, 2014-04-29 02:28:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jgraettinger@chromium.org/246073007/10...

[commit-bot: I haz the power, 2014-04-29 05:34:43]

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

[commit-bot: I haz the power, 2014-04-29 05:34:43]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[Johnny, 2014-04-29 14:34:54]

The CQ bit was checked by jgraettinger@chromium.org

[commit-bot: I haz the power, 2014-04-29 14:35:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jgraettinger@chromium.org/246073007/10...

[commit-bot: I haz the power, 2014-04-29 17:02:08]

Change committed as 266904