Allow SPDY POSTs to be terminated by an empty FIN data frame.

Allow chunked SPDY POSTs to be terminated by an empty FIN data frame.

This allows embedders to send a chunk when they do not yet know if it is
the final chunk or not.

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

[mmenke, 2014-08-21 15:40:25]

Hrm...was expecting this to be a little more complicated.

https://codereview.chromium.org/492063002/diff/1/net/spdy/spdy_http_stream_un...
File net/spdy/spdy_http_stream_unittest.cc (right):

https://codereview.chromium.org/492063002/diff/1/net/spdy/spdy_http_stream_un...
net/spdy/spdy_http_stream_unittest.cc:577: // Unclear if this is a case worth
supporting.
Not sure if we should really bother with this test.  My feeling is that we
should either have a story that explicitly disallows it, or not crash when it
happens.  Opted for the latter, but I'm fine with the former.

[Johnny, 2014-08-21 16:16:58]

lgtm

https://codereview.chromium.org/492063002/diff/1/net/spdy/spdy_http_stream_un...
File net/spdy/spdy_http_stream_unittest.cc (right):

https://codereview.chromium.org/492063002/diff/1/net/spdy/spdy_http_stream_un...
net/spdy/spdy_http_stream_unittest.cc:576: // Test that the SpdyStream state
machine can an chunked upload with no payload.
"can an chunked" => "handles a chunked"?

https://codereview.chromium.org/492063002/diff/1/net/spdy/spdy_http_stream_un...
net/spdy/spdy_http_stream_unittest.cc:577: // Unclear if this is a case worth
supporting.
On 2014/08/21 15:40:24, mmenke wrote:
> Not sure if we should really bother with this test.  My feeling is that we
> should either have a story that explicitly disallows it, or not crash when it
> happens.  Opted for the latter, but I'm fine with the former.

It's allowed at the protocol level, and I can imagine some utility (speculative
requests where the upload body isn't yet known). I think this what you have here
is good, but could happily go either way.

[mmenke, 2014-08-21 16:22:04]

Thanks!

https://codereview.chromium.org/492063002/diff/1/net/spdy/spdy_http_stream_un...
File net/spdy/spdy_http_stream_unittest.cc (right):

https://codereview.chromium.org/492063002/diff/1/net/spdy/spdy_http_stream_un...
net/spdy/spdy_http_stream_unittest.cc:576: // Test that the SpdyStream state
machine can an chunked upload with no payload.
On 2014/08/21 16:16:58, Johnny wrote:
> "can an chunked" => "handles a chunked"?

Oops.  Done.

[mmenke, 2014-08-21 16:22:46]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2014-08-21 16:25:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mmenke@chromium.org/492063002/20001

[commit-bot: I haz the power, 2014-08-21 21:33:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-21 22:58:12]

Committed patchset #2 (20001) as 291222

[mef, 2014-08-26 12:58:41]

mef@chromium.org changed reviewers:
+ mef@chromium.org

[mef, 2014-08-26 12:58:41]

https://codereview.chromium.org/492063002/diff/20001/net/spdy/spdy_http_strea...
File net/spdy/spdy_http_stream.cc (right):

https://codereview.chromium.org/492063002/diff/20001/net/spdy/spdy_http_strea...
net/spdy/spdy_http_stream.cc:427: // Only the final fame may have a length of 0.
nit: fame -> frame. :)

https://codereview.chromium.org/492063002/diff/20001/net/spdy/spdy_stream.cc
File net/spdy/spdy_stream.cc (right):

https://codereview.chromium.org/492063002/diff/20001/net/spdy/spdy_stream.cc#...
net/spdy/spdy_stream.cc:797: // Only the final fame may have a length of 0.
nit: fame -> frame.