[Net] Fix error handling on wrong file in UploadData

net/url_request/url_request_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shlobj.h>
 #endif
@@ skipping 4454 matching lines @@
     path = path.Append(FILE_PATH_LITERAL("net"));
     path = path.Append(FILE_PATH_LITERAL("data"));
     path = path.Append(FILE_PATH_LITERAL("url_request_unittest"));
     path = path.Append(FILE_PATH_LITERAL("with-headers.html"));
     element_readers.push_back(
         new UploadFileElementReader(base::MessageLoopProxy::current().get(),
                                     path,
                                     0,
                                     kuint64max,
                                     base::Time()));
-
-    // This file should just be ignored in the upload stream.
-    element_readers.push_back(new UploadFileElementReader(
-        base::MessageLoopProxy::current().get(),
-        base::FilePath(FILE_PATH_LITERAL(
-            "c:\\path\\to\\non\\existant\\file.randomness.12345")),
-        0,
-        kuint64max,
-        base::Time()));
     r.set_upload(make_scoped_ptr(
         new UploadDataStream(element_readers.Pass(), 0)));
 
     r.Start();
     EXPECT_TRUE(r.is_pending());
 
     base::RunLoop().Run();
 
     int64 size = 0;
     ASSERT_EQ(true, file_util::GetFileSize(path, &size));
     scoped_ptr<char[]> buf(new char[size]);
 
     ASSERT_EQ(size, file_util::ReadFile(path, buf.get(), size));
 
     ASSERT_EQ(1, d.response_started_count())
         << "request failed: " << r.status().status()
         << ", error: " << r.status().error();
 
     EXPECT_FALSE(d.received_data_before_response());
 
     EXPECT_EQ(size, d.bytes_received());
     EXPECT_EQ(std::string(&buf[0], size), d.data_received());
   }
 }
 
+TEST_F(URLRequestTestHTTP, PostUnreadableFileTest) {
+  ASSERT_TRUE(test_server_.Start());
+
+  TestDelegate d;
+  {
+    URLRequest r(test_server_.GetURL("echo"), &d, &default_context_);
+    r.set_method("POST");
+
+    base::FilePath dir;
+    PathService::Get(base::DIR_EXE, &dir);
+    file_util::SetCurrentDirectory(dir);
+
+    ScopedVector<UploadElementReader> element_readers;
+
+    element_readers.push_back(new UploadFileElementReader(
+        base::MessageLoopProxy::current().get(),
+        base::FilePath(FILE_PATH_LITERAL(
+            "c:\\path\\to\\non\\existant\\file.randomness.12345")),
+        0,
+        kuint64max,
+        base::Time()));
+    r.set_upload(make_scoped_ptr(
+        new UploadDataStream(element_readers.Pass(), 0)));
+
+    r.Start();
+    EXPECT_TRUE(r.is_pending());
+
+    base::RunLoop().Run();
+
+    EXPECT_TRUE(d.request_failed());
+    EXPECT_FALSE(d.received_data_before_response());
+    EXPECT_EQ(0, d.bytes_received());

[hashimoto, 2013/10/29 04:08:59]

How about having checks for r.status().status() and r.status().error()?

[tzik, 2013/10/29 06:06:19]

The request should be aborted before it set status(). So status().status() and
status().error() are SUCCESS/OK as they were initialized.
I'm not sure we should check them.

[hashimoto, 2013/10/29 06:55:26]

Really? In my local environment, FAILED/ERR_FILE_NOT_FOUND is set with the
calling stack below.
[7868:7868:1029/154135:3022819135420:ERROR:url_request.h(690)]  [0x7f575a0b0a2e]
base::debug::StackTrace::StackTrace()
 [0x7f5759c404e6] net::URLRequest::set_status()
 [0x7f5759c3f9c4] net::URLRequestJob::NotifyStartError()
 [0x7f5759c31e67] net::URLRequestHttpJob::OnStartCompleted()
 [0x7f5759c3a639] base::internal::RunnableAdapter<>::Run()
 [0x7f5759c3c334] base::internal::InvokeHelper<>::MakeItSo()
 [0x7f5759c3c2c2] base::internal::Invoker<>::Run()
 [0x7f5759694256] base::Callback<>::Run()
 [0x7f5759917260] net::HttpCache::Transaction::DoCallback()
 [0x7f5759917356] net::HttpCache::Transaction::HandleResult()
 [0x7f57599144b5] net::HttpCache::Transaction::DoLoop()
 [0x7f5759912c3b] net::HttpCache::Transaction::OnIOComplete()
 [0x7f5759923669] base::internal::RunnableAdapter<>::Run()
 [0x7f5759923568] base::internal::InvokeHelper<>::MakeItSo()
 [0x7f57599234ed] base::internal::Invoker<>::Run()
 [0x7f5759694256] base::Callback<>::Run()
 [0x7f575992ec22] net::HttpNetworkTransaction::DoCallback()
 [0x7f575992b39b] net::HttpNetworkTransaction::OnIOComplete()
 [0x7f5759935db9] base::internal::RunnableAdapter<>::Run()
 [0x7f5759935d14] base::internal::InvokeHelper<>::MakeItSo()
 [0x7f5759935ca2] base::internal::Invoker<>::Run()
 [0x7f5759694256] base::Callback<>::Run()
 [0x7f575971048b] net::UploadDataStream::ResumePendingInit()
 [0x7f57597121e3] base::internal::RunnableAdapter<>::Run()
 [0x7f5759712116] base::internal::InvokeHelper<>::MakeItSo()
 [0x7f575971206b] base::internal::Invoker<>::Run()
 [0x7f5759694256] base::Callback<>::Run()
 [0x7f57597148cb] net::UploadFileElementReader::OnInitCompleted()

[tzik, 2013/10/31 12:19:28]

Sorry, right it is.
My build might be from another branch when I tried it.
Fixed.

+  }
+}
+
 TEST_F(URLRequestTestHTTP, TestPostChunkedDataBeforeStart) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
   {
     URLRequest r(test_server_.GetURL("echo"), &d, &default_context_);
     r.EnableChunkedUpload();
     r.set_method("POST");
     AddChunksToUpload(&r);
     r.Start();
@@ skipping 2447 matching lines @@
 
     EXPECT_FALSE(r.is_pending());
     EXPECT_EQ(1, d->response_started_count());
     EXPECT_FALSE(d->received_data_before_response());
     EXPECT_EQ(d->bytes_received(), static_cast<int>(file_size));
   }
 }
 #endif  // !defined(DISABLE_FTP_SUPPORT)
 
 }  // namespace net