[SPDY] Change SpdyStream::QueueStreamData() To SendStreamData()

[SPDY] Change SpdyStream::QueueStreamData() To SendStreamData()

Change its semantics so that it holds a reference to the passed-in data
and calls back to the delegate only when all the data has been sent.

Fix bug where flow control wouldn't work for non-HTTP streams. Now it
just queues up the next data frame (which must exist, since it must
have been trying to send something when it got stalled).

Remove now-redundant bytes_sent parameters from SpdyStream::Delegate
methods.

Remove now-redundant DrainableIOBuffer member variable in SpdyHttpStream.

Remove now-redundant logic in SpdyProxyClientSocket to split up data to
send into frames.

Add SpdyStream tests for sending large blobs of data.

Enable bidirectional flow control tests for SpdyStream.

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

[akalin, 2013-05-23 00:13:12]

+rch for review

[Ryan Hamilton, 2013-05-23 04:12:00]

lgtm with nits

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_proxy...
File net/spdy/spdy_proxy_client_socket.cc (left):

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_proxy...
net/spdy/spdy_proxy_client_socket.cc:241: // we need to send multiple data
frames
I gather we no longer need this logic because it's handled in
SpdyStream/SpdySession now/?

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_proxy...
File net/spdy/spdy_proxy_client_socket.cc (right):

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_proxy...
net/spdy/spdy_proxy_client_socket.cc:512: CompletionCallback c =
write_callback_;
nit: I think you can use ResetAndReturn() here.

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_strea...
File net/spdy/spdy_stream.cc (right):

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_strea...
net/spdy/spdy_stream.cc:957: pending_send_flags_ = DATA_FLAG_NONE;
It's a pity that lines 948-957 are almost completely duplicates of 907-917.  Can
you factor it out in some way?  What do you think?

[akalin, 2013-05-23 05:29:38]

committing soon

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_proxy...
File net/spdy/spdy_proxy_client_socket.cc (left):

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_proxy...
net/spdy/spdy_proxy_client_socket.cc:241: // we need to send multiple data
frames
On 2013/05/23 04:12:00, Ryan Hamilton wrote:
> I gather we no longer need this logic because it's handled in
> SpdyStream/SpdySession now/?

Yeap, that's right.

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_proxy...
File net/spdy/spdy_proxy_client_socket.cc (right):

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_proxy...
net/spdy/spdy_proxy_client_socket.cc:512: CompletionCallback c =
write_callback_;
On 2013/05/23 04:12:00, Ryan Hamilton wrote:
> nit: I think you can use ResetAndReturn() here.

Done.

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_strea...
File net/spdy/spdy_stream.cc (right):

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_strea...
net/spdy/spdy_stream.cc:957: pending_send_flags_ = DATA_FLAG_NONE;
On 2013/05/23 04:12:00, Ryan Hamilton wrote:
> It's a pity that lines 948-957 are almost completely duplicates of 907-917. 
Can
> you factor it out in some way?  What do you think?

I did think about this. The problem is, factoring it into a helper function now
would be a bit awkward (would have to check the return value of such a function,
it would non-obviously have to set io_state_, etc.). However, I am working
towards simplifying the state machine, so one of the copies might go away
eventually. If not, then I'll factor it out then.

[commit-bot: I haz the power, 2013-05-23 05:30:41]

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

[Ryan Hamilton, 2013-05-23 16:26:30]

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_strea...
File net/spdy/spdy_stream.cc (right):

https://chromiumcodereview.appspot.com/15740018/diff/6001/net/spdy/spdy_strea...
net/spdy/spdy_stream.cc:957: pending_send_flags_ = DATA_FLAG_NONE;
On 2013/05/23 05:29:38, akalin wrote:
> On 2013/05/23 04:12:00, Ryan Hamilton wrote:
> > It's a pity that lines 948-957 are almost completely duplicates of 907-917. 
> Can
> > you factor it out in some way?  What do you think?
> 
> I did think about this. The problem is, factoring it into a helper function
now
> would be a bit awkward (would have to check the return value of such a
function,
> it would non-obviously have to set io_state_, etc.). However, I am working
> towards simplifying the state machine, so one of the copies might go away
> eventually. If not, then I'll factor it out then.

Sounds great.  I thought about a helper that returned a bool and then ... and
then I decided that didn't seem awesome so I thought, "Perhaps Fred has a better
idea" :>  As long as you've thought about this, I'm happy.

[commit-bot: I haz the power, 2013-05-23 16:41:41]

Change committed as 201828