Code cleanup of control frame size enforcement.

Code cleanup of control frame size enforcement.

Introduce kMaxControlFrameSize global const for fragmenting outgoing frames.
Get rid of GetControlFrameBufferMaxSize() and GetHeaderFragmentMaxSize() inline
members.  Motivation is to converge with server side code.

Only functional change is to decrease control frame fragment size to 1 kB for
SPDY.  Since frame headers are tiny, this causes little overhead.  Also,
incoming control frame size limit is now properly enforced (there was an
incorrect offset by frame header size before).

This CL lands server change 78267288 among other modifications.

Committed: https://crrev.com/0e2deda3f0c59c8fc18323ede48a552bef341c24
Cr-Commit-Position: refs/heads/master@{#301154}

[Bence, 2014-10-24 13:01:52]

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

[Bence, 2014-10-24 13:01:53]

Ryan: PTAL.

[Ryan Hamilton, 2014-10-24 16:54:43]

Can you clarify what changes are NOT part of the internal change?

I think it LGTM, but I'd like to confirm.

[Bence, 2014-10-24 17:16:10]

On 2014/10/24 16:54:43, Ryan Hamilton wrote:
> Can you clarify what changes are NOT part of the internal change?
> 
> I think it LGTM, but I'd like to confirm.

It's difficult to explain without seeing the two changes side-by-side, but
roughly this is what's going on in this CL:

All changes in spdy_framer.h are to make it closer to server version.  Server
change 78267288 does not in fact touch this file.

kMaxControlFrameSize is introduced in spdy_framer.cc to make it closer to server
version.  DCHECK in SpdyFramer constructor and current_frame_lenght_ check are
from the server change, but modified to Chromium's bounds which are slightly
different.

Introduction of sent_control_frame_max_size() in spdy_framer_test.cc is from the
server change.  However, kPartialPushPromiseFrameData had to be modified because
kMaxControlFrameSize is one more than GetHeaderFragmentMaxSize() was, this is
specific to Chromium.

Change in spdy_protocol.h is identical to server change.

[Ryan Hamilton, 2014-10-24 17:45:38]

thanks. lgtm

[Bence, 2014-10-24 17:53:31]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2014-10-24 17:54:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/675863002/1

[commit-bot: I haz the power, 2014-10-24 18:40:19]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-10-24 18:41:43]

Patchset 1 (id:??) landed as
https://crrev.com/0e2deda3f0c59c8fc18323ede48a552bef341c24
Cr-Commit-Position: refs/heads/master@{#301154}

[Ryan Hamilton, 2016-02-08 17:57:02]

Bence,

Alyssa was recently debugging some SPDY framing issues and noticed that Chrome
sends CONTINUATION frames quite regularly. Tracing through the code, this is
because of the 1KB limit on control frames in this CL. It seems like this limit
is probably too small. I think we should consider raising it. Perhaps by
consulting UMA for some data about common header block sizes.

[Bence, 2016-02-09 15:28:09]

I am not aware of any metrics on on header block size.  It's a shame that this
fragment limit is so low.

Note that it would kinda make sense to have the same fragment size for control
and data frames, because we fragment data to avoid head-of-line blocking.  The
current data fragment size is kMaxSpdyFrameChunkSize, about 3 kB, but that
should also be increased, see https://crbug.com/143193.

https://codereview.chromium.org/675863002/diff/1/net/spdy/spdy_framer.h
File net/spdy/spdy_framer.h (left):

https://codereview.chromium.org/675863002/diff/1/net/spdy/spdy_framer.h#oldco...
net/spdy/spdy_framer.h:731: // fragment at smaller payload boundaries.
Okay, so this was the original motivation for fragmenting at 1 kB.  I guess it
is reasonable now to raise it.

[Bence, 2016-02-09 23:30:16]

On 2016/02/09 15:28:09, Bence wrote:
> I am not aware of any metrics on on header block size.  It's a shame that this
> fragment limit is so low.
> 
> Note that it would kinda make sense to have the same fragment size for control
> and data frames, because we fragment data to avoid head-of-line blocking.  The
> current data fragment size is kMaxSpdyFrameChunkSize, about 3 kB, but that
> should also be increased, see https://crbug.com/143193.
> 
> https://codereview.chromium.org/675863002/diff/1/net/spdy/spdy_framer.h
> File net/spdy/spdy_framer.h (left):
> 
>
https://codereview.chromium.org/675863002/diff/1/net/spdy/spdy_framer.h#oldco...
> net/spdy/spdy_framer.h:731: // fragment at smaller payload boundaries.
> Okay, so this was the original motivation for fragmenting at 1 kB.  I guess it
> is reasonable now to raise it.

Fixed in https://crrev.com/1679043003.