[SPDY] Refactor tests in preparation for a fix for a session flow control bug

[SPDY] Refactor tests in preparation for a fix for a session flow control bug

Add tests for the session flow control bug. They will be enabled when the
fix goes in.

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

[akalin, 2013-03-14 06:21:36]

+rch for review

I split this off into its own CL since it was getting too big. The CL with the
actual bug fix (itself a lot smaller) is coming soon.

[Ryan Hamilton, 2013-03-14 15:29:45]

I think this looks good, but I'm a bit confused by the SYN_STREAM GET w/ FIN
set..

https://codereview.chromium.org/12743006/diff/1/net/spdy/spdy_session_spdy3_u...
File net/spdy/spdy_session_spdy3_unittest.cc (right):

https://codereview.chromium.org/12743006/diff/1/net/spdy/spdy_session_spdy3_u...
net/spdy/spdy_session_spdy3_unittest.cc:45: // CONTROL_FLAG_NONE vs.
CONTROL_FLAG_FIN.)
Curious...  Isn't it illegal for a SYN_STREAM which encodes a GET to have the
fin bit unset?  (Since a GET can't have a body?)  From the SPDY/3 Spec:

For requests which do not contain a body, the SYN_STREAM frame MUST set the
FLAG_FIN, indicating that the client intends to send no further data on this
stream. 

I gather it is important that this request not have the fin bit set.  Should it
be a POST instead?  (I think we might have a helper method for that)

https://codereview.chromium.org/12743006/diff/1/net/spdy/spdy_test_util_spdy2.h
File net/spdy/spdy_test_util_spdy2.h (right):

https://codereview.chromium.org/12743006/diff/1/net/spdy/spdy_test_util_spdy2...
net/spdy/spdy_test_util_spdy2.h:118: SpdyFrame* ConstructSpdyPacket(const
SpdyHeaderInfo& header_info,
Yak: Can you rename this to ConstructSpdyFrame (or some such)?  I don't think
there is any such thing as a spdy packet.

[akalin, 2013-03-15 17:28:56]

PTAL

https://codereview.chromium.org/12743006/diff/1/net/spdy/spdy_session_spdy3_u...
File net/spdy/spdy_session_spdy3_unittest.cc (right):

https://codereview.chromium.org/12743006/diff/1/net/spdy/spdy_session_spdy3_u...
net/spdy/spdy_session_spdy3_unittest.cc:45: // CONTROL_FLAG_NONE vs.
CONTROL_FLAG_FIN.)
On 2013/03/14 15:29:45, Ryan Hamilton wrote:
> Curious...  Isn't it illegal for a SYN_STREAM which encodes a GET to have the
> fin bit unset?  (Since a GET can't have a body?)  From the SPDY/3 Spec:
> 
> For requests which do not contain a body, the SYN_STREAM frame MUST set the
> FLAG_FIN, indicating that the client intends to send no further data on this
> stream. 
> 
> I gather it is important that this request not have the fin bit set.  Should
it
> be a POST instead?  (I think we might have a helper method for that)

Okay, converted these tests to use posts and added the necessary util functions.
Also removed these ad-hoc util functions.

https://codereview.chromium.org/12743006/diff/1/net/spdy/spdy_test_util_spdy2.h
File net/spdy/spdy_test_util_spdy2.h (right):

https://codereview.chromium.org/12743006/diff/1/net/spdy/spdy_test_util_spdy2...
net/spdy/spdy_test_util_spdy2.h:118: SpdyFrame* ConstructSpdyPacket(const
SpdyHeaderInfo& header_info,
On 2013/03/14 15:29:45, Ryan Hamilton wrote:
> Yak: Can you rename this to ConstructSpdyFrame (or some such)?  I don't think
> there is any such thing as a spdy packet.

Done. search-and-replaced packet with frame in a bunch of files.

[akalin, 2013-03-18 19:25:04]

ping!

[Ryan Hamilton, 2013-03-18 19:29:23]

lgtm

https://codereview.chromium.org/12743006/diff/6001/net/spdy/spdy_network_tran...
File net/spdy/spdy_network_transaction_spdy2_unittest.cc (right):

https://codereview.chromium.org/12743006/diff/6001/net/spdy/spdy_network_tran...
net/spdy/spdy_network_transaction_spdy2_unittest.cc:41: const char kStreamUrl[]
= "http://www.google.com/";
nit: kRequestUrl?

[akalin, 2013-03-18 21:38:08]

committing via commit-bot

https://codereview.chromium.org/12743006/diff/6001/net/spdy/spdy_network_tran...
File net/spdy/spdy_network_transaction_spdy2_unittest.cc (right):

https://codereview.chromium.org/12743006/diff/6001/net/spdy/spdy_network_tran...
net/spdy/spdy_network_transaction_spdy2_unittest.cc:41: const char kStreamUrl[]
= "http://www.google.com/";
On 2013/03/18 19:29:23, Ryan Hamilton wrote:
> nit: kRequestUrl?

Done.

[commit-bot: I haz the power, 2013-03-18 21:42:35]

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

[akalin, 2013-03-18 23:28:04]

Committed patchset #3 manually as r188878 (presubmit successful).