[Cronet] Make delaying sending request headers explicit in bidirectional stream

[Cronet]Make delaying sending request headers explicit in bidirectional stream

Always delaying sending request headers when
disableAutoFlush is not safe (in the case of bidirectional
streaming). Because server might be expecting request
headers before sending a response, while client might only
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell
net::BidirectionalStream when to delay sending request
headers and coalesce them with data frames in
SendData/SendvData.

BUG=599902
Committed: https://crrev.com/bcb0f86e685b9d603bebb97e9574e2e0e32948c2
Cr-Commit-Position: refs/heads/master@{#397567}

[xunjieli, 2016-05-19 22:32:37]

Description was changed from

==========
Make delay sending request headers explicit

BUG=
==========

to

==========
Make delaying sending request headers explicit

Delaying sending request headers when auto flush is 
disabled is not safe in the case of bidirectional 
streaming. Because server might be expecting request 
headers before sending a response, while client might only call
SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.
==========

[xunjieli, 2016-05-19 22:32:38]

xunjieli@chromium.org changed reviewers:
+ kapishnikov@chromium.org, mef@chromium.org

[xunjieli, 2016-05-19 22:32:56]

Description was changed from

==========
Make delaying sending request headers explicit

Delaying sending request headers when auto flush is 
disabled is not safe in the case of bidirectional 
streaming. Because server might be expecting request 
headers before sending a response, while client might only call
SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.
==========

to

==========
Make delaying sending request headers explicit

Delaying sending request headers when auto flush is 
disabled is not safe in the case of bidirectional 
streaming. Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.
==========

[xunjieli, 2016-05-19 22:41:21]

Hi Misha and Andrei, this is for internal bug: 28851164.
A bit more context. There were two things wrong with my previous CL.

#1. When disableAutoFlush is true, we can still call in
BidirectionStreamSpdyImpl.SendData if there is only one buffer. This lead to the
DCHECK failure in b/28851164. Getting rid of the DCHECK fixes this issue, and I
added a Java unit test for this case.

#2. When disableAutoFlush is true, we can still call in
BidirectionalStreamQuicImpl.SendData without sending request headers. However,
after talking to rocking@, I realized that always delaying sending request
headers when disableAutoFlush == true is not safe in bidirectional streaming
case. We decided to add an explicit flag, which should only be enabled when it
is safe to do (e.g. in the case of unary request, where client always writes the
request before hearing from the server)

We can chat more tomorrow. Thanks!

[xunjieli, 2016-05-19 22:43:47]

Description was changed from

==========
Make delaying sending request headers explicit

Delaying sending request headers when auto flush is 
disabled is not safe in the case of bidirectional 
streaming. Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.
==========

to

==========
Make delaying sending request headers explicit

Always delaying sending request headers when 
disableAutoFlush is not safe (in the case of bidirectional 
streaming). Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.
==========

[xunjieli, 2016-05-23 13:09:17]

Pinging Misha and Andrei! Any chance we can get this in this week's Dev release?
flush() isn't usable without the fix :(

[mef, 2016-05-23 14:46:40]

On 2016/05/23 13:09:17, xunjieli wrote:
> Pinging Misha and Andrei! Any chance we can get this in this week's Dev
release?
> flush() isn't usable without the fix :(

Sorry for the delay, looking now.

[mef, 2016-05-23 14:57:26]

Need crbug number.

Would it make sense to always delay sending headers if disableAutoFlash is true,
BUT allow the app to call 'flush()' before any writes() if headers need to be
send?

https://codereview.chromium.org/1992953004/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java
(right):

https://codereview.chromium.org/1992953004/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java:187:
public Builder delayHeadersUntilNextWrite(boolean delayHeadersUntilNextWrite) {
Does it have an effect if disableAutoFlush is false?

https://codereview.chromium.org/1992953004/diff/20001/components/cronet/andro...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(left):

https://codereview.chromium.org/1992953004/diff/20001/components/cronet/andro...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:226:
mRequestContext.getUrlRequestContextAdapter(), mDisableAutoFlush);
Is mDisableAutoFlush not used now?

https://codereview.chromium.org/1992953004/diff/20001/net/http/bidirectional_...
File net/http/bidirectional_stream.h (right):

https://codereview.chromium.org/1992953004/diff/20001/net/http/bidirectional_...
net/http/bidirectional_stream.h:100: bool delay_headers_until_next_send_data,
Um, shouldn't |disable_auto_flush| still be a thing?

[xunjieli, 2016-05-23 21:55:21]

Description was changed from

==========
Make delaying sending request headers explicit

Always delaying sending request headers when 
disableAutoFlush is not safe (in the case of bidirectional 
streaming). Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.
==========

to

==========
Make delaying sending request headers explicit

Always delaying sending request headers when 
disableAutoFlush is not safe (in the case of bidirectional 
streaming). Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.

BUG=599902
==========

[xunjieli, 2016-05-25 14:26:26]

Thanks Misha and Andrei for the suggestions. I believe I have modified the CL
according to what was discussed. Could you take a look? I will patch the CL and
see if this passes the internals tests in b 28851164.

[kapishnikov, 2016-05-31 18:42:12]

https://codereview.chromium.org/1992953004/diff/140001/components/cronet/andr...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/1992953004/diff/140001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:326:
}
Should we change mWriteState to State.WRITING_DONE if the http method is GET or
HEAD?

