net: Split FileStream::Read/Write() into sync and async versions.

Historically, FileStream::Read/Write() used to take NULL for synchronous
operations, but these are replaced with ComplocationCallback(), which
is rather ugly. ReadSync() and WriteSync() which do not take a
CompletionCallback are introduced for synchronous operations.
Having function separate signatures make clients code cleaner,
and easier to catch synchronous operations on wrong threads.

This convention also matches with Open/OpenSync and Close/CloseSync.

BUG=72001
TEST=try bots.

Review URL: https://chromiumcodereview.appspot.com/9365024

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

8 files changed, 139 insertions, 50 deletions

diff --git a/net/base/file_stream.h b/net/base/file_stream.h
index 39863a5..7017f30 100644
--- a/net/base/file_stream.h
+++ b/net/base/file_stream.h
@@ -75,56 +75,84 @@ class NET_EXPORT FileStream {
   // position until the end of the file.  Otherwise, an error code is returned.
   virtual int64 Available();
 
-  // Call this method to read data from the current stream position.  Up to
-  // buf_len bytes will be copied into buf.  (In other words, partial reads are
-  // allowed.)  Returns the number of bytes copied, 0 if at end-of-file, or an
-  // error code if the operation could not be performed.
+  // Call this method to read data from the current stream position
+  // asynchronously. Up to buf_len bytes will be copied into buf.  (In
+  // other words, partial reads are allowed.)  Returns the number of bytes
+  // copied, 0 if at end-of-file, or an error code if the operation could
+  // not be performed.
   //
-  // If opened with PLATFORM_FILE_ASYNC, then a non-null callback
-  // must be passed to this method.  In asynchronous mode, if the read could
-  // not complete synchronously, then ERR_IO_PENDING is returned, and the
-  // callback will be notified on the current thread (via the MessageLoop) when
-  // the read has completed.
+  // The file must be opened with PLATFORM_FILE_ASYNC, and a non-null
+  // callback must be passed to this method. If the read could not
+  // complete synchronously, then ERR_IO_PENDING is returned, and the
+  // callback will be notified on the current thread (via the MessageLoop)
+  // when the read has completed.
   //
-  // In the case of an asychronous read, the memory pointed to by |buf| must
-  // remain valid until the callback is notified.  However, it is valid to
-  // destroy or close the file stream while there is an asynchronous read in
-  // progress.  That will cancel the read and allow the buffer to be freed.
+  // The memory pointed to by |buf| must remain valid until the callback is
+  // notified. TODO(satorux): Use IOBuffer instead of char*.
   //
-  // This method should not be called if the stream was opened WRITE_ONLY.
+  // It is valid to destroy or close the file stream while there is an
+  // asynchronous read in progress.  That will cancel the read and allow
+  // the buffer to be freed.
   //
-  // You can pass a null callback for synchronous I/O.
+  // It is invalid to request any asynchronous operations while there is an
+  // in-flight asynchronous operation.
+  //
+  // This method must not be called if the stream was opened WRITE_ONLY.
   virtual int Read(char* buf, int buf_len, const CompletionCallback& callback);
 
-  // Performs the same as Read, but ensures that exactly buf_len bytes
+  // Call this method to read data from the current stream position
+  // synchronously. Up to buf_len bytes will be copied into buf.  (In
+  // other words, partial reads are allowed.)  Returns the number of bytes
+  // copied, 0 if at end-of-file, or an error code if the operation could
+  // not be performed.
+  //
+  // The file must not be opened with PLATFORM_FILE_ASYNC.
+  // This method must not be called if the stream was opened WRITE_ONLY.
+  virtual int ReadSync(char* buf, int buf_len);
+
+  // Performs the same as ReadSync, but ensures that exactly buf_len bytes
   // are copied into buf.  A partial read may occur, but only as a result of
   // end-of-file or fatal error.  Returns the number of bytes copied into buf,
   // 0 if at end-of-file and no bytes have been read into buf yet,
   // or an error code if the operation could not be performed.
   virtual int ReadUntilComplete(char *buf, int buf_len);
 
-  // Call this method to write data at the current stream position.  Up to
-  // buf_len bytes will be written from buf. (In other words, partial writes are
-  // allowed.)  Returns the number of bytes written, or an error code if the
-  // operation could not be performed.
+  // Call this method to write data at the current stream position
+  // asynchronously.  Up to buf_len bytes will be written from buf. (In
+  // other words, partial writes are allowed.)  Returns the number of
+  // bytes written, or an error code if the operation could not be
+  // performed.
   //
-  // If opened with PLATFORM_FILE_ASYNC, then a non-null callback
-  // must be passed to this method.  In asynchronous mode, if the write could
-  // not complete synchronously, then ERR_IO_PENDING is returned, and the
-  // callback will be notified on the current thread (via the MessageLoop) when
-  // the write has completed.
+  // The file must be opened with PLATFORM_FILE_ASYNC, and a non-null
+  // callback must be passed to this method. If the write could not
+  // complete synchronously, then ERR_IO_PENDING is returned, and the
+  // callback will be notified on the current thread (via the MessageLoop)
+  // when the write has completed.
   //
-  // In the case of an asychronous write, the memory pointed to by |buf| must
-  // remain valid until the callback is notified.  However, it is valid to
-  // destroy or close the file stream while there is an asynchronous write in
-  // progress.  That will cancel the write and allow the buffer to be freed.
+  // The memory pointed to by |buf| must remain valid until the callback
+  // is notified. TODO(satorux): Use IOBuffer instead of char*.
   //
-  // This method should not be called if the stream was opened READ_ONLY.
+  // It is valid to destroy or close the file stream while there is an
+  // asynchronous write in progress.  That will cancel the write and allow
+  // the buffer to be freed.
+  //
+  // It is invalid to request any asynchronous operations while there is an
+  // in-flight asynchronous operation.
   //
-  // You can pass a null callback for synchronous I/O.
+  // This method must not be called if the stream was opened READ_ONLY.
   virtual int Write(const char* buf, int buf_len,
                     const CompletionCallback& callback);
 
+  // Call this method to write data at the current stream position
+  // synchronously.  Up to buf_len bytes will be written from buf. (In
+  // other words, partial writes are allowed.)  Returns the number of
+  // bytes written, or an error code if the operation could not be
+  // performed.
+  //
+  // The file must not be opened with PLATFORM_FILE_ASYNC.
+  // This method must not be called if the stream was opened READ_ONLY.
+  virtual int WriteSync(const char* buf, int buf_len);
+
   // Truncates the file to be |bytes| length. This is only valid for writable
   // files. After truncation the file stream is positioned at |bytes|. The new
   // position is retured, or a value < 0 on error.
@@ -153,6 +181,12 @@ class NET_EXPORT FileStream {
   void SetBoundNetLogSource(const net::BoundNetLog& owner_bound_net_log);
 
  private:
+  // Helper functions used to implement reads and writes.
+  int ReadInternal(char* buf, int buf_len,
+                   const CompletionCallback& callback);
+  int WriteInternal(const char* buf, int buf_len,
+                    const CompletionCallback& callback);
+
   class AsyncContext;
   friend class AsyncContext;
   friend class FileStreamTest;
diff --git a/net/base/file_stream_posix.cc b/net/base/file_stream_posix.cc
index 86f64ec..ded0cfc 100644
--- a/net/base/file_stream_posix.cc
+++ b/net/base/file_stream_posix.cc
@@ -441,6 +441,17 @@ int64 FileStream::Available() {
 
 int FileStream::Read(
     char* buf, int buf_len, const CompletionCallback& callback) {
+  DCHECK(async_context_.get());
+  return ReadInternal(buf, buf_len, callback);
+}
+
+int FileStream::ReadSync(char* buf, int buf_len) {
+  DCHECK(!async_context_.get());
+  return ReadInternal(buf, buf_len, CompletionCallback());
+}
+
+int FileStream::ReadInternal(
+    char* buf, int buf_len, const CompletionCallback& callback) {
   if (!IsOpen())
     return ERR_UNEXPECTED;
 
@@ -467,7 +478,7 @@ int FileStream::ReadUntilComplete(char *buf, int buf_len) {
   int bytes_total = 0;
 
   do {
-    int bytes_read = Read(buf, to_read, CompletionCallback());
+    int bytes_read = ReadSync(buf, to_read);
     if (bytes_read <= 0) {
       if (bytes_total == 0)
         return bytes_read;
@@ -485,6 +496,18 @@ int FileStream::ReadUntilComplete(char *buf, int buf_len) {
 
 int FileStream::Write(
     const char* buf, int buf_len, const CompletionCallback& callback) {
+  DCHECK(async_context_.get());
+  return WriteInternal(buf, buf_len, callback);
+}
+
+int FileStream::WriteSync(
+    const char* buf, int buf_len) {
+  DCHECK(!async_context_.get());
+  return WriteInternal(buf, buf_len, CompletionCallback());
+}
+
+int FileStream::WriteInternal(
+    const char* buf, int buf_len, const CompletionCallback& callback) {
   // write(..., 0) will return 0, which indicates end-of-file.
   DCHECK_GT(buf_len, 0);
 
diff --git a/net/base/file_stream_unittest.cc b/net/base/file_stream_unittest.cc
index 712904e..44044f6 100644
--- a/net/base/file_stream_unittest.cc
+++ b/net/base/file_stream_unittest.cc
@@ -106,7 +106,7 @@ TEST_F(FileStreamTest, UseFileHandle) {
   // Read into buffer and compare.
   char buffer[kTestDataSize];
   ASSERT_EQ(kTestDataSize,
-            read_stream.Read(buffer, kTestDataSize, CompletionCallback()));
+            read_stream.ReadSync(buffer, kTestDataSize));
   ASSERT_EQ(0, memcmp(kTestData, buffer, kTestDataSize));
   read_stream.CloseSync();
 
@@ -118,7 +118,7 @@ TEST_F(FileStreamTest, UseFileHandle) {
   FileStream write_stream(file, flags, NULL);
   ASSERT_EQ(0, write_stream.Seek(FROM_BEGIN, 0));
   ASSERT_EQ(kTestDataSize,
-            write_stream.Write(kTestData, kTestDataSize, CompletionCallback()));
+            write_stream.WriteSync(kTestData, kTestDataSize));
   write_stream.CloseSync();
 
   // Read into buffer and compare to make sure the handle worked fine.
@@ -142,7 +142,7 @@ TEST_F(FileStreamTest, UseClosedStream) {
 
   // Try reading...
   char buf[10];
-  int rv = stream.Read(buf, arraysize(buf), CompletionCallback());
+  int rv = stream.ReadSync(buf, arraysize(buf));
   EXPECT_EQ(ERR_UNEXPECTED, rv);
 }
 
@@ -165,7 +165,7 @@ TEST_F(FileStreamTest, BasicRead) {
   std::string data_read;
   for (;;) {
     char buf[4];
-    rv = stream.Read(buf, arraysize(buf), CompletionCallback());
+    rv = stream.ReadSync(buf, arraysize(buf));
     EXPECT_LE(0, rv);
     if (rv <= 0)
       break;
@@ -264,7 +264,7 @@ TEST_F(FileStreamTest, BasicRead_FromOffset) {
   std::string data_read;
   for (;;) {
     char buf[4];
-    rv = stream.Read(buf, arraysize(buf), CompletionCallback());
+    rv = stream.ReadSync(buf, arraysize(buf));
     EXPECT_LE(0, rv);
     if (rv <= 0)
       break;
@@ -350,7 +350,7 @@ TEST_F(FileStreamTest, BasicWrite) {
   EXPECT_TRUE(ok);
   EXPECT_EQ(0, file_size);
 
-  rv = stream.Write(kTestData, kTestDataSize, CompletionCallback());
+  rv = stream.WriteSync(kTestData, kTestDataSize);
   EXPECT_EQ(kTestDataSize, rv);
   stream.CloseSync();
 
@@ -439,7 +439,7 @@ TEST_F(FileStreamTest, BasicWrite_FromOffset) {
   int64 new_offset = stream.Seek(FROM_END, kOffset);
   EXPECT_EQ(kTestDataSize, new_offset);
 
-  rv = stream.Write(kTestData, kTestDataSize, CompletionCallback());
+  rv = stream.WriteSync(kTestData, kTestDataSize);
   EXPECT_EQ(kTestDataSize, rv);
   stream.CloseSync();
 
@@ -503,7 +503,7 @@ TEST_F(FileStreamTest, BasicReadWrite) {
   std::string data_read;
   for (;;) {
     char buf[4];
-    rv = stream.Read(buf, arraysize(buf), CompletionCallback());
+    rv = stream.ReadSync(buf, arraysize(buf));
     EXPECT_LE(0, rv);
     if (rv <= 0)
       break;
@@ -513,7 +513,7 @@ TEST_F(FileStreamTest, BasicReadWrite) {
   EXPECT_EQ(file_size, total_bytes_read);
   EXPECT_TRUE(data_read == kTestData);
 
-  rv = stream.Write(kTestData, kTestDataSize, CompletionCallback());
+  rv = stream.WriteSync(kTestData, kTestDataSize);
   EXPECT_EQ(kTestDataSize, rv);
   stream.CloseSync();
 
@@ -540,7 +540,7 @@ TEST_F(FileStreamTest, BasicWriteRead) {
   int64 offset = stream.Seek(FROM_END, 0);
   EXPECT_EQ(offset, file_size);
 
-  rv = stream.Write(kTestData, kTestDataSize, CompletionCallback());
+  rv = stream.WriteSync(kTestData, kTestDataSize);
   EXPECT_EQ(kTestDataSize, rv);
 
   offset = stream.Seek(FROM_BEGIN, 0);
@@ -551,7 +551,7 @@ TEST_F(FileStreamTest, BasicWriteRead) {
   std::string data_read;
   for (;;) {
     char buf[4];
-    rv = stream.Read(buf, arraysize(buf), CompletionCallback());
+    rv = stream.ReadSync(buf, arraysize(buf));
     EXPECT_LE(0, rv);
     if (rv <= 0)
       break;
@@ -926,13 +926,13 @@ TEST_F(FileStreamTest, Truncate) {
 
   // Write some data to the file.
   const char test_data[] = "0123456789";
-  write_stream.Write(test_data, arraysize(test_data), CompletionCallback());
+  write_stream.WriteSync(test_data, arraysize(test_data));
 
   // Truncate the file.
   ASSERT_EQ(4, write_stream.Truncate(4));
 
   // Write again.
-  write_stream.Write(test_data, 4, CompletionCallback());
+  write_stream.WriteSync(test_data, 4);
 
   // Close the stream.
   write_stream.CloseSync();
diff --git a/net/base/file_stream_win.cc b/net/base/file_stream_win.cc
index 425a3f7..e111733 100644
--- a/net/base/file_stream_win.cc
+++ b/net/base/file_stream_win.cc
@@ -278,6 +278,17 @@ int64 FileStream::Available() {
 
 int FileStream::Read(
     char* buf, int buf_len, const CompletionCallback& callback) {
+  DCHECK(async_context_.get());
+  return ReadInternal(buf, buf_len, callback);
+}
+
+int FileStream::ReadSync(char* buf, int buf_len) {
+  DCHECK(!async_context_.get());
+  return ReadInternal(buf, buf_len, CompletionCallback());
+}
+
+int FileStream::ReadInternal(
+    char* buf, int buf_len, const CompletionCallback& callback) {
   if (!IsOpen())
     return ERR_UNEXPECTED;
 
@@ -325,7 +336,7 @@ int FileStream::ReadUntilComplete(char *buf, int buf_len) {
   int bytes_total = 0;
 
   do {
-    int bytes_read = Read(buf, to_read, CompletionCallback());
+    int bytes_read = ReadSync(buf, to_read);
     if (bytes_read <= 0) {
       if (bytes_total == 0)
         return bytes_read;
@@ -343,6 +354,18 @@ int FileStream::ReadUntilComplete(char *buf, int buf_len) {
 
 int FileStream::Write(
     const char* buf, int buf_len, const CompletionCallback& callback) {
+  DCHECK(async_context_.get());
+  return WriteInternal(buf, buf_len, callback);
+}
+
+int FileStream::WriteSync(
+    const char* buf, int buf_len) {
+  DCHECK(!async_context_.get());
+  return WriteInternal(buf, buf_len, CompletionCallback());
+}
+
+int FileStream::WriteInternal(
+    const char* buf, int buf_len, const CompletionCallback& callback) {
   if (!IsOpen())
     return ERR_UNEXPECTED;
 
diff --git a/net/base/mock_file_stream.cc b/net/base/mock_file_stream.cc
index afca747..e87491f 100644
--- a/net/base/mock_file_stream.cc
+++ b/net/base/mock_file_stream.cc
@@ -27,6 +27,10 @@ int MockFileStream::Read(char* buf,
   return ReturnError(FileStream::Read(buf, buf_len, callback));
 }
 
+int MockFileStream::ReadSync(char* buf, int buf_len) {
+  return ReturnError(FileStream::ReadSync(buf, buf_len));
+}
+
 int MockFileStream::ReadUntilComplete(char *buf, int buf_len) {
   return ReturnError(FileStream::ReadUntilComplete(buf, buf_len));
 }
@@ -37,6 +41,10 @@ int MockFileStream::Write(const char* buf,
   return ReturnError(FileStream::Write(buf, buf_len, callback));
 }
 
+int MockFileStream::WriteSync(const char* buf, int buf_len) {
+  return ReturnError(FileStream::WriteSync(buf, buf_len));
+}
+
 int64 MockFileStream::Truncate(int64 bytes) {
   return ReturnError64(FileStream::Truncate(bytes));
 }
diff --git a/net/base/mock_file_stream.h b/net/base/mock_file_stream.h
index f07785a..8424e51 100644
--- a/net/base/mock_file_stream.h
+++ b/net/base/mock_file_stream.h
@@ -33,10 +33,12 @@ class MockFileStream : public net::FileStream {
   virtual int Read(char* buf,
                    int buf_len,
                    const CompletionCallback& callback) OVERRIDE;
+  virtual int ReadSync(char* buf, int buf_len) OVERRIDE;
   virtual int ReadUntilComplete(char *buf, int buf_len) OVERRIDE;
   virtual int Write(const char* buf,
                     int buf_len,
                     const CompletionCallback& callback) OVERRIDE;
+  virtual int WriteSync(const char* buf, int buf_len) OVERRIDE;
   virtual int64 Truncate(int64 bytes) OVERRIDE;
   virtual int Flush() OVERRIDE;
 
diff --git a/net/base/upload_data_stream.cc b/net/base/upload_data_stream.cc
index 5b2e5d3..7e399da 100644
--- a/net/base/upload_data_stream.cc
+++ b/net/base/upload_data_stream.cc
@@ -117,9 +117,8 @@ int UploadDataStream::Read(IOBuffer* buf, int buf_len) {
         // missing or not readable.
         if (element_file_stream_.get()) {
           num_bytes_consumed =
-              element_file_stream_->Read(buf->data() + bytes_copied,
-                                         num_bytes_to_read,
-                                         CompletionCallback());
+              element_file_stream_->ReadSync(buf->data() + bytes_copied,
+                                             num_bytes_to_read);
         }
         if (num_bytes_consumed <= 0) {
           // If there's less data to read than we initially observed, then
diff --git a/net/tools/dump_cache/dump_files.cc b/net/tools/dump_cache/dump_files.cc
index f5656b0..51b8c390 100644
--- a/net/tools/dump_cache/dump_files.cc
+++ b/net/tools/dump_cache/dump_files.cc
@@ -34,7 +34,7 @@ bool ReadHeader(const std::wstring& name, char* header, int header_size) {
     return false;
   }
 
-  int read = file.Read(header, header_size, net::CompletionCallback());
+  int read = file.ReadSync(header, header_size);
   if (read != header_size) {
     printf("Unable to read file %ls\n", name.c_str());
     return false;