Introduce error handling in SpdyHttpStream on UploadDataStream::Read() failure.

net/spdy/spdy_http_stream_unittest.cc

Index: net/spdy/spdy_http_stream_unittest.cc
diff --git a/net/spdy/spdy_http_stream_unittest.cc b/net/spdy/spdy_http_stream_unittest.cc
index a6d4a12abb1c652b04ae657ca56f87e73508d9c7..83c29039e5253447d703e540adac192a381ac9bd 100644
--- a/net/spdy/spdy_http_stream_unittest.cc
+++ b/net/spdy/spdy_http_stream_unittest.cc
@@ -75,6 +75,39 @@ void TestLoadTimingNotReused(const HttpStream& stream) {
   ExpectLoadTimingHasOnlyConnectionTimes(load_timing_info);
 }
 
+class ReadErrorUploadDataStream : public UploadDataStream {
+ public:
+  enum class FailureMode { SYNC, ASYNC };
+
+  explicit ReadErrorUploadDataStream(FailureMode mode)
+      : UploadDataStream(true, 0), async_(mode), weak_factory_(this) {}
+
+ private:
+  void CompleteRead() { UploadDataStream::OnReadCompleted(ERR_FAILED); }
+
+  // UploadDataStream implementation:
+  int InitInternal() override { return OK; }
+
+  int ReadInternal(IOBuffer* buf, int buf_len) override {
+    if (async_ == FailureMode::ASYNC) {
+      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+          FROM_HERE, base::Bind(&ReadErrorUploadDataStream::CompleteRead,
+                                weak_factory_.GetWeakPtr()),
+          base::TimeDelta::FromSeconds(1));
+      return ERR_IO_PENDING;
+    }
+    return ERR_FAILED;
+  }
+
+  void ResetInternal() override {}
+
+  const FailureMode async_;
+
+  base::WeakPtrFactory<ReadErrorUploadDataStream> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ReadErrorUploadDataStream);
+};
+
 }  // namespace
 
 class SpdyHttpStreamTest : public testing::Test,
@@ -438,7 +471,9 @@ TEST_P(SpdyHttpStreamTest, ConnectionClosedDuringChunkedPost) {
             http_stream.SendRequest(headers, &response, callback.callback()));
   EXPECT_TRUE(HasSpdySession(http_session_->spdy_session_pool(), key));
 
-  EXPECT_EQ(OK, callback.WaitForResult());
+  // Server closes the connection. Thus, callback now returns
+  // "CONNECTION_CLOSED" error.
+  EXPECT_EQ(ERR_CONNECTION_CLOSED, callback.WaitForResult());
 
   EXPECT_EQ(static_cast<int64_t>(req->size() + body->size()),
             http_stream.GetTotalSentBytes());
@@ -888,6 +923,116 @@ TEST_P(SpdyHttpStreamTest, DelayedSendChunkedPostWithWindowUpdate) {
   ASSERT_EQ(200, response.headers->response_code());
 }
 
