Implement basic classes for HPACK (HTTP/2 compression)

Implement basic classes for HPACK (HTTP/2 compression)

Create HpackEncodingContext, HpackHeaderTable, and HpackEntry classes,
which are based on the draft
http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-04 .

Implement some basic tests.

This implementation is based on
https://github.com/akalin-chromium/httpbis-header-compression and
https://codereview.chromium.org/22074002/ (which was based on an
earlier draft).

This lands server change 55803359 by akalin.

R=akalin@chromium.org

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

[Johnny, 2014-01-14 16:49:28]

PTAL

[akalin, 2014-01-14 20:54:10]

lgtm, assuming tryjobs pass, etc.

https://codereview.chromium.org/138243003/diff/90001/net/spdy/hpack_string_ut...
File net/spdy/hpack_string_util_test.cc (right):

https://codereview.chromium.org/138243003/diff/90001/net/spdy/hpack_string_ut...
net/spdy/hpack_string_util_test.cc:25: // TODO(jgraettinger): Support this
benchmark.
Out of curiosity, what's the issue here? Does Chromium's gtest not support
benchmarks?

[akalin, 2014-01-14 20:55:28]

On 2014/01/14 20:54:10, akalin wrote:
> lgtm, assuming tryjobs pass, etc.
> 
>
https://codereview.chromium.org/138243003/diff/90001/net/spdy/hpack_string_ut...
> File net/spdy/hpack_string_util_test.cc (right):
> 
>
https://codereview.chromium.org/138243003/diff/90001/net/spdy/hpack_string_ut...
> net/spdy/hpack_string_util_test.cc:25: // TODO(jgraettinger): Support this
> benchmark.
> Out of curiosity, what's the issue here? Does Chromium's gtest not support
> benchmarks?

btw, i usually leave the committer name out of the "This lands server change
..." line, but I'm okay either way.

[akalin, 2014-01-14 20:58:19]

On 2014/01/14 20:55:28, akalin wrote:
> On 2014/01/14 20:54:10, akalin wrote:
> > lgtm, assuming tryjobs pass, etc.
> > 
> >
>
https://codereview.chromium.org/138243003/diff/90001/net/spdy/hpack_string_ut...
> > File net/spdy/hpack_string_util_test.cc (right):
> > 
> >
>
https://codereview.chromium.org/138243003/diff/90001/net/spdy/hpack_string_ut...
> > net/spdy/hpack_string_util_test.cc:25: // TODO(jgraettinger): Support this
> > benchmark.
> > Out of curiosity, what's the issue here? Does Chromium's gtest not support
> > benchmarks?
> 
> btw, i usually leave the committer name out of the "This lands server change
> ..." line, but I'm okay either way.

Also, aren't there a few changes before this one that should get landed too? Or
did they get landed via the QUIC merges? The last one I see on Chromium is
51214714 (r227429).

[Johnny, 2014-01-14 21:10:25]

> Also, aren't there a few changes before this one that should get landed too?
Or
> did they get landed via the QUIC merges? The last one I see on Chromium is
> 51214714 (r227429).

You landed 52442340 in October, here: https://codereview.chromium.org/26413002
I've been pulling patches starting at that point; is there prior stuff which
wasn't merged? If so, I haven't noticed it yet.

This massive QUIC merge also did pull in a SPDY change (56467687):
https://codereview.chromium.org/76723002

[Johnny, 2014-01-14 21:11:20]

https://codereview.chromium.org/138243003/diff/90001/net/spdy/hpack_string_ut...
File net/spdy/hpack_string_util_test.cc (right):

https://codereview.chromium.org/138243003/diff/90001/net/spdy/hpack_string_ut...
net/spdy/hpack_string_util_test.cc:25: // TODO(jgraettinger): Support this
benchmark.
Correct, not that I can tell.

On 2014/01/14 20:54:10, akalin wrote:
> Out of curiosity, what's the issue here? Does Chromium's gtest not support
> benchmarks?

[akalin, 2014-01-15 01:18:11]

On 2014/01/14 21:10:25, Johnny wrote:
> > Also, aren't there a few changes before this one that should get landed too?
> Or
> > did they get landed via the QUIC merges? The last one I see on Chromium is
> > 51214714 (r227429).
> 
> You landed 52442340 in October, here: https://codereview.chromium.org/26413002
> I've been pulling patches starting at that point; is there prior stuff which
> wasn't merged? If so, I haven't noticed it yet.
> 
> This massive QUIC merge also did pull in a SPDY change (56467687):
> https://codereview.chromium.org/76723002

Ah, you're right, I didn't see your earlier landings.

[akalin, 2014-01-15 01:19:23]

On 2014/01/15 01:18:11, akalin wrote:
> On 2014/01/14 21:10:25, Johnny wrote:
> > > Also, aren't there a few changes before this one that should get landed
too?
> > Or
> > > did they get landed via the QUIC merges? The last one I see on Chromium is
> > > 51214714 (r227429).
> > 
> > You landed 52442340 in October, here:
https://codereview.chromium.org/26413002
> > I've been pulling patches starting at that point; is there prior stuff which
> > wasn't merged? If so, I haven't noticed it yet.
> > 
> > This massive QUIC merge also did pull in a SPDY change (56467687):
> > https://codereview.chromium.org/76723002
> 
> Ah, you're right, I didn't see your earlier landings.

Still lgtm, although it seems you have some compile errors in the tests.

(One of the most common things to run into when pulling code into Chromium is
the signed/unsigned warning.)

[Johnny, 2014-01-15 16:25:18]

Looks like PS4 did the trick, though I'm not sure why this issue hasn't updated
with the try successes yet.

Lesson learned: build & run locally with component=shared_library to catch
export omissions.

[commit-bot: I haz the power, 2014-01-15 17:40:44]

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

[commit-bot: I haz the power, 2014-01-15 18:22:39]

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

[commit-bot: I haz the power, 2014-01-15 23:53:02]

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

[commit-bot: I haz the power, 2014-01-16 06:15:21]

Change committed as 245162