Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(660)

Side by Side Diff: net/url_request/url_fetcher_response_writer.cc

Issue 2425673006: Make URLFetcherFileWriter::Finish() skip closing file when there is an error (Closed)
Patch Set: Fix test Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2013 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "net/url_request/url_fetcher_response_writer.h" 5 #include "net/url_request/url_fetcher_response_writer.h"
6 6
7 #include "base/callback_helpers.h"
7 #include "base/files/file_util.h" 8 #include "base/files/file_util.h"
8 #include "base/location.h" 9 #include "base/location.h"
9 #include "base/sequenced_task_runner.h" 10 #include "base/sequenced_task_runner.h"
10 #include "base/task_runner_util.h" 11 #include "base/task_runner_util.h"
11 #include "net/base/file_stream.h" 12 #include "net/base/file_stream.h"
12 #include "net/base/io_buffer.h" 13 #include "net/base/io_buffer.h"
13 #include "net/base/net_errors.h" 14 #include "net/base/net_errors.h"
14 15
15 namespace net { 16 namespace net {
16 17
(...skipping 16 matching lines...) Expand all
33 return OK; 34 return OK;
34 } 35 }
35 36
36 int URLFetcherStringWriter::Write(IOBuffer* buffer, 37 int URLFetcherStringWriter::Write(IOBuffer* buffer,
37 int num_bytes, 38 int num_bytes,
38 const CompletionCallback& callback) { 39 const CompletionCallback& callback) {
39 data_.append(buffer->data(), num_bytes); 40 data_.append(buffer->data(), num_bytes);
40 return num_bytes; 41 return num_bytes;
41 } 42 }
42 43
43 int URLFetcherStringWriter::Finish(const CompletionCallback& callback) { 44 int URLFetcherStringWriter::Finish(int net_error,
45 const CompletionCallback& callback) {
44 // Do nothing. 46 // Do nothing.
45 return OK; 47 return OK;
46 } 48 }
47 49
48 URLFetcherStringWriter* URLFetcherStringWriter::AsStringWriter() { 50 URLFetcherStringWriter* URLFetcherStringWriter::AsStringWriter() {
49 return this; 51 return this;
50 } 52 }
51 53
52 URLFetcherFileWriter::URLFetcherFileWriter( 54 URLFetcherFileWriter::URLFetcherFileWriter(
53 scoped_refptr<base::SequencedTaskRunner> file_task_runner, 55 scoped_refptr<base::SequencedTaskRunner> file_task_runner,
54 const base::FilePath& file_path) 56 const base::FilePath& file_path)
55 : file_task_runner_(file_task_runner), 57 : file_task_runner_(file_task_runner),
56 file_path_(file_path), 58 file_path_(file_path),
57 owns_file_(false), 59 owns_file_(false),
58 weak_factory_(this) { 60 weak_factory_(this) {
59 DCHECK(file_task_runner_.get()); 61 DCHECK(file_task_runner_.get());
60 } 62 }
61 63
62 URLFetcherFileWriter::~URLFetcherFileWriter() { 64 URLFetcherFileWriter::~URLFetcherFileWriter() {
63 CloseAndDeleteFile(); 65 CloseAndDeleteFile();
64 } 66 }
65 67
66 int URLFetcherFileWriter::Initialize(const CompletionCallback& callback) { 68 int URLFetcherFileWriter::Initialize(const CompletionCallback& callback) {
69 DCHECK(!callback_);
70
67 file_stream_.reset(new FileStream(file_task_runner_)); 71 file_stream_.reset(new FileStream(file_task_runner_));
68 72
69 int result = ERR_IO_PENDING; 73 int result = ERR_IO_PENDING;
74 owns_file_ = true;
70 if (file_path_.empty()) { 75 if (file_path_.empty()) {
71 base::FilePath* temp_file_path = new base::FilePath; 76 base::FilePath* temp_file_path = new base::FilePath;
72 base::PostTaskAndReplyWithResult( 77 base::PostTaskAndReplyWithResult(
73 file_task_runner_.get(), 78 file_task_runner_.get(),
74 FROM_HERE, 79 FROM_HERE,
75 base::Bind(&base::CreateTemporaryFile, temp_file_path), 80 base::Bind(&base::CreateTemporaryFile, temp_file_path),
76 base::Bind(&URLFetcherFileWriter::DidCreateTempFile, 81 base::Bind(&URLFetcherFileWriter::DidCreateTempFile,
77 weak_factory_.GetWeakPtr(), 82 weak_factory_.GetWeakPtr(),
78 callback,
79 base::Owned(temp_file_path))); 83 base::Owned(temp_file_path)));
80 } else { 84 } else {
81 result = file_stream_->Open( 85 result = file_stream_->Open(file_path_, base::File::FLAG_WRITE |
82 file_path_, 86 base::File::FLAG_ASYNC |
83 base::File::FLAG_WRITE | base::File::FLAG_ASYNC | 87 base::File::FLAG_CREATE_ALWAYS,
84 base::File::FLAG_CREATE_ALWAYS, 88 base::Bind(&URLFetcherFileWriter::OnIOCompleted,
85 base::Bind(&URLFetcherFileWriter::DidOpenFile, 89 weak_factory_.GetWeakPtr()));
86 weak_factory_.GetWeakPtr(),
87 callback));
88 DCHECK_NE(OK, result); 90 DCHECK_NE(OK, result);
89 } 91 }
92
93 if (result == ERR_IO_PENDING) {
94 callback_ = callback;
95 return result;
96 }
97 if (result < 0)
98 CloseAndDeleteFile();
90 return result; 99 return result;
91 } 100 }
92 101
93 int URLFetcherFileWriter::Write(IOBuffer* buffer, 102 int URLFetcherFileWriter::Write(IOBuffer* buffer,
94 int num_bytes, 103 int num_bytes,
95 const CompletionCallback& callback) { 104 const CompletionCallback& callback) {
96 DCHECK(file_stream_); 105 DCHECK(file_stream_);
97 DCHECK(owns_file_); 106 DCHECK(owns_file_);
107 DCHECK(!callback_);
98 108
99 int result = file_stream_->Write(buffer, num_bytes, 109 int result = file_stream_->Write(
100 base::Bind(&URLFetcherFileWriter::DidWrite, 110 buffer, num_bytes, base::Bind(&URLFetcherFileWriter::OnIOCompleted,
101 weak_factory_.GetWeakPtr(), 111 weak_factory_.GetWeakPtr()));
102 callback)); 112 if (result == ERR_IO_PENDING) {
103 if (result < 0 && result != ERR_IO_PENDING) 113 callback_ = callback;
114 return result;
115 }
116 if (result < 0)
104 CloseAndDeleteFile(); 117 CloseAndDeleteFile();
105
106 return result; 118 return result;
107 } 119 }
108 120
109 int URLFetcherFileWriter::Finish(const CompletionCallback& callback) { 121 int URLFetcherFileWriter::Finish(int net_error,
122 const CompletionCallback& callback) {
123 DCHECK_NE(ERR_IO_PENDING, net_error);
124
125 // If an error occurred, simply delete the file after any pending operation
126 // is done. Do not call file_stream_->Close() because there might be an
127 // operation pending. See crbug.com/487732.
128 if (net_error < 0) {
129 // Cancel callback and invalid weak ptrs so as to cancel any posted task.
130 callback_.Reset();
131 weak_factory_.InvalidateWeakPtrs();
132 CloseAndDeleteFile();
133 return OK;
134 }
135 DCHECK(!callback_);
110 // If the file_stream_ still exists at this point, close it. 136 // If the file_stream_ still exists at this point, close it.
111 if (file_stream_) { 137 if (file_stream_) {
112 int result = file_stream_->Close(base::Bind( 138 int result = file_stream_->Close(base::Bind(
113 &URLFetcherFileWriter::CloseComplete, 139 &URLFetcherFileWriter::CloseComplete, weak_factory_.GetWeakPtr()));
114 weak_factory_.GetWeakPtr(), callback)); 140 if (result == ERR_IO_PENDING) {
115 if (result != ERR_IO_PENDING) 141 callback_ = callback;
116 file_stream_.reset(); 142 return result;
143 }
144 file_stream_.reset();
117 return result; 145 return result;
118 } 146 }
119 return OK; 147 return OK;
120 } 148 }
121 149
122 URLFetcherFileWriter* URLFetcherFileWriter::AsFileWriter() { 150 URLFetcherFileWriter* URLFetcherFileWriter::AsFileWriter() {
123 return this; 151 return this;
124 } 152 }
125 153
126 void URLFetcherFileWriter::DisownFile() { 154 void URLFetcherFileWriter::DisownFile() {
127 // Disowning is done by the delegate's OnURLFetchComplete method. 155 // Disowning is done by the delegate's OnURLFetchComplete method.
128 // The file should be closed by the time that method is called. 156 // The file should be closed by the time that method is called.
129 DCHECK(!file_stream_); 157 DCHECK(!file_stream_);
130 158
131 owns_file_ = false; 159 owns_file_ = false;
132 } 160 }
133 161
134 void URLFetcherFileWriter::DidWrite(const CompletionCallback& callback,
135 int result) {
136 if (result < 0)
137 CloseAndDeleteFile();
138
139 callback.Run(result);
140 }
141
142 void URLFetcherFileWriter::CloseAndDeleteFile() { 162 void URLFetcherFileWriter::CloseAndDeleteFile() {
143 if (!owns_file_) 163 if (!owns_file_)
144 return; 164 return;
145 165
146 file_stream_.reset(); 166 file_stream_.reset();
147 DisownFile(); 167 DisownFile();
148 file_task_runner_->PostTask(FROM_HERE, 168 file_task_runner_->PostTask(FROM_HERE,
149 base::Bind(base::IgnoreResult(&base::DeleteFile), 169 base::Bind(base::IgnoreResult(&base::DeleteFile),
150 file_path_, 170 file_path_,
151 false /* recursive */)); 171 false /* recursive */));
152 } 172 }
153 173
154 void URLFetcherFileWriter::DidCreateTempFile(const CompletionCallback& callback, 174 void URLFetcherFileWriter::DidCreateTempFile(base::FilePath* temp_file_path,
155 base::FilePath* temp_file_path,
156 bool success) { 175 bool success) {
157 if (!success) { 176 if (!success) {
158 callback.Run(ERR_FILE_NOT_FOUND); 177 OnIOCompleted(ERR_FILE_NOT_FOUND);
159 return; 178 return;
160 } 179 }
161 file_path_ = *temp_file_path; 180 file_path_ = *temp_file_path;
162 owns_file_ = true;
163 const int result = file_stream_->Open( 181 const int result = file_stream_->Open(
164 file_path_, 182 file_path_,
165 base::File::FLAG_WRITE | base::File::FLAG_ASYNC | 183 base::File::FLAG_WRITE | base::File::FLAG_ASYNC | base::File::FLAG_OPEN,
166 base::File::FLAG_OPEN, 184 base::Bind(&URLFetcherFileWriter::OnIOCompleted,
167 base::Bind(&URLFetcherFileWriter::DidOpenFile, 185 weak_factory_.GetWeakPtr()));
168 weak_factory_.GetWeakPtr(),
169 callback));
170 if (result != ERR_IO_PENDING) 186 if (result != ERR_IO_PENDING)
171 DidOpenFile(callback, result); 187 OnIOCompleted(result);
172 } 188 }
173 189
174 void URLFetcherFileWriter::DidOpenFile(const CompletionCallback& callback, 190 void URLFetcherFileWriter::OnIOCompleted(int result) {
175 int result) { 191 if (result < OK)
176 if (result == OK)
177 owns_file_ = true;
178 else
179 CloseAndDeleteFile(); 192 CloseAndDeleteFile();
180 193
181 callback.Run(result); 194 if (!callback_.is_null())
195 base::ResetAndReturn(&callback_).Run(result);
182 } 196 }
183 197
184 void URLFetcherFileWriter::CloseComplete(const CompletionCallback& callback, 198 void URLFetcherFileWriter::CloseComplete(int result) {
185 int result) {
186 // Destroy |file_stream_| whether or not the close succeeded. 199 // Destroy |file_stream_| whether or not the close succeeded.
187 file_stream_.reset(); 200 file_stream_.reset();
188 callback.Run(result); 201 if (!callback_.is_null())
202 base::ResetAndReturn(&callback_).Run(result);
189 } 203 }
190 204
191 } // namespace net 205 } // namespace net
OLDNEW
« no previous file with comments | « net/url_request/url_fetcher_response_writer.h ('k') | net/url_request/url_fetcher_response_writer_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698