Make URLFetcherFileWriter::Finish() skip closing file when there is an error

net/url_request/url_fetcher_response_writer.h

Index: net/url_request/url_fetcher_response_writer.h
diff --git a/net/url_request/url_fetcher_response_writer.h b/net/url_request/url_fetcher_response_writer.h
index f739502b9c9eb93e8d6d90bb40a10008dd163878..768d10b9faa11f32be28dd35d3203c1de3b347b0 100644
--- a/net/url_request/url_fetcher_response_writer.h
+++ b/net/url_request/url_fetcher_response_writer.h
@@ -45,9 +45,11 @@ class NET_EXPORT URLFetcherResponseWriter {
                     int num_bytes,
                     const CompletionCallback& callback) = 0;
 
-  // Finishes writing. If ERR_IO_PENDING is returned, |callback| will be run
-  // later with the result.
-  virtual int Finish(const CompletionCallback& callback) = 0;
+  // Finishes writing. If |net_error| is not OK, this method can be called
+  // in the middle of another operation (eg. Initialize() and Write()). If
+  // |net_error| is not OK, graceful shutdown might be skipped. If

[mmenke, 2016/10/19 20:49:34]

nit:  Maybe reword the 2nd and third sentences?

"On errors (|net_error| not net::OK), this method may be called before the
previous operation completed. In this case, URLFetcherResponseWriter may skip
graceful shutdown and completion of the pending operation. After such a failure,
the ResponseWriter may be reused.

[xunjieli, 2016/10/20 15:57:49]

Done.

+  // ERR_IO_PENDING is returned, |callback| will be run later with the result.
+  virtual int Finish(int net_error, const CompletionCallback& callback) = 0;
 
   // Returns this instance's pointer as URLFetcherStringWriter when possible.
   virtual URLFetcherStringWriter* AsStringWriter();
@@ -69,7 +71,7 @@ class NET_EXPORT URLFetcherStringWriter : public URLFetcherResponseWriter {
   int Write(IOBuffer* buffer,
             int num_bytes,
             const CompletionCallback& callback) override;
-  int Finish(const CompletionCallback& callback) override;
+  int Finish(int net_error, const CompletionCallback& callback) override;
   URLFetcherStringWriter* AsStringWriter() override;
 
  private:
@@ -95,7 +97,7 @@ class NET_EXPORT URLFetcherFileWriter : public URLFetcherResponseWriter {
   int Write(IOBuffer* buffer,
             int num_bytes,
             const CompletionCallback& callback) override;
-  int Finish(const CompletionCallback& callback) override;
+  int Finish(int net_error, const CompletionCallback& callback) override;
   URLFetcherFileWriter* AsFileWriter() override;
 
   // Drops ownership of the file at |file_path_|.
@@ -103,23 +105,19 @@ class NET_EXPORT URLFetcherFileWriter : public URLFetcherResponseWriter {
   void DisownFile();
 
  private:
-  // Called when a write has been done.
-  void DidWrite(const CompletionCallback& callback, int result);
-
   // Closes the file if it is open and then delete it.
   void CloseAndDeleteFile();
 
   // Callback which gets the result of a temporary file creation.
-  void DidCreateTempFile(const CompletionCallback& callback,
-                         base::FilePath* temp_file_path,
+  void DidCreateTempFile(base::FilePath* temp_file_path,
                          bool success);
 
-  // Callback which gets the result of FileStream::Open.
-  void DidOpenFile(const CompletionCallback& callback,
-                   int result);
+  // Run |callback_| if it is non-null when FileStream::Open or
+  // FileStream::Write is completed.
+  void OnIOCompleted(int result);
 
   // Callback which gets the result of closing a file.
-  void CloseComplete(const CompletionCallback& callback, int result);
+  void CloseComplete(int result);
 
   // Task runner on which file operations should happen.
   scoped_refptr<base::SequencedTaskRunner> file_task_runner_;
@@ -133,6 +131,8 @@ class NET_EXPORT URLFetcherFileWriter : public URLFetcherResponseWriter {
 
   std::unique_ptr<FileStream> file_stream_;
 
+  CompletionCallback callback_;
+
   // Callbacks are created for use with base::FileUtilProxy.
   base::WeakPtrFactory<URLFetcherFileWriter> weak_factory_;