Modify SpdyHeaderBlock's internals to consolidate header values only on first access.

Modify SpdyHeaderBlock's internals to consolidate header values only on first access.

This CL lands server changes 143452565 by birenroy and 143815813 by bnc.

BUG=488484
Review-Url: https://codereview.chromium.org/2611173004
Cr-Commit-Position: refs/heads/master@{#442924}
Committed: https://chromium.googlesource.com/chromium/src/+/e4800119bbc8228364586e61597e8fde5c9cddfd

[Bence, 2017-01-06 22:26:25]

The CQ bit was checked by bnc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 22:26:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bence, 2017-01-06 22:29:59]

Description was changed from

==========
Modify SpdyHeaderBlock's internals to consolidate header values only on first
access.

This CL lands server change 143452565 by birenroy.

BUG=488484
==========

to

==========
Modify SpdyHeaderBlock's internals to consolidate header values only on first
access.

This CL lands server changes 143452565 by birenroy and 143815813 by bnc.

BUG=488484
==========

[commit-bot: I haz the power, 2017-01-06 22:43:45]

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

[commit-bot: I haz the power, 2017-01-06 22:43:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Bence, 2017-01-09 13:27:06]

The CQ bit was checked by bnc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 13:27:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-09 13:44:26]

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

[commit-bot: I haz the power, 2017-01-09 13:44:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Bence, 2017-01-09 19:56:15]

The CQ bit was checked by bnc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 19:56:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-09 22:00:49]

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

[commit-bot: I haz the power, 2017-01-09 22:00:50]

Dry run: This issue passed the CQ dry run.

[Bence, 2017-01-09 23:45:28]

The CQ bit was checked by bnc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 23:46:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 01:50:47]

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

[commit-bot: I haz the power, 2017-01-10 01:50:48]

Dry run: This issue passed the CQ dry run.

[Bence, 2017-01-10 13:03:12]

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

[Bence, 2017-01-10 13:03:13]

Ryan: PTAL.  Thank you.

[Ryan Hamilton, 2017-01-10 22:46:12]

lgtm

https://codereview.chromium.org/2611173004/diff/140001/net/base/arena.h
File net/base/arena.h (right):

https://codereview.chromium.org/2611173004/diff/140001/net/base/arena.h#newco...
net/base/arena.h:27: };
Instead of a class, I wonder if a struct might be simpler. No need to change
this CL, but maybe a followup? (Or perhaps make status_ just a naked size_t
called bytes_allocated, or something?)

https://codereview.chromium.org/2611173004/diff/140001/net/spdy/spdy_header_b...
File net/spdy/spdy_header_block_test.cc (right):

https://codereview.chromium.org/2611173004/diff/140001/net/spdy/spdy_header_b...
net/spdy/spdy_header_block_test.cc:19: using std::vector;
nit: IIRC, the internal code has been modified to use the std:: in front of most
things. We recently landed a cleanup to QUIC shared code to remove these usings.
Not something for this CL, but something to think about.

[Bence, 2017-01-11 14:25:13]

Thank you.

https://codereview.chromium.org/2611173004/diff/140001/net/base/arena.h
File net/base/arena.h (right):

https://codereview.chromium.org/2611173004/diff/140001/net/base/arena.h#newco...
net/base/arena.h:27: };
On 2017/01/10 22:46:12, Ryan Hamilton wrote:
> Instead of a class, I wonder if a struct might be simpler. No need to change
> this CL, but maybe a followup? (Or perhaps make status_ just a naked size_t
> called bytes_allocated, or something?)

I would love that, but unfortunately Status is used in shared code, so the API
should match that of the one used internally.

https://codereview.chromium.org/2611173004/diff/140001/net/spdy/spdy_header_b...
File net/spdy/spdy_header_block_test.cc (right):

https://codereview.chromium.org/2611173004/diff/140001/net/spdy/spdy_header_b...
net/spdy/spdy_header_block_test.cc:19: using std::vector;
On 2017/01/10 22:46:12, Ryan Hamilton wrote:
> nit: IIRC, the internal code has been modified to use the std:: in front of
most
> things. We recently landed a cleanup to QUIC shared code to remove these
usings.
> Not something for this CL, but something to think about.

This is definitely worth exploring.  For the time being though, it seems that
vector does not have std:: in front in this particular file in the internal
code.

[Bence, 2017-01-11 14:25:21]

The CQ bit was checked by bnc@chromium.org

[Bence, 2017-01-11 14:25:21]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/2611173004/#ps160001
(title: "Rebase.")

[commit-bot: I haz the power, 2017-01-11 14:25:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Hamilton, 2017-01-11 15:05:06]

https://codereview.chromium.org/2611173004/diff/140001/net/base/arena.h
File net/base/arena.h (right):

https://codereview.chromium.org/2611173004/diff/140001/net/base/arena.h#newco...
net/base/arena.h:27: };
On 2017/01/11 14:25:13, Bence wrote:
> On 2017/01/10 22:46:12, Ryan Hamilton wrote:
> > Instead of a class, I wonder if a struct might be simpler. No need to change
> > this CL, but maybe a followup? (Or perhaps make status_ just a naked size_t
> > called bytes_allocated, or something?)
> 
> I would love that, but unfortunately Status is used in shared code, so the API
> should match that of the one used internally.

Yes, I meant doing this in shared code.

[commit-bot: I haz the power, 2017-01-11 16:02:59]

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

[commit-bot: I haz the power, 2017-01-11 16:03:00]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Bence, 2017-01-11 16:37:34]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2017-01-11 16:38:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-11 16:44:55]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1484152654687310,
"parent_rev": "48a22a1c2870a87e9e6ee10af358717480f72589", "commit_rev":
"e4800119bbc8228364586e61597e8fde5c9cddfd"}

[commit-bot: I haz the power, 2017-01-11 16:45:23]

Description was changed from

==========
Modify SpdyHeaderBlock's internals to consolidate header values only on first
access.

This CL lands server changes 143452565 by birenroy and 143815813 by bnc.

BUG=488484
==========

to

==========
Modify SpdyHeaderBlock's internals to consolidate header values only on first
access.

This CL lands server changes 143452565 by birenroy and 143815813 by bnc.

BUG=488484

Review-Url: https://codereview.chromium.org/2611173004
Cr-Commit-Position: refs/heads/master@{#442924}
Committed:
https://chromium.googlesource.com/chromium/src/+/e4800119bbc8228364586e61597e...
==========

[commit-bot: I haz the power, 2017-01-11 16:45:24]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/e4800119bbc8228364586e61597e...