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

Side by Side Diff: content/browser/streams/stream_url_request_job.cc

Issue 1467603002: URLRequestJob: change ReadRawData contract (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: add done_reading_called_ to MockNetworkTransaction Created 5 years, 1 month 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 "content/browser/streams/stream_url_request_job.h" 5 #include "content/browser/streams/stream_url_request_job.h"
6 6
7 #include "base/location.h" 7 #include "base/location.h"
8 #include "base/single_thread_task_runner.h" 8 #include "base/single_thread_task_runner.h"
9 #include "base/strings/string_number_conversions.h" 9 #include "base/strings/string_number_conversions.h"
10 #include "base/thread_task_runner_handle.h" 10 #include "base/thread_task_runner_handle.h"
(...skipping 22 matching lines...) Expand all
33 weak_factory_(this) { 33 weak_factory_(this) {
34 DCHECK(stream_.get()); 34 DCHECK(stream_.get());
35 stream_->SetReadObserver(this); 35 stream_->SetReadObserver(this);
36 } 36 }
37 37
38 StreamURLRequestJob::~StreamURLRequestJob() { 38 StreamURLRequestJob::~StreamURLRequestJob() {
39 ClearStream(); 39 ClearStream();
40 } 40 }
41 41
42 void StreamURLRequestJob::OnDataAvailable(Stream* stream) { 42 void StreamURLRequestJob::OnDataAvailable(Stream* stream) {
43 // Clear the IO_PENDING status.
44 SetStatus(net::URLRequestStatus());
45 // Do nothing if pending_buffer_ is empty, i.e. there's no ReadRawData() 43 // Do nothing if pending_buffer_ is empty, i.e. there's no ReadRawData()
46 // operation waiting for IO completion. 44 // operation waiting for IO completion.
47 if (!pending_buffer_.get()) 45 if (!pending_buffer_.get())
48 return; 46 return;
49 47
50 // pending_buffer_ is set to the IOBuffer instance provided to ReadRawData() 48 // pending_buffer_ is set to the IOBuffer instance provided to ReadRawData()
51 // by URLRequestJob. 49 // by URLRequestJob.
52 50
53 int bytes_read; 51 int result = 0;
54 switch (stream_->ReadRawData(pending_buffer_.get(), pending_buffer_size_, 52 switch (stream_->ReadRawData(pending_buffer_.get(), pending_buffer_size_,
55 &bytes_read)) { 53 &result)) {
56 case Stream::STREAM_HAS_DATA: 54 case Stream::STREAM_HAS_DATA:
57 DCHECK_GT(bytes_read, 0); 55 DCHECK_GT(result, 0);
58 break; 56 break;
59 case Stream::STREAM_COMPLETE: 57 case Stream::STREAM_COMPLETE:
60 // Ensure this. Calling NotifyReadComplete call with 0 signals 58 // Ensure ReadRawData gives net::OK.
61 // completion. 59 DCHECK_EQ(net::OK, result);
62 bytes_read = 0;
63 break; 60 break;
64 case Stream::STREAM_EMPTY: 61 case Stream::STREAM_EMPTY:
65 NOTREACHED(); 62 NOTREACHED();
66 break; 63 break;
67 case Stream::STREAM_ABORTED: 64 case Stream::STREAM_ABORTED:
68 // Handle this as connection reset. 65 // Handle this as connection reset.
69 NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, 66 result = net::ERR_CONNECTION_RESET;
70 net::ERR_CONNECTION_RESET));
71 break; 67 break;
72 } 68 }
73 69
74 // Clear the buffers before notifying the read is complete, so that it is 70 // Clear the buffers before notifying the read is complete, so that it is
75 // safe for the observer to read. 71 // safe for the observer to read.
76 pending_buffer_ = NULL; 72 pending_buffer_ = NULL;
77 pending_buffer_size_ = 0; 73 pending_buffer_size_ = 0;
78 74
79 total_bytes_read_ += bytes_read; 75 if (result > 0)
80 NotifyReadComplete(bytes_read); 76 total_bytes_read_ += result;
77 ReadRawDataComplete(result);
81 } 78 }
82 79
83 // net::URLRequestJob methods. 80 // net::URLRequestJob methods.
84 void StreamURLRequestJob::Start() { 81 void StreamURLRequestJob::Start() {
85 // Continue asynchronously. 82 // Continue asynchronously.
86 base::ThreadTaskRunnerHandle::Get()->PostTask( 83 base::ThreadTaskRunnerHandle::Get()->PostTask(
87 FROM_HERE, 84 FROM_HERE,
88 base::Bind(&StreamURLRequestJob::DidStart, weak_factory_.GetWeakPtr())); 85 base::Bind(&StreamURLRequestJob::DidStart, weak_factory_.GetWeakPtr()));
89 } 86 }
90 87
91 void StreamURLRequestJob::Kill() { 88 void StreamURLRequestJob::Kill() {
92 net::URLRequestJob::Kill(); 89 net::URLRequestJob::Kill();
93 weak_factory_.InvalidateWeakPtrs(); 90 weak_factory_.InvalidateWeakPtrs();
94 ClearStream(); 91 ClearStream();
95 } 92 }
96 93
97 bool StreamURLRequestJob::ReadRawData(net::IOBuffer* buf, 94 int StreamURLRequestJob::ReadRawData(net::IOBuffer* buf, int buf_size) {
98 int buf_size, 95 // TODO(ellyjones): This is not right. The old code returned true here, but
99 int* bytes_read) { 96 // ReadRawData's old contract was to return true only for synchronous
97 // successes, which had the effect of treating all errors as synchronous EOFs.
98 // See https://crbug.com/508957
100 if (request_failed_) 99 if (request_failed_)
101 return true; 100 return 0;
102 101
103 DCHECK(buf); 102 DCHECK(buf);
104 DCHECK(bytes_read);
105 int to_read = buf_size; 103 int to_read = buf_size;
106 if (max_range_ && to_read) { 104 if (max_range_ && to_read) {
107 if (to_read + total_bytes_read_ > max_range_) 105 if (to_read + total_bytes_read_ > max_range_)
108 to_read = max_range_ - total_bytes_read_; 106 to_read = max_range_ - total_bytes_read_;
109 107
110 if (to_read <= 0) { 108 if (to_read == 0)
111 *bytes_read = 0; 109 return 0;
112 return true;
113 }
114 } 110 }
115 111
116 switch (stream_->ReadRawData(buf, to_read, bytes_read)) { 112 int bytes_read = 0;
113 switch (stream_->ReadRawData(buf, to_read, &bytes_read)) {
117 case Stream::STREAM_HAS_DATA: 114 case Stream::STREAM_HAS_DATA:
118 case Stream::STREAM_COMPLETE: 115 case Stream::STREAM_COMPLETE:
119 total_bytes_read_ += *bytes_read; 116 total_bytes_read_ += bytes_read;
120 return true; 117 return bytes_read;
121 case Stream::STREAM_EMPTY: 118 case Stream::STREAM_EMPTY:
122 pending_buffer_ = buf; 119 pending_buffer_ = buf;
123 pending_buffer_size_ = to_read; 120 pending_buffer_size_ = to_read;
124 SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0)); 121 return net::ERR_IO_PENDING;
125 return false;
126 case Stream::STREAM_ABORTED: 122 case Stream::STREAM_ABORTED:
127 // Handle this as connection reset. 123 // Handle this as connection reset.
128 NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, 124 return net::ERR_CONNECTION_RESET;
129 net::ERR_CONNECTION_RESET));
130 return false;
131 } 125 }
132 NOTREACHED(); 126 NOTREACHED();
133 return false; 127 return net::ERR_FAILED;
134 } 128 }
135 129
136 bool StreamURLRequestJob::GetMimeType(std::string* mime_type) const { 130 bool StreamURLRequestJob::GetMimeType(std::string* mime_type) const {
137 if (!response_info_) 131 if (!response_info_)
138 return false; 132 return false;
139 133
140 // TODO(zork): Support registered MIME types if needed. 134 // TODO(zork): Support registered MIME types if needed.
141 return response_info_->headers->GetMimeType(mime_type); 135 return response_info_->headers->GetMimeType(mime_type);
142 } 136 }
143 137
(...skipping 38 matching lines...) Expand 10 before | Expand all | Expand 10 after
182 NotifyFailure(net::ERR_METHOD_NOT_SUPPORTED); 176 NotifyFailure(net::ERR_METHOD_NOT_SUPPORTED);
183 return; 177 return;
184 } 178 }
185 179
186 HeadersCompleted(net::HTTP_OK); 180 HeadersCompleted(net::HTTP_OK);
187 } 181 }
188 182
189 void StreamURLRequestJob::NotifyFailure(int error_code) { 183 void StreamURLRequestJob::NotifyFailure(int error_code) {
190 request_failed_ = true; 184 request_failed_ = true;
191 185
192 // If we already return the headers on success, we can't change the headers 186 // This method can only be called before headers are set.
193 // now. Instead, we just error out. 187 DCHECK(!headers_set_);
194 if (headers_set_) {
195 NotifyDone(
196 net::URLRequestStatus(net::URLRequestStatus::FAILED, error_code));
197 return;
198 }
199 188
200 // TODO(zork): Share these with BlobURLRequestJob. 189 // TODO(zork): Share these with BlobURLRequestJob.
201 net::HttpStatusCode status_code = net::HTTP_INTERNAL_SERVER_ERROR; 190 net::HttpStatusCode status_code = net::HTTP_INTERNAL_SERVER_ERROR;
202 switch (error_code) { 191 switch (error_code) {
203 case net::ERR_ACCESS_DENIED: 192 case net::ERR_ACCESS_DENIED:
204 status_code = net::HTTP_FORBIDDEN; 193 status_code = net::HTTP_FORBIDDEN;
205 break; 194 break;
206 case net::ERR_FILE_NOT_FOUND: 195 case net::ERR_FILE_NOT_FOUND:
207 status_code = net::HTTP_NOT_FOUND; 196 status_code = net::HTTP_NOT_FOUND;
208 break; 197 break;
(...skipping 33 matching lines...) Expand 10 before | Expand all | Expand 10 after
242 } 231 }
243 232
244 void StreamURLRequestJob::ClearStream() { 233 void StreamURLRequestJob::ClearStream() {
245 if (stream_.get()) { 234 if (stream_.get()) {
246 stream_->RemoveReadObserver(this); 235 stream_->RemoveReadObserver(this);
247 stream_ = NULL; 236 stream_ = NULL;
248 } 237 }
249 } 238 }
250 239
251 } // namespace content 240 } // namespace content
OLDNEW
« no previous file with comments | « content/browser/streams/stream_url_request_job.h ('k') | content/browser/webui/url_data_manager_backend.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698