https://codereview.chromium.org/1992953004/diff/140001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:330:
assert !(mPendingData.isEmpty() && mFlushData.isEmpty());
Will it prevent calling flush() two times in a roll without calling write() in
between? Should we 'return' instead?

[xunjieli, 2016-05-31 19:31:53]

Thanks for the review! PTAL

https://codereview.chromium.org/1992953004/diff/140001/components/cronet/andr...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/1992953004/diff/140001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:326:
}
On 2016/05/31 18:42:12, kapishnikov wrote:
> Should we change mWriteState to State.WRITING_DONE if the http method is GET
or
> HEAD?

Done. Good catch!

https://codereview.chromium.org/1992953004/diff/140001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:330:
assert !(mPendingData.isEmpty() && mFlushData.isEmpty());
On 2016/05/31 18:42:12, kapishnikov wrote:
> Will it prevent calling flush() two times in a roll without calling write() in
> between? Should we 'return' instead?

We have an early return on line 327 to prevent calling flush() two times in a
roll.

[kapishnikov, 2016-05-31 19:47:24]

Looks good. LGTM

https://codereview.chromium.org/1992953004/diff/140001/components/cronet/andr...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/1992953004/diff/140001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:330:
assert !(mPendingData.isEmpty() && mFlushData.isEmpty());
On 2016/05/31 19:31:53, xunjieli wrote:
> On 2016/05/31 18:42:12, kapishnikov wrote:
> > Will it prevent calling flush() two times in a roll without calling write()
in
> > between? Should we 'return' instead?
> 
> We have an early return on line 327 to prevent calling flush() two times in a
> roll.

Acknowledged.

[mef, 2016-06-01 21:33:21]