+TEST_P(SpdyHttpStreamTest, DataReadErrorSynchronous) {
+  BufferedSpdyFramer framer(spdy_util_.spdy_version());
+
+  std::unique_ptr<SpdySerializedFrame> req(
+      spdy_util_.ConstructChunkedSpdyPost(NULL, 0));

[mmenke, 2016/06/01 17:24:07]

Note that my comments apply to both test.

nit:  nullptr.

+  std::unique_ptr<SpdySerializedFrame> rst_frame(
+      framer.CreateRstStream(1, RST_STREAM_INTERNAL_ERROR));

[mmenke, 2016/06/01 17:24:07]

This should probably use spdy_util_.ConstructSpdyFrame, to match how the request
is created.

[maksims (do not use this acc), 2016/06/03 11:39:08]

spdy_util_.ConstructSpdyRstStream
does the great job. Thanks.

+  MockWrite writes[] = {
+      CreateMockWrite(*req, 0, SYNCHRONOUS),       // request
+      CreateMockWrite(*rst_frame, 1, SYNCHRONOUS)  // POST upload frame
+  };
+
+  std::unique_ptr<SpdySerializedFrame> resp(
+      spdy_util_.ConstructSpdyPostSynReply(NULL, 0));
+  MockRead reads[] = {
+      CreateMockRead(*resp, 2), CreateMockRead(*rst_frame, 3),

[mmenke, 2016/06/01 17:24:07]

Are these frames needed?

[maksims (do not use this acc), 2016/06/02 12:43:12]

Otherwise I got !AllReadDataConsumed failures.

[mmenke, 2016/06/02 15:02:10]

Even if you just have the "MockRead(SYNCHRONOUS, 0, 2)" as the only read here?

[maksims (do not use this acc), 2016/06/03 11:39:08]

Oh, I got it. the resp frame is needed, but the rst_frame is not needed.

+      MockRead(SYNCHRONOUS, 0, 4),
+  };
+
+  HostPortPair host_port_pair("www.example.org", 80);
+  SpdySessionKey key(host_port_pair, ProxyServer::Direct(),
+                     PRIVACY_MODE_DISABLED);
+  InitSession(reads, arraysize(reads), writes, arraysize(writes), key);
+  EXPECT_EQ(spdy_util_.spdy_version(), session_->GetProtocolVersion());
+  ReadErrorUploadDataStream upload_data_stream(
+      ReadErrorUploadDataStream::FailureMode::SYNC);
+  ASSERT_EQ(OK, upload_data_stream.Init(TestCompletionCallback().callback()));
+
+  HttpRequestInfo request;
+  request.method = "POST";
+  request.url = GURL("http://www.example.org/");
+  request.upload_data_stream = &upload_data_stream;
+
+  TestCompletionCallback callback;
+  HttpResponseInfo response;
+  HttpRequestHeaders headers;
+  BoundNetLog net_log;
+  SpdyHttpStream http_stream(session_, true);
+  ASSERT_EQ(OK, http_stream.InitializeStream(&request, DEFAULT_PRIORITY,
+                                             net_log, CompletionCallback()));
+
+  int result = http_stream.SendRequest(headers, &response, callback.callback());
+  EXPECT_EQ(ERR_FAILED, callback.GetResult(result));
+
+  // Because the server has not closed the connection yet, there shouldn't be
+  // a stream but a session in the pool
+  EXPECT_FALSE(HasSpdySession(http_session_->spdy_session_pool(), key));
+}
+
+TEST_P(SpdyHttpStreamTest, DataReadErrorAsynchronous) {
+  BufferedSpdyFramer framer(spdy_util_.spdy_version());
+
+  std::unique_ptr<SpdySerializedFrame> req(
+      spdy_util_.ConstructChunkedSpdyPost(NULL, 0));
+
+  // Server receives RST_STREAM_INTERNAL_ERROR on clients an internal failure.
+  // The failure is reading error in this case cause by
+  // UploadDataStream::Read()
+  std::unique_ptr<SpdySerializedFrame> rst_frame(
+      framer.CreateRstStream(1, RST_STREAM_INTERNAL_ERROR));
+  MockWrite writes[] = {
+      CreateMockWrite(*req, 0),       // Request
+      CreateMockWrite(*rst_frame, 1)  // Reset frame
+  };
+
+  std::unique_ptr<SpdySerializedFrame> resp(
+      spdy_util_.ConstructSpdyPostSynReply(NULL, 0));
+  MockRead reads[] = {
+      CreateMockRead(*resp, 2),
+      CreateMockRead(*rst_frame, 3),
+      MockRead(ASYNC, 0, 4),
+  };
+
+  HostPortPair host_port_pair("www.example.org", 80);
+  SpdySessionKey key(host_port_pair, ProxyServer::Direct(),
+                     PRIVACY_MODE_DISABLED);
+  InitSession(reads, arraysize(reads), writes, arraysize(writes), key);
+  EXPECT_EQ(spdy_util_.spdy_version(), session_->GetProtocolVersion());
+
+  ReadErrorUploadDataStream upload_data_stream(
+      ReadErrorUploadDataStream::FailureMode::ASYNC);
+  ASSERT_EQ(OK, upload_data_stream.Init(TestCompletionCallback().callback()));
+
+  HttpRequestInfo request;
+  request.method = "POST";
+  request.url = GURL("http://www.example.org/");
+  request.upload_data_stream = &upload_data_stream;
+
+  TestCompletionCallback callback;
+  HttpResponseInfo response;
+  HttpRequestHeaders headers;
+  BoundNetLog net_log;
+  SpdyHttpStream http_stream(session_, true);
+  ASSERT_EQ(OK, http_stream.InitializeStream(&request, DEFAULT_PRIORITY,
+                                             net_log, CompletionCallback()));
+
+  int result = http_stream.SendRequest(headers, &response, callback.callback());
+  EXPECT_EQ(ERR_IO_PENDING, result);
+
+  // Headers sent
+  EXPECT_EQ(OK, callback.GetResult(result));
+
+  // Body read failed
+  EXPECT_EQ(ERR_FAILED, callback.GetResult(result));
+
+  // Because the server has closed the connection, there shouldn't be a session
+  // in the pool anymore.
+  EXPECT_FALSE(HasSpdySession(http_session_->spdy_session_pool(), key));
+}
+
 // TODO(willchan): Write a longer test for SpdyStream that exercises all
 // methods.