net: Give more descriptive names for code around the request merging logic.

This is a follow-up patch based on wtc's comments on the earlier patch:
http://codereview.chromium.org/9270030/

BUG=72001
TEST=no logic change


Review URL: http://codereview.chromium.org/9284033

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@118942 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 33 insertions, 21 deletions

diff --git a/net/base/upload_data.cc b/net/base/upload_data.cc
index 019254e..01eaaca 100644
--- a/net/base/upload_data.cc
+++ b/net/base/upload_data.cc
@@ -178,7 +178,11 @@ uint64 UploadData::GetContentLength() {
 }
 
 bool UploadData::IsInMemory() const {
-  // Chunks are provided as a stream, hence it's not in memory.
+  // Chunks are in memory, but UploadData does not have all the chunks at
+  // once. Chunks are provided progressively with AppendChunk() as chunks
+  // are ready. Check is_chunked_ here, rather than relying on the loop
+  // below, as there is a case that is_chunked_ is set to true, but the
+  // first chunk is not yet delivered.
   if (is_chunked_)
     return false;
 
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 7966889..be1c828 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -139,7 +139,7 @@ int HttpStreamParser::SendRequest(const std::string& request_line,
   // If we have a small request body, then we'll merge with the headers into a
   // single write.
   bool did_merge = false;
-  if (ShouldMerge(request, request_body_.get())) {
+  if (ShouldMergeRequestHeadersAndBody(request, request_body_.get())) {
     size_t merged_size = request.size() + request_body->size();
     scoped_refptr<IOBuffer> merged_request_headers_and_body(
         new IOBuffer(merged_size));
@@ -811,13 +811,14 @@ int HttpStreamParser::EncodeChunk(const base::StringPiece& payload,
 }
 
 // static
-bool HttpStreamParser::ShouldMerge(const std::string& request,
-                                   const UploadDataStream* request_body) {
+bool HttpStreamParser::ShouldMergeRequestHeadersAndBody(
+    const std::string& request_headers,
+    const UploadDataStream* request_body) {
   if (request_body != NULL &&
       // IsInMemory() ensures that the request body is not chunked.
       request_body->IsInMemory() &&
       request_body->size() > 0) {
-    size_t merged_size = request.size() + request_body->size();
+    size_t merged_size = request_headers.size() + request_body->size();
     if (merged_size <= kMaxMergedHeaderAndBodySize)
       return true;
   }
diff --git a/net/http/http_stream_parser.h b/net/http/http_stream_parser.h
index b87beab..303bc10 100644
--- a/net/http/http_stream_parser.h
+++ b/net/http/http_stream_parser.h
@@ -91,10 +91,11 @@ class NET_EXPORT_PRIVATE HttpStreamParser  : public ChunkCallback {
                          char* output,
                          size_t output_size);
 
-  // Returns true if request and body should be merged (i.e. the sum is
-  // small enough and the body is in memory, and not chunked).
-  static bool ShouldMerge(const std::string& request,
-                          const UploadDataStream* request_body);
+  // Returns true if request headers and body should be merged (i.e. the
+  // sum is small enough and the body is in memory, and not chunked).
+  static bool ShouldMergeRequestHeadersAndBody(
+      const std::string& request_headers,
+      const UploadDataStream* request_body);
 
   // The number of extra bytes required to encode a chunk.
   static const size_t kChunkHeaderFooterSize;
diff --git a/net/http/http_stream_parser_unittest.cc b/net/http/http_stream_parser_unittest.cc
index 1e96038..808a3e3 100644
--- a/net/http/http_stream_parser_unittest.cc
+++ b/net/http/http_stream_parser_unittest.cc
@@ -79,20 +79,22 @@ TEST(HttpStreamParser, EncodeChunk_TooLargePayload) {
   ASSERT_EQ(ERR_INVALID_ARGUMENT, num_bytes_written);
 }
 
-TEST(HttpStreamParser, ShouldMerge_NoBody) {
+TEST(HttpStreamParser, ShouldMergeRequestHeadersAndBody_NoBody) {
   // Shouldn't be merged if upload data is non-existent.
-  ASSERT_FALSE(HttpStreamParser::ShouldMerge("some header", NULL));
+  ASSERT_FALSE(HttpStreamParser::ShouldMergeRequestHeadersAndBody(
+      "some header", NULL));
 }
 
-TEST(HttpStreamParser, ShouldMerge_EmptyBody) {
+TEST(HttpStreamParser, ShouldMergeRequestHeadersAndBody_EmptyBody) {
   scoped_refptr<UploadData> upload_data = new UploadData;
   scoped_ptr<UploadDataStream> body(
       UploadDataStream::Create(upload_data.get(), NULL));
   // Shouldn't be merged if upload data is empty.
-  ASSERT_FALSE(HttpStreamParser::ShouldMerge("some header", body.get()));
+  ASSERT_FALSE(HttpStreamParser::ShouldMergeRequestHeadersAndBody(
+      "some header", body.get()));
 }
 
-TEST(HttpStreamParser, ShouldMerge_ChunkedBody) {
+TEST(HttpStreamParser, ShouldMergeRequestHeadersAndBody_ChunkedBody) {
   scoped_refptr<UploadData> upload_data = new UploadData;
   upload_data->set_is_chunked(true);
   const std::string payload = "123";
@@ -101,10 +103,11 @@ TEST(HttpStreamParser, ShouldMerge_ChunkedBody) {
   scoped_ptr<UploadDataStream> body(
       UploadDataStream::Create(upload_data.get(), NULL));
   // Shouldn't be merged if upload data carries chunked data.
-  ASSERT_FALSE(HttpStreamParser::ShouldMerge("some header", body.get()));
+  ASSERT_FALSE(HttpStreamParser::ShouldMergeRequestHeadersAndBody(
+      "some header", body.get()));
 }
 
-TEST(HttpStreamParser, ShouldMerge_FileBody) {
+TEST(HttpStreamParser, ShouldMergeRequestHeadersAndBody_FileBody) {
   scoped_refptr<UploadData> upload_data = new UploadData;
 
   // Create an empty temporary file.
@@ -119,10 +122,11 @@ TEST(HttpStreamParser, ShouldMerge_FileBody) {
   scoped_ptr<UploadDataStream> body(
       UploadDataStream::Create(upload_data.get(), NULL));
   // Shouldn't be merged if upload data carries a file, as it's not in-memory.
-  ASSERT_FALSE(HttpStreamParser::ShouldMerge("some header", body.get()));
+  ASSERT_FALSE(HttpStreamParser::ShouldMergeRequestHeadersAndBody(
+      "some header", body.get()));
 }
 
-TEST(HttpStreamParser, ShouldMerge_SmallBodyInMemory) {
+TEST(HttpStreamParser, ShouldMergeRequestHeadersAndBody_SmallBodyInMemory) {
   scoped_refptr<UploadData> upload_data = new UploadData;
   const std::string payload = "123";
   upload_data->AppendBytes(payload.data(), payload.size());
@@ -130,10 +134,11 @@ TEST(HttpStreamParser, ShouldMerge_SmallBodyInMemory) {
   scoped_ptr<UploadDataStream> body(
       UploadDataStream::Create(upload_data.get(), NULL));
   // Yes, should be merged if the in-memory body is small here.
-  ASSERT_TRUE(HttpStreamParser::ShouldMerge("some header", body.get()));
+  ASSERT_TRUE(HttpStreamParser::ShouldMergeRequestHeadersAndBody(
+      "some header", body.get()));
 }
 
-TEST(HttpStreamParser, ShouldMerge_LargeBodyInMemory) {
+TEST(HttpStreamParser, ShouldMergeRequestHeadersAndBody_LargeBodyInMemory) {
   scoped_refptr<UploadData> upload_data = new UploadData;
   const std::string payload(10000, 'a');  // 'a' x 10000.
   upload_data->AppendBytes(payload.data(), payload.size());
@@ -141,7 +146,8 @@ TEST(HttpStreamParser, ShouldMerge_LargeBodyInMemory) {
   scoped_ptr<UploadDataStream> body(
       UploadDataStream::Create(upload_data.get(), NULL));
   // Shouldn't be merged if the in-memory body is large here.
-  ASSERT_FALSE(HttpStreamParser::ShouldMerge("some header", body.get()));
+  ASSERT_FALSE(HttpStreamParser::ShouldMergeRequestHeadersAndBody(
+      "some header", body.get()));
 }
 
 }  // namespace net