Make URLRequestFileDirJob::FillReadBuffer() return an int indicating …

…the result

BUG=586393

URLRequestFileDirJob::FillReadBuffer() to return bytes reads if it really reads data.
If list_complete, returns list_complete_result_, and otherwise ERR_IO_PENDING.

Review URL: https://codereview.chromium.org/1702713004

Cr-Commit-Position: refs/heads/master@{#377689}

chrome/browser/file_select_helper.cc

@@ -15,6 +15,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/worker_pool.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/platform_util.h"

chrome/browser/file_select_helper.h

@@ -12,7 +12,6 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/threading/worker_pool.h"
 #include "build/build_config.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"

net/url_request/url_request_file_dir_job.cc

@@ -76,18 +76,16 @@ void URLRequestFileDirJob::Kill() {
 }
 
 int URLRequestFileDirJob::ReadRawData(IOBuffer* buf, int buf_size) {
-  int bytes_read = 0;
-  if (FillReadBuffer(buf->data(), buf_size, &bytes_read)) {
-    if (bytes_read == 0 && list_complete_)
-      return list_complete_result_;
-    return bytes_read;
+  int result = ReadBuffer(buf->data(), buf_size);
+  if (result == ERR_IO_PENDING) {
+    // We are waiting for more data
+    read_pending_ = true;
+    read_buffer_ = buf;
+    read_buffer_length_ = buf_size;
+    return ERR_IO_PENDING;
   }
 
-  // We are waiting for more data
-  read_pending_ = true;
-  read_buffer_ = buf;
-  read_buffer_length_ = buf_size;
-  return ERR_IO_PENDING;
+  return result;
 }
 
 bool URLRequestFileDirJob::GetMimeType(std::string* mime_type) const {
@@ -160,10 +158,8 @@ void URLRequestFileDirJob::CompleteRead(Error error) {
 
   int result = error;
   if (error == OK) {
-    int filled_bytes = 0;
-    if (FillReadBuffer(read_buffer_->data(), read_buffer_length_,
-                       &filled_bytes)) {
-      result = filled_bytes;
+    result = ReadBuffer(read_buffer_->data(), read_buffer_length_);
+    if (result >= 0) {
       // We completed the read, so reset the read buffer.
       read_buffer_ = NULL;
       read_buffer_length_ = 0;
@@ -178,23 +174,17 @@ void URLRequestFileDirJob::CompleteRead(Error error) {
   ReadRawDataComplete(result);
 }
 
-bool URLRequestFileDirJob::FillReadBuffer(char* buf, int buf_size,
-                                          int* bytes_read) {
-  DCHECK(bytes_read);
-
-  *bytes_read = 0;
-
+int URLRequestFileDirJob::ReadBuffer(char* buf, int buf_size) {
   int count = std::min(buf_size, static_cast<int>(data_.size()));
   if (count) {
     memcpy(buf, &data_[0], count);
     data_.erase(0, count);
-    *bytes_read = count;
-    return true;
+    return count;
   } else if (list_complete_) {
     // EOF
-    return true;
+    return list_complete_result_;
   }
-  return false;
+  return ERR_IO_PENDING;
 }
 
 }  // namespace net

net/url_request/url_request_file_dir_job.h

@@ -55,8 +55,7 @@ class NET_EXPORT_PRIVATE URLRequestFileDirJob
   // appropriately.
   void CompleteRead(Error error);
 
-  // Fills a buffer with the output.
-  bool FillReadBuffer(char* buf, int buf_size, int* bytes_read);
+  int ReadBuffer(char* buf, int buf_size);
 
   DirectoryLister lister_;
   base::FilePath dir_path_;

net/url_request/url_request_file_dir_job_unittest.cc

@@ -7,6 +7,7 @@
 #include <string>
 
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -128,6 +129,124 @@ TEST_F(URLRequestFileDirTest, ListCompletionOnNoPending) {
   EXPECT_EQ(ERR_FILE_NOT_FOUND, request->status().ToNetError());
 }
 
+// Test the case where reading the response completes synchronously.
+TEST_F(URLRequestFileDirTest, DirectoryWithASingleFileSync) {
+  base::ScopedTempDir directory;
+  ASSERT_TRUE(directory.CreateUniqueTempDir());
+  base::FilePath path;
+  base::CreateTemporaryFileInDir(directory.path(), &path);
+
+  TestJobFactory factory(directory.path());
+  context_.set_job_factory(&factory);
+
+  scoped_ptr<URLRequest> request(context_.CreateRequest(
+      FilePathToFileURL(path), DEFAULT_PRIORITY, &delegate_));
+  request->Start();
+  EXPECT_TRUE(request->is_pending());
+
+  // Since the DirectoryLister is running on the network thread, this will spin
+  // the message loop until the URLRequetsFileDirJob has received the
+  // entire directory listing and cached it.
+  base::RunLoop().RunUntilIdle();
+
+  int bytes_read = 0;
+  // This will complete synchronously, since the URLRequetsFileDirJob had
+  // directory listing cached in memory.
+  EXPECT_TRUE(request->Read(buffer_.get(), kBufferSize, &bytes_read));
+
+  EXPECT_EQ(URLRequestStatus::SUCCESS, request->status().status());
+
+  ASSERT_GT(bytes_read, 0);
+  ASSERT_LE(bytes_read, kBufferSize);
+  std::string data(buffer_->data(), bytes_read);
+  EXPECT_TRUE(data.find(directory.path().BaseName().MaybeAsASCII()) !=
+              std::string::npos);
+  EXPECT_TRUE(data.find(path.BaseName().MaybeAsASCII()) != std::string::npos);
+}
+
+// Test the case where reading the response completes asynchronously.
+TEST_F(URLRequestFileDirTest, DirectoryWithASingleFileAsync) {
+  base::ScopedTempDir directory;
+  ASSERT_TRUE(directory.CreateUniqueTempDir());
+  base::FilePath path;
+  base::CreateTemporaryFileInDir(directory.path(), &path);
+
+  TestJobFactory factory(directory.path());
+  context_.set_job_factory(&factory);
+
+  TestDelegate delegate;
+  scoped_ptr<URLRequest> request(context_.CreateRequest(
+      FilePathToFileURL(path), DEFAULT_PRIORITY, &delegate));
+  request->Start();
+  EXPECT_TRUE(request->is_pending());
+
+  base::RunLoop().Run();
+
+  ASSERT_GT(delegate.bytes_received(), 0);
+  ASSERT_LE(delegate.bytes_received(), kBufferSize);
+  EXPECT_TRUE(delegate.data_received().find(
+                  directory.path().BaseName().MaybeAsASCII()) !=
+              std::string::npos);
+  EXPECT_TRUE(delegate.data_received().find(path.BaseName().MaybeAsASCII()) !=
+              std::string::npos);
+}
+
+TEST_F(URLRequestFileDirTest, DirectoryWithAFileAndSubdirectory) {
+  base::ScopedTempDir directory;
+  ASSERT_TRUE(directory.CreateUniqueTempDir());
+
+  base::FilePath sub_dir;
+  CreateTemporaryDirInDir(directory.path(),
+                          FILE_PATH_LITERAL("CreateNewSubDirectoryInDirectory"),
+                          &sub_dir);
+
+  base::FilePath path;
+  base::CreateTemporaryFileInDir(directory.path(), &path);
+
+  TestJobFactory factory(directory.path());
+  context_.set_job_factory(&factory);
+
+  TestDelegate delegate;
+  scoped_ptr<URLRequest> request(context_.CreateRequest(
+      FilePathToFileURL(path), DEFAULT_PRIORITY, &delegate));
+  request->Start();
+  EXPECT_TRUE(request->is_pending());
+
+  base::RunLoop().Run();
+
+  ASSERT_GT(delegate.bytes_received(), 0);
+  ASSERT_LE(delegate.bytes_received(), kBufferSize);
+  EXPECT_TRUE(delegate.data_received().find(
+                  directory.path().BaseName().MaybeAsASCII()) !=
+              std::string::npos);
+  EXPECT_TRUE(delegate.data_received().find(
+                  sub_dir.BaseName().MaybeAsASCII()) != std::string::npos);
+  EXPECT_TRUE(delegate.data_received().find(path.BaseName().MaybeAsASCII()) !=
+              std::string::npos);
+}
+
+TEST_F(URLRequestFileDirTest, EmptyDirectory) {
+  base::ScopedTempDir directory;
+  ASSERT_TRUE(directory.CreateUniqueTempDir());
+
+  TestJobFactory factory(directory.path());
+  context_.set_job_factory(&factory);
+
+  TestDelegate delegate;
+  scoped_ptr<URLRequest> request(context_.CreateRequest(
+      FilePathToFileURL(directory.path()), DEFAULT_PRIORITY, &delegate));
+  request->Start();
+  EXPECT_TRUE(request->is_pending());
+
+  base::RunLoop().Run();
+
+  ASSERT_GT(delegate.bytes_received(), 0);
+  ASSERT_LE(delegate.bytes_received(), kBufferSize);
+  EXPECT_TRUE(delegate.data_received().find(
+                  directory.path().BaseName().MaybeAsASCII()) !=
+              std::string::npos);
+}
+
 }  // namespace
 
 }  // namespace net