[Cronet] Don't send endOfStream if there is pending data to flush on …

…iOS. BUG=623988 Review-Url: https://codereview.chromium.org/2128633004 Cr-Commit-Position: refs/heads/master@{#404691}
Denger-Network · Jul 11, 2016 · a3e2f52 · a3e2f52
1 parent 20be8a5
commit a3e2f52
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/components/cronet/ios/cronet_bidirectional_stream.cc b/components/cronet/ios/cronet_bidirectional_stream.cc
@@ -220,9 +220,10 @@ void CronetBidirectionalStream::OnDataSent() {
     SendFlushingWriteData();
     return;
   }
-  if (write_end_of_stream_ && pending_write_data_->Empty())
+  if (write_end_of_stream_ && pending_write_data_->Empty()) {
     write_state_ = WRITING_DONE;
-  MaybeOnSucceded();
+    MaybeOnSucceded();
+  }
 }
 
 void CronetBidirectionalStream::OnTrailersReceived(
@@ -333,7 +334,8 @@ void CronetBidirectionalStream::SendFlushingWriteData() {
   write_state_ = WRITING;
   flushing_write_data_->MoveTo(sending_write_data_.get());
   bidi_stream_->SendvData(sending_write_data_->buffers(),
-                          sending_write_data_->lengths(), write_end_of_stream_);
+                          sending_write_data_->lengths(),
+                          write_end_of_stream_ && pending_write_data_->Empty());
 }
 
 void CronetBidirectionalStream::CancelOnNetworkThread() {

diff --git a/components/cronet/ios/test/cronet_bidirectional_stream_test.mm b/components/cronet/ios/test/cronet_bidirectional_stream_test.mm
@@ -438,6 +438,55 @@ static void on_canceled_callback(cronet_bidirectional_stream* stream) {
   cronet_bidirectional_stream_destroy(test.stream);
 }
 
+TEST_P(CronetBidirectionalStreamTest, TestDelayedFlush) {
+  class CustomTestBidirectionalStreamCallback
+      : public TestBidirectionalStreamCallback {
+    void MaybeWriteNextData(cronet_bidirectional_stream* stream) override {
+      DCHECK_EQ(stream, this->stream);
+      if (write_data.empty())
+        return;
+      // Write all buffers when stream is ready.
+      // Flush after "3" and "5".
+      // EndOfStream is set with "6" but not flushed, so it is not sent.
+      if (write_data.front()->buffer == "1") {
+        for (const auto& data : write_data) {
+          cronet_bidirectional_stream_write(stream, data->buffer.c_str(),
+                                            data->buffer.size(),
+                                            data == write_data.back());
+          if (data->flush) {
+            cronet_bidirectional_stream_flush(stream);
+          }
+        }
+      }
+      // Flush the final buffer with endOfStream flag.
+      if (write_data.front()->buffer == "6")
+        cronet_bidirectional_stream_flush(stream);
+    }
+  };
+
+  CustomTestBidirectionalStreamCallback test;
+  test.AddWriteData("1", false);
+  test.AddWriteData("2", false);
+  test.AddWriteData("3", true);
+  test.AddWriteData("4", false);
+  test.AddWriteData("5", true);
+  test.AddWriteData("6", false);
+  test.stream =
+      cronet_bidirectional_stream_create(engine(), &test, test.callback());
+  DCHECK(test.stream);
+  cronet_bidirectional_stream_disable_auto_flush(test.stream, true);
+  cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,
+                                                                GetParam());
+  // Flush before start is ignored.
+  cronet_bidirectional_stream_flush(test.stream);
+  cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",
+                                    &kTestHeadersArray, false);
+  test.BlockForDone();
+  // Flush after done is ignored.
+  cronet_bidirectional_stream_flush(test.stream);
+  cronet_bidirectional_stream_destroy(test.stream);
+}
+
 TEST_P(CronetBidirectionalStreamTest, CancelOnRead) {
   TestBidirectionalStreamCallback test;
   test.stream =