Experimental Revert of 184577

ASAN bots started timing out consistently last night at the time
this was submitted.  It's a possible culprit.  It doesn't involve
extensions, but it does things with streams and I assume those
are connected to the feeds in this test somehow.

browser_tests browser_tests failed 4 ( 12 mins, 52 secs )
stdio
ParseFeedInvalidFeed1
ParseFeedInvalidFeed2
ParseFeedValidFeed3
ParseFeedValidFeed4

http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASAN%20Tests%20%281%29/builds/3019
http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASAN%20Tests%20%282%29/builds/3186
http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASAN%20Tests%20%283%29/builds/2716


> Fix net::FileStream to handle all errors correctly.
> 
> This CL adds FileStream::Context::IOResult struct that's used to pass
> results of IO operations inside FileStream::Context. Following bugs are
> fixes:
> 
> 1. On POSIX net::FileStream::Read() and net::FileStream::Write() would
> return a positive result in case of an error. This happens because
> POSIX errors are positive integers and FileStream was written with
> assumption that they are negative.
> 2. On Windows Seek() and Flush() errors were not handled correctly.
> This is because CheckForIOError() was assuming that all error codes
> are negative, but RecordAndMapError() was mapping positive errors.
> 3. On Windows results of asynchronous ReadFile() and WriteFile() were
> not handled correctly - ERR_IO_PENDING would be returned even when
> operation completes synchronously.
> 4. On Windows OVERLAPPED struct wasn't set to zeros as necessary.
> 
> Also added unittests to check that error codes are handled correctly
> now.
> 
> TBR=mmenke@chromium.org
> 
> Review URL: https://codereview.chromium.org/12326107

TBR=sergeyu@chromium.org
Review URL: https://codereview.chromium.org/12330144

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

1 files changed, 38 insertions, 63 deletions