Mostly nits and questions. I'm yet to review
bidirectional_stream_quic_impl_unittest.cc.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java:400:
* Flushes pending writes. This method should not be invoked before {@link
I think this deserves more verbose comment, explaining that onWriteCompleted
callbacks will be invoked for previously delayed writes or somesuch.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:715: *     
   headers until next flush(), and try to combine them with the next
I wonder whether it should be 'delayHeadersUntilFirstFlush'? I don't feel
strongly though.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File components/cronet/android/cronet_bidirectional_stream_adapter.h (right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/cronet_bidirectional_stream_adapter.h:93: // This
method cannot be called when |send_request_headers_automatically_| is
nit: maybe just describe scenario where this method could and should be used?

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:100:
private boolean mRequestHeadersFlushed;
nit: requestHeadersFlushed => requestHeadersSent to match 'sendRequestHeaders'
parameters.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:333:
assert !(mPendingData.isEmpty() && mFlushData.isEmpty());
nit: maybe restate as "assert !mPendingData.isEmpty() ||
!mFlushData.isEmpty();"?

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:361:
// Still waiting on write. This is just to have consistent
nit: on write -> for flush

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File
components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java:153:
boolean delayHeadersUntilNextWrite) {
nit: maybe delayHeadersUntilFirstFlush? In any case, name must be consistent
with CronetEngine.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File
components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamQuicTest.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamQuicTest.java:145:
for (int i = 0; i < 2; i++) {
FTR: Yeah, this is ugly hack.

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
File net/http/bidirectional_stream.cc (right):

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
net/http/bidirectional_stream.cc:135: DCHECK(!request_headers_sent_);
add DCHECK(!send_request_headers_automatically_);

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
File net/http/bidirectional_stream.h (right):

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
net/http/bidirectional_stream.h:101: // sent only if until SendRequestHeaders()
is invoked or until next
if until => when
until => with

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
File net/http/bidirectional_stream_unittest.cc (right):

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
net/http/bidirectional_stream_unittest.cc:120: /*ignored*/ true, this,
if it is /*ignored*/, why does it have to change to 'true'?

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
File net/quic/bidirectional_stream_quic_impl.cc (right):

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:83: http_request_info.method =
request_info_->method;
Not for this CL, but we may consider making BidirectionalStreamRequestInfo based
on HttpRequestInfo to avoid needless copy here.

Not sure how expensive is copying the content of HttpRequestHeaders.

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:89: size_t frame_len =
stream_->WriteHeaders(
nit: size_t frame_len => headers_bytes_sent_

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:274:
delegate_->OnStreamReady(send_request_headers_automatically_);
send_request_headers_automatically_ => has_sent_headers_

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
File net/quic/bidirectional_stream_quic_impl.h (right):

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.h:115: // If false, headers will be sent
until SendRequestHeaders() is called or
will be -> will not be

https://codereview.chromium.org/1992953004/diff/160001/net/spdy/bidirectional...
File net/spdy/bidirectional_stream_spdy_impl.cc (right):

https://codereview.chromium.org/1992953004/diff/160001/net/spdy/bidirectional...
net/spdy/bidirectional_stream_spdy_impl.cc:80: NOTREACHED();
can the app call this explicitly?

[xunjieli, 2016-06-01 22:24:55]

Patchset #10 (id:180001) has been deleted

[xunjieli, 2016-06-01 22:27:18]

Thanks for the review!

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java:400:
* Flushes pending writes. This method should not be invoked before {@link
On 2016/06/01 21:33:20, mef wrote:
> I think this deserves more verbose comment, explaining that onWriteCompleted
> callbacks will be invoked for previously delayed writes or somesuch.

Done.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:715: *     
   headers until next flush(), and try to combine them with the next
On 2016/06/01 21:33:20, mef wrote:
> I wonder whether it should be 'delayHeadersUntilFirstFlush'? I don't feel
> strongly though.

Done.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File components/cronet/android/cronet_bidirectional_stream_adapter.h (right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/cronet_bidirectional_stream_adapter.h:93: // This
method cannot be called when |send_request_headers_automatically_| is
On 2016/06/01 21:33:20, mef wrote:
> nit: maybe just describe scenario where this method could and should be used?

Done. I shifted things around. Hope it is clearer.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:100:
private boolean mRequestHeadersFlushed;
On 2016/06/01 21:33:21, mef wrote:
> nit: requestHeadersFlushed => requestHeadersSent to match 'sendRequestHeaders'
> parameters.

Done.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:333:
assert !(mPendingData.isEmpty() && mFlushData.isEmpty());
On 2016/06/01 21:33:20, mef wrote:
> nit: maybe restate as "assert !mPendingData.isEmpty() ||
> !mFlushData.isEmpty();"?

Done.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:361:
// Still waiting on write. This is just to have consistent
On 2016/06/01 21:33:21, mef wrote:
> nit: on write -> for flush

Done.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File
components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java:153:
boolean delayHeadersUntilNextWrite) {
On 2016/06/01 21:33:21, mef wrote:
> nit: maybe delayHeadersUntilFirstFlush? In any case, name must be consistent
> with CronetEngine.

Done.

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
File
components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamQuicTest.java
(right):

https://codereview.chromium.org/1992953004/diff/160001/components/cronet/andr...
components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamQuicTest.java:145:
for (int i = 0; i < 2; i++) {
On 2016/06/01 21:33:21, mef wrote:
> FTR: Yeah, this is ugly hack.

Acknowledged. But luckily there is a solution. I tried to change our test
fixture to use ParaterizedTest, but it turns out not to be trivial. I will
handle that in a follow-up.

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
File net/http/bidirectional_stream.cc (right):

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
net/http/bidirectional_stream.cc:135: DCHECK(!request_headers_sent_);
On 2016/06/01 21:33:21, mef wrote:
> add DCHECK(!send_request_headers_automatically_);

Done.

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
File net/http/bidirectional_stream.h (right):

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
net/http/bidirectional_stream.h:101: // sent only if until SendRequestHeaders()
is invoked or until next
On 2016/06/01 21:33:21, mef wrote:
> if until => when
> until => with

Done.

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
File net/http/bidirectional_stream_unittest.cc (right):

https://codereview.chromium.org/1992953004/diff/160001/net/http/bidirectional...
net/http/bidirectional_stream_unittest.cc:120: /*ignored*/ true, this,
On 2016/06/01 21:33:21, mef wrote:
> if it is /*ignored*/, why does it have to change to 'true'?

Done.

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
File net/quic/bidirectional_stream_quic_impl.cc (right):

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:83: http_request_info.method =
request_info_->method;
On 2016/06/01 21:33:21, mef wrote:
> Not for this CL, but we may consider making BidirectionalStreamRequestInfo
based
> on HttpRequestInfo to avoid needless copy here.
> 
> Not sure how expensive is copying the content of HttpRequestHeaders.

Acknowledged.

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:89: size_t frame_len =
stream_->WriteHeaders(
On 2016/06/01 21:33:21, mef wrote:
> nit: size_t frame_len => headers_bytes_sent_

Done.

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.cc:274:
delegate_->OnStreamReady(send_request_headers_automatically_);
On 2016/06/01 21:33:21, mef wrote:
> send_request_headers_automatically_ => has_sent_headers_

Done.

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
File net/quic/bidirectional_stream_quic_impl.h (right):

https://codereview.chromium.org/1992953004/diff/160001/net/quic/bidirectional...
net/quic/bidirectional_stream_quic_impl.h:115: // If false, headers will be sent
until SendRequestHeaders() is called or
On 2016/06/01 21:33:21, mef wrote:
> will be -> will not be

Done.

https://codereview.chromium.org/1992953004/diff/160001/net/spdy/bidirectional...
File net/spdy/bidirectional_stream_spdy_impl.cc (right):

https://codereview.chromium.org/1992953004/diff/160001/net/spdy/bidirectional...
net/spdy/bidirectional_stream_spdy_impl.cc:80: NOTREACHED();
On 2016/06/01 21:33:21, mef wrote:
> can the app call this explicitly?

The app shouldn't call this explicitly because OnStreamReady() will always be
invoked with request_headers_sent = true. It is illegal to call
SendRequestHeaders() since request headers have been sent.

[mef, 2016-06-02 20:58:46]

lgtm, thanks for your patience!

Maybe add [Cronet] and/or bidirectional stream to description?

[xunjieli, 2016-06-02 21:07:26]

Description was changed from

==========
Make delaying sending request headers explicit

Always delaying sending request headers when 
disableAutoFlush is not safe (in the case of bidirectional 
streaming). Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.

BUG=599902
==========

to

==========
[Cronet]Make delaying sending request headers explicit in bidirectional stream

Always delaying sending request headers when 
disableAutoFlush is not safe (in the case of bidirectional 
streaming). Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.

BUG=599902
==========

[xunjieli, 2016-06-02 21:12:32]

On 2016/06/02 20:58:46, mef wrote:
> lgtm, thanks for your patience!
> 
> Maybe add [Cronet] and/or bidirectional stream to description?

Done. Thanks!

[xunjieli, 2016-06-02 21:12:39]

The CQ bit was checked by xunjieli@chromium.org

[xunjieli, 2016-06-02 21:12:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
kapishnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/1992953004/#ps220001
(title: "create a typo")

[commit-bot: I haz the power, 2016-06-02 21:14:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992953004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992953004/220001

[commit-bot: I haz the power, 2016-06-03 00:49:40]

Description was changed from

==========
[Cronet]Make delaying sending request headers explicit in bidirectional stream

Always delaying sending request headers when 
disableAutoFlush is not safe (in the case of bidirectional 
streaming). Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.

BUG=599902
==========

to

==========
[Cronet]Make delaying sending request headers explicit in bidirectional stream

Always delaying sending request headers when 
disableAutoFlush is not safe (in the case of bidirectional 
streaming). Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.

BUG=599902
==========

[commit-bot: I haz the power, 2016-06-03 00:49:42]

Committed patchset #11 (id:220001)

[commit-bot: I haz the power, 2016-06-03 00:50:48]

Description was changed from

==========
[Cronet]Make delaying sending request headers explicit in bidirectional stream

Always delaying sending request headers when 
disableAutoFlush is not safe (in the case of bidirectional 
streaming). Because server might be expecting request 
headers before sending a response, while client might only 
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell 
net::BidirectionalStream when to delay sending request 
headers and coalesce them with data frames in 
SendData/SendvData.

BUG=599902
==========

to

==========
[Cronet]Make delaying sending request headers explicit in bidirectional stream

Always delaying sending request headers when
disableAutoFlush is not safe (in the case of bidirectional
streaming). Because server might be expecting request
headers before sending a response, while client might only
call SendData/SendvData after server responds.

This CL adds an explicit flag to tell
net::BidirectionalStream when to delay sending request
headers and coalesce them with data frames in
SendData/SendvData.

BUG=599902

Committed: https://crrev.com/bcb0f86e685b9d603bebb97e9574e2e0e32948c2
Cr-Commit-Position: refs/heads/master@{#397567}
==========

[commit-bot: I haz the power, 2016-06-03 00:50:49]

Patchset 11 (id:??) landed as
https://crrev.com/bcb0f86e685b9d603bebb97e9574e2e0e32948c2
Cr-Commit-Position: refs/heads/master@{#397567}