diff --git a/net/base/file_stream_context.cc b/net/base/file_stream_context.cc
index 092302f..53f3c3d 100644
--- a/net/base/file_stream_context.cc
+++ b/net/base/file_stream_context.cc
@@ -22,32 +22,6 @@ void CallInt64ToInt(const net::CompletionCallback& callback, int64 result) {
 
 namespace net {
 
-FileStream::Context::IOResult::IOResult()
-    : result(OK),
-      os_error(0) {
-}
-
-FileStream::Context::IOResult::IOResult(int64 result, int os_error)
-    : result(result),
-      os_error(os_error) {
-}
-
-// static
-FileStream::Context::IOResult FileStream::Context::IOResult::FromOSError(
-    int64 os_error) {
-  return IOResult(MapSystemError(os_error), os_error);
-}
-
-FileStream::Context::OpenResult::OpenResult()
-    : file(base::kInvalidPlatformFileValue) {
-}
-
-FileStream::Context::OpenResult::OpenResult(base::PlatformFile file,
-                                            IOResult error_code)
-    : file(file),
-      error_code(error_code) {
-}
-
 void FileStream::Context::Orphan() {
   DCHECK(!orphaned_);
 
@@ -88,14 +62,14 @@ int FileStream::Context::OpenSync(const base::FilePath& path, int open_flags) {
   OpenResult result = OpenFileImpl(path, open_flags);
   file_ = result.file;
   if (file_ == base::kInvalidPlatformFileValue) {
-    ProcessOpenError(result.error_code);
+    result.error_code = ProcessOpenError(result.error_code);
   } else {
     // TODO(satorux): Remove this once all async clients are migrated to use
     // Open(). crbug.com/114783
     if (open_flags & base::PLATFORM_FILE_ASYNC)
       OnAsyncFileOpened();
   }
-  return result.error_code.result;
+  return result.error_code;
 }
 
 void FileStream::Context::CloseSync() {
@@ -125,9 +99,9 @@ void FileStream::Context::SeekAsync(Whence whence,
 }
 
 int64 FileStream::Context::SeekSync(Whence whence, int64 offset) {
-  IOResult result = SeekFileImpl(whence, offset);
-  RecordError(result, FILE_ERROR_SOURCE_SEEK);
-  return result.result;
+  int64 result = SeekFileImpl(whence, offset);
+  CheckForIOError(&result, FILE_ERROR_SOURCE_SEEK);
+  return result;
 }
 
 void FileStream::Context::FlushAsync(const CompletionCallback& callback) {
@@ -147,32 +121,26 @@ void FileStream::Context::FlushAsync(const CompletionCallback& callback) {
 }
 
 int FileStream::Context::FlushSync() {
-  IOResult result = FlushFileImpl();
-  RecordError(result, FILE_ERROR_SOURCE_FLUSH);
-  return result.result;
+  int64 result = FlushFileImpl();
+  CheckForIOError(&result, FILE_ERROR_SOURCE_FLUSH);
+  return result;
 }
 
-void FileStream::Context::RecordError(const IOResult& result,
-                                      FileErrorSource source) const {
-  if (result.result >= 0) {
-    // |result| is not an error.
-    return;
-  }
-
+int FileStream::Context::RecordAndMapError(int error,
+                                           FileErrorSource source) const {
   // The following check is against incorrect use or bug. File descriptor
   // shouldn't ever be closed outside of FileStream while it still tries to do
   // something with it.
-  DCHECK_NE(result.result, ERR_INVALID_HANDLE);
+  DCHECK(error != ERROR_BAD_FILE);
+  Error net_error = MapSystemError(error);
 
   if (!orphaned_) {
-    bound_net_log_.AddEvent(
-        NetLog::TYPE_FILE_STREAM_ERROR,
-        base::Bind(&NetLogFileStreamErrorCallback,
-                   source, result.os_error,
-                   static_cast<net::Error>(result.result)));
+    bound_net_log_.AddEvent(NetLog::TYPE_FILE_STREAM_ERROR,
+                            base::Bind(&NetLogFileStreamErrorCallback,
+                                       source, error, net_error));
   }
-
-  RecordFileError(result.os_error, source, record_uma_);
+  RecordFileError(error, source, record_uma_);
+  return net_error;
 }
 
 void FileStream::Context::BeginOpenEvent(const base::FilePath& path) {
@@ -188,27 +156,28 @@ FileStream::Context::OpenResult FileStream::Context::OpenFileImpl(
   // delete the file right after FileStream deletion. Thus we are always
   // adding SHARE_DELETE flag to accommodate such use case.
   open_flags |= base::PLATFORM_FILE_SHARE_DELETE;
-  base::PlatformFile file =
-      base::CreatePlatformFile(path, open_flags, NULL, NULL);
-  if (file == base::kInvalidPlatformFileValue)
-    return OpenResult(file, IOResult::FromOSError(GetLastErrno()));
+  OpenResult result;
+  result.error_code = OK;
+  result.file = base::CreatePlatformFile(path, open_flags, NULL, NULL);
+  if (result.file == base::kInvalidPlatformFileValue)
+    result.error_code = GetLastErrno();
 
-  return OpenResult(file, IOResult(OK, 0));
+  return result;
 }
 
-void FileStream::Context::ProcessOpenError(const IOResult& error_code) {
+int FileStream::Context::ProcessOpenError(int error_code) {
   bound_net_log_.EndEvent(NetLog::TYPE_FILE_STREAM_OPEN);
-  RecordError(error_code, FILE_ERROR_SOURCE_OPEN);
+  return RecordAndMapError(error_code, FILE_ERROR_SOURCE_OPEN);
 }
 
 void FileStream::Context::OnOpenCompleted(const CompletionCallback& callback,
-                                          OpenResult open_result) {
-  file_ = open_result.file;
+                                          OpenResult result) {
+  file_ = result.file;
   if (file_ == base::kInvalidPlatformFileValue)
-    ProcessOpenError(open_result.error_code);
+    result.error_code = ProcessOpenError(result.error_code);
   else if (!orphaned_)
     OnAsyncFileOpened();
-  OnAsyncCompleted(IntToInt64(callback), open_result.error_code.result);
+  OnAsyncCompleted(IntToInt64(callback), result.error_code);
 }
 
 void FileStream::Context::CloseAndDelete() {
@@ -236,12 +205,18 @@ Int64CompletionCallback FileStream::Context::IntToInt64(
   return base::Bind(&CallInt64ToInt, callback);
 }
 
+void FileStream::Context::CheckForIOError(int64* result,
+                                          FileErrorSource source) {
+  if (*result < 0)
+    *result = RecordAndMapError(static_cast<int>(*result), source);
+}
+
 void FileStream::Context::ProcessAsyncResult(
     const Int64CompletionCallback& callback,
     FileErrorSource source,
-    const IOResult& result) {
-  RecordError(result, source);
-  OnAsyncCompleted(callback, result.result);
+    int64 result) {
+  CheckForIOError(&result, source);
+  OnAsyncCompleted(callback, result);
 }
 
 void FileStream::Context::OnAsyncCompleted(