OLD | NEW |
---|---|
(Empty) | |
1 // Copyright (c) 2017 The Chromium Authors. All rights reserved. | |
2 // Use of this source code is governed by a BSD-style license that can be | |
3 // found in the LICENSE file. | |
4 | |
5 #include "net/http/http_cache_writers.h" | |
6 | |
7 #include <algorithm> | |
8 #include <utility> | |
9 #include "net/disk_cache/disk_cache.h" | |
10 #include "net/http/http_cache_transaction.h" | |
11 | |
12 namespace net { | |
13 | |
14 HttpCache::Writers::Writers(HttpCache* cache, ActiveEntry* entry) | |
15 : cache_(cache), entry_(entry), weak_factory_(this) { | |
16 io_callback_ = | |
17 base::Bind(&HttpCache::Writers::OnIOComplete, weak_factory_.GetWeakPtr()); | |
18 } | |
19 | |
20 HttpCache::Writers::~Writers() {} | |
21 | |
22 int HttpCache::Writers::Read(scoped_refptr<IOBuffer> buf, | |
23 int buf_len, | |
24 const CompletionCallback& callback, | |
25 Transaction* transaction) { | |
26 DCHECK(buf); | |
27 DCHECK_GT(buf_len, 0); | |
28 DCHECK(!callback.is_null()); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
nit, suggestion: DCHECK(transaction)
shivanisha
2017/05/18 20:59:44
done
| |
29 | |
30 // If another transaction is already reading from the network, then this | |
31 // transaction waits for the read to complete and gets its buffer filled | |
32 // with the data returned from that read. | |
33 if (active_transaction_) { | |
34 WaitingForRead waiting_transaction(transaction, buf, buf_len, callback); | |
35 waiting_for_read_.push_back(waiting_transaction); | |
36 return ERR_IO_PENDING; | |
37 } | |
38 | |
39 DCHECK_EQ(next_state_, State::NONE); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
This (and the test above) seem wrong to me--if an
shivanisha
2017/05/18 20:59:44
Ah, that's right. Changed the condition to next_st
Randy Smith (Not in Mondays)
2017/05/22 01:56:55
I'm not seeing the change (looking at PS6->8 diff)
shivanisha
2017/05/31 19:21:26
It is present in PS8.
Randy Smith (Not in Mondays)
2017/06/06 12:54:57
Ah, sorry, was looking at the DCHECK. Sorry.
I d
shivanisha
2017/06/07 15:19:18
Removed the dcheck.
| |
40 DCHECK(callback_.is_null()); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
This feels redundant with the below same DCHECK; d
shivanisha
2017/05/18 20:59:44
removed from below
| |
41 active_transaction_ = transaction; | |
42 | |
43 read_buf_ = std::move(buf); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
Why std::move on a scoped_refptr<>? This is just
shivanisha
2017/05/18 20:59:44
Its just taking the buffer away from the argument
Randy Smith (Not in Mondays)
2017/05/24 23:09:19
Acknowledged.
| |
44 io_buf_len_ = buf_len; | |
45 next_state_ = State::NETWORK_READ; | |
46 | |
47 int rv = DoLoop(OK); | |
48 if (rv == ERR_IO_PENDING) { | |
49 DCHECK(callback_.is_null()); | |
50 callback_ = callback; | |
51 } | |
52 | |
53 return rv; | |
54 } | |
55 | |
56 bool HttpCache::Writers::StopCaching(Transaction* transaction) { | |
57 // If this is the only transaction in Writers, then stopping will be | |
58 // successful. If not, then we will not stop caching since there are | |
59 // other consumers waiting to read from the cache. | |
60 if (all_writers_.size() == 1) { | |
61 DCHECK(all_writers_.count(transaction)); | |
62 network_read_only_ = true; | |
63 return true; | |
64 } | |
65 return false; | |
66 } | |
67 | |
68 void HttpCache::Writers::AddTransaction( | |
69 Transaction* transaction, | |
70 std::unique_ptr<HttpTransaction> network_transaction) { | |
71 DCHECK(transaction); | |
72 DCHECK(CanAddWriters()); | |
73 | |
74 auto return_val = all_writers_.insert(transaction); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
nit, suggestion: I don't find the type of return_v
shivanisha
2017/05/18 20:59:44
done
| |
75 DCHECK_EQ(return_val.second, true); | |
76 | |
77 if (network_transaction) | |
78 network_transaction_ = std::move(network_transaction); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
What drove adding the network transaction here rat
shivanisha
2017/05/18 20:59:44
I am hoping writers can be a member variable of Ac
Randy Smith (Not in Mondays)
2017/05/24 23:09:18
Hmmm. Ok. There's a range of design choices here
Randy Smith (Not in Mondays)
2017/05/24 23:09:18
Worthwhile adding a DCHECK that network_transactio
shivanisha
2017/05/31 19:21:26
Done
shivanisha
2017/05/31 19:21:26
Added the dcheck
| |
79 DCHECK(network_transaction_); | |
80 | |
81 priority_ = std::max(transaction->priority(), priority_); | |
82 network_transaction_->SetPriority(priority_); | |
83 } | |
84 | |
85 void HttpCache::Writers::RemoveTransaction(Transaction* transaction) { | |
86 if (!transaction) | |
87 return; | |
88 | |
89 // The transaction should be part of all_writers. | |
90 auto it = all_writers_.find(transaction); | |
91 DCHECK(it != all_writers_.end()); | |
92 all_writers_.erase(transaction); | |
93 PriorityChanged(); | |
94 | |
95 if (active_transaction_ == transaction) { | |
96 active_transaction_ = nullptr; | |
97 callback_.Reset(); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
Is this really all that needs to happen if we're n
shivanisha
2017/05/18 20:59:44
Updated Read to see that if next_state_ is not NON
| |
98 return; | |
99 } | |
100 | |
101 auto waiting_it = waiting_for_read_.begin(); | |
102 for (; waiting_it != waiting_for_read_.end(); waiting_it++) { | |
103 if (transaction == waiting_it->transaction) { | |
104 waiting_for_read_.erase(waiting_it); | |
105 // If a waiting transaction existed, there should have been an | |
106 // active_transaction_. | |
107 DCHECK(active_transaction_); | |
108 return; | |
109 } | |
110 } | |
111 } | |
112 | |
113 void HttpCache::Writers::RemoveAllTransactions() { | |
114 all_writers_.clear(); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
No need to nuke waiting_for_read_ or do anything w
shivanisha
2017/05/18 20:59:44
On second thoughts, I actually do not see a need f
| |
115 } | |
116 | |
117 void HttpCache::Writers::PriorityChanged() { | |
118 RequestPriority current_highest = GetCurrentHighestPriority(); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
nit, suggestion: My personal bias is that, if ther
shivanisha
2017/05/18 20:59:44
sgtm, done
| |
119 if (priority_ != current_highest) { | |
120 network_transaction_->SetPriority(current_highest); | |
121 priority_ = current_highest; | |
122 } | |
123 } | |
124 | |
125 void HttpCache::Writers::MoveIdleWritersToReaders() { | |
126 // Should be invoked after |waiting_for_read_| transactions and | |
127 // |active_transaction_| are processed so that all_writers_ only contains idle | |
128 // writers. | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
This reads to me as a requirement on the caller, a
shivanisha
2017/05/18 20:59:44
done in the comment and also added a function Cont
| |
129 DCHECK(waiting_for_read_.empty()); | |
130 DCHECK(!active_transaction_); | |
131 for (auto* idle_writer : all_writers_) { | |
132 entry_->readers.insert(idle_writer); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
Hmm. I think this is dependent on the cache entry
shivanisha
2017/05/18 20:59:44
In CL https://codereview.chromium.org/2721933002/
Randy Smith (Not in Mondays)
2017/05/22 01:56:55
SGTM.
| |
133 } | |
134 all_writers_.clear(); | |
135 } | |
136 | |
137 bool HttpCache::Writers::CanAddWriters() const { | |
138 if (all_writers_.empty()) { | |
139 DCHECK(!network_read_only_); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
Why?
shivanisha
2017/05/18 20:59:43
I believe this should be changed to network_read_o
Randy Smith (Not in Mondays)
2017/05/22 01:56:55
I'd be more inclined to say that network_read_only
shivanisha
2017/05/31 19:21:26
Added a function ResetStateForEmptyWriters which i
| |
140 return true; | |
141 } | |
142 | |
143 return !is_exclusive_ && !network_read_only_; | |
144 } | |
145 | |
146 void HttpCache::Writers::ProcessFailure(Transaction* transaction, int error) { | |
147 DCHECK(!transaction || transaction == active_transaction_); | |
148 | |
149 // Notify waiting_for_read_ of the failure. Tasks will be posted for all the | |
150 // transactions. | |
151 ProcessWaitingForReadTransactions(error); | |
152 | |
153 // Idle readers should know to fail when Read is invoked by their consumers. | |
154 SetIdleWritersFailState(error); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
Why have this logic in the consumers (HC::Ts) inst
shivanisha
2017/05/18 20:59:44
Its possible that entry is destroyed and then only
shivanisha
2017/05/19 13:49:57
Or we can doom this entry but let these transactio
Randy Smith (Not in Mondays)
2017/05/24 23:09:18
Yeah, that was the direction I think I was gesturi
shivanisha
2017/05/31 19:21:26
If there is a failure then waiting_for_read_ trans
Randy Smith (Not in Mondays)
2017/06/06 12:54:57
Makes sense. Ok, I'm slightly in favor at the arc
shivanisha
2017/06/07 15:19:18
Changed the logic to keep this state in Writers it
shivanisha
2017/06/08 13:38:41
Actually while working on the follow up CL, a use
Randy Smith (Not in Mondays)
2017/06/08 17:10:00
Accepted if that's how you decide that's the best
shivanisha
2017/06/08 17:55:56
We actually want future reads on the active_transa
Randy Smith (Not in Mondays)
2017/06/13 17:45:05
Ah, makes sense. Drat.
| |
155 DCHECK(all_writers_.empty()); | |
156 } | |
157 | |
158 void HttpCache::Writers::TruncateEntry() { | |
159 // TODO(shivanisha) On integration, see if the entry really needs to be | |
160 // truncated on the lines of Transaction::AddTruncatedFlag and then proceed. | |
161 DCHECK_EQ(next_state_, State::NONE); | |
162 next_state_ = State::CACHE_WRITE_TRUNCATED_RESPONSE; | |
163 DoLoop(OK); | |
164 } | |
165 | |
166 LoadState HttpCache::Writers::GetWriterLoadState() { | |
167 return network_transaction_->GetLoadState(); | |
168 } | |
169 | |
170 HttpCache::Writers::WaitingForRead::WaitingForRead( | |
171 Transaction* cache_transaction, | |
172 scoped_refptr<IOBuffer> buf, | |
173 int len, | |
174 const CompletionCallback& consumer_callback) | |
175 : transaction(cache_transaction), | |
176 read_buf(std::move(buf)), | |
177 read_buf_len(len), | |
178 write_len(0), | |
179 callback(consumer_callback) { | |
180 DCHECK(cache_transaction); | |
181 DCHECK(buf); | |
182 DCHECK_GT(len, 0); | |
183 DCHECK(!consumer_callback.is_null()); | |
184 } | |
185 | |
186 HttpCache::Writers::WaitingForRead::~WaitingForRead() {} | |
187 HttpCache::Writers::WaitingForRead::WaitingForRead(const WaitingForRead&) = | |
188 default; | |
189 | |
190 int HttpCache::Writers::DoLoop(int result) { | |
191 DCHECK(next_state_ != State::NONE); | |
192 int rv = result; | |
193 do { | |
194 State state = next_state_; | |
195 next_state_ = State::NONE; | |
196 switch (state) { | |
197 case State::NETWORK_READ: | |
198 DCHECK_EQ(OK, rv); | |
199 rv = DoNetworkRead(); | |
200 break; | |
201 case State::NETWORK_READ_COMPLETE: | |
202 rv = DoNetworkReadComplete(rv); | |
203 break; | |
204 case State::CACHE_WRITE_DATA: | |
205 rv = DoCacheWriteData(rv); | |
206 break; | |
207 case State::CACHE_WRITE_DATA_COMPLETE: | |
208 rv = DoCacheWriteDataComplete(rv); | |
209 break; | |
210 case State::CACHE_WRITE_TRUNCATED_RESPONSE: | |
211 rv = DoCacheWriteTruncatedResponse(); | |
212 break; | |
213 case State::CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE: | |
214 rv = DoCacheWriteTruncatedResponseComplete(rv); | |
215 break; | |
216 default: | |
217 NOTREACHED() << "bad state"; | |
218 rv = ERR_FAILED; | |
219 break; | |
220 } | |
221 } while (next_state_ != State::NONE && rv != ERR_IO_PENDING); | |
222 | |
223 if (rv != ERR_IO_PENDING && !callback_.is_null()) { | |
224 read_buf_ = NULL; // Release the buffer before invoking the callback. | |
225 base::ResetAndReturn(&callback_).Run(rv); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
nit, suggestion: The pattern I've seen more common
shivanisha
2017/05/18 20:59:44
I guess I am only much aware of the way it is hand
Randy Smith (Not in Mondays)
2017/05/24 23:09:18
Acknowledged.
| |
226 } | |
227 return rv; | |
228 } | |
229 | |
230 int HttpCache::Writers::DoNetworkRead() { | |
231 next_state_ = State::NETWORK_READ_COMPLETE; | |
232 return network_transaction_->Read(read_buf_.get(), io_buf_len_, io_callback_); | |
233 } | |
234 | |
235 int HttpCache::Writers::DoNetworkReadComplete(int result) { | |
236 if (result < 0) { | |
237 OnNetworkReadFailure(result); | |
238 return result; | |
239 } | |
240 | |
241 next_state_ = State::CACHE_WRITE_DATA; | |
242 return result; | |
243 } | |
244 | |
245 void HttpCache::Writers::OnNetworkReadFailure(int result) { | |
246 // At this point active_transaction_ may or may not be alive. | |
247 // If no consumer, invoke entry processing here itself. | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
suggestion, possibly not for this CL: I'd rather h
shivanisha
2017/05/18 20:59:43
I will look at this suggestion again in the integr
shivanisha
2017/05/19 13:49:57
Actually on second thoughts, its simpler to invoke
| |
248 if (!active_transaction_) | |
249 cache_->DoneWithEntry(entry_, nullptr, true); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
Is there some path from here to ProcessFailure? H
shivanisha
2017/05/18 20:59:44
DoneWithEntry => DoneWritingToEntry => ProcessFail
Randy Smith (Not in Mondays)
2017/05/24 23:09:18
Acknowledged.
| |
250 } | |
251 | |
252 int HttpCache::Writers::DoCacheWriteData(int num_bytes) { | |
253 next_state_ = State::CACHE_WRITE_DATA_COMPLETE; | |
254 write_len_ = num_bytes; | |
255 if (!num_bytes || network_read_only_) | |
256 return num_bytes; | |
257 | |
258 int current_size = entry_->disk_entry->GetDataSize(kResponseContentIndex); | |
259 return WriteToEntry(kResponseContentIndex, current_size, read_buf_.get(), | |
260 num_bytes, io_callback_); | |
261 } | |
262 | |
263 int HttpCache::Writers::WriteToEntry(int index, | |
264 int offset, | |
265 IOBuffer* data, | |
266 int data_len, | |
267 const CompletionCallback& callback) { | |
268 int rv = 0; | |
269 | |
270 PartialData* partial = nullptr; | |
271 // Transaction must be alive if this is a partial request. | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
This isn't backed up with a DCHECK, and I'm not su
shivanisha
2017/05/18 20:59:43
In case of partial requests, SetExclusive will be
Randy Smith (Not in Mondays)
2017/05/24 23:09:18
Thanks for the explanation.
(nit, suggestion:) I
shivanisha
2017/05/31 19:21:26
done
| |
272 // Todo(shivanisha): When partial requests support parallel writing, this | |
273 // assumption will not be true. | |
274 if (active_transaction_) | |
275 partial = active_transaction_->partial(); | |
276 | |
277 if (!partial || !data_len) { | |
278 rv = entry_->disk_entry->WriteData(index, offset, data, data_len, callback, | |
279 true); | |
280 } else { | |
281 rv = partial->CacheWrite(entry_->disk_entry, data, data_len, callback); | |
282 } | |
283 return rv; | |
284 } | |
285 | |
286 int HttpCache::Writers::DoCacheWriteDataComplete(int result) { | |
287 if (result != write_len_) { | |
288 OnCacheWriteFailure(); | |
289 | |
290 // |active_transaction_| can continue reading from the network. | |
291 result = write_len_; | |
292 } else { | |
293 OnDataReceived(result); | |
294 } | |
295 return result; | |
296 } | |
297 | |
298 int HttpCache::Writers::DoCacheWriteTruncatedResponse() { | |
299 next_state_ = State::CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE; | |
300 return WriteResponseInfo(true); | |
301 } | |
302 | |
303 int HttpCache::Writers::DoCacheWriteTruncatedResponseComplete(int result) { | |
304 if (result != io_buf_len_) { | |
305 DLOG(ERROR) << "failed to write response info to cache"; | |
306 } | |
307 truncated_ = true; | |
308 return OK; | |
309 } | |
310 | |
311 void HttpCache::Writers::OnDataReceived(int result) { | |
312 if (result == 0 && !active_transaction_) { | |
313 // Check if the response is actually completed or if not, attempt to mark | |
314 // the entry as truncated. | |
315 if (IsResponseCompleted()) { | |
316 ProcessWaitingForReadTransactions(result); | |
317 cache_->DoneWritingToEntry(entry_, true); | |
318 } else { | |
319 OnNetworkReadFailure(result); | |
320 return; | |
321 } | |
322 } | |
323 | |
324 // Save the data in all the waiting transactions' read buffers. | |
325 for (auto it = waiting_for_read_.begin(); it != waiting_for_read_.end(); | |
326 it++) { | |
327 it->write_len = std::min(it->read_buf_len, result); | |
328 memcpy(it->read_buf->data(), read_buf_->data(), it->write_len); | |
329 } | |
330 | |
331 // Notify waiting_for_read_. Tasks will be posted for all the | |
332 // transactions. | |
333 ProcessWaitingForReadTransactions(write_len_); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
nit, suggestion: The grouping would feel slightly
shivanisha
2017/05/19 13:49:57
done
| |
334 | |
335 if (result > 0) { // not the end of response | |
336 active_transaction_ = nullptr; | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
Why keep the active transaction set if the end of
shivanisha
2017/05/19 13:49:57
Now that DoneWithEntry/DoneWritingToEntry will be
| |
337 return; | |
338 } | |
339 DCHECK_EQ(result, 0); | |
340 if (!active_transaction_) | |
341 cache_->DoneWritingToEntry(entry_, true); | |
342 } | |
343 | |
344 void HttpCache::Writers::OnCacheWriteFailure() { | |
345 network_read_only_ = true; | |
346 | |
347 // Call the cache_ function here even if active_transaction_ is alive because | |
348 // it wouldn't know if this was an error case, since it gets a positive result | |
349 // back. This will in turn ProcessFailure. | |
350 // TODO(shivanisha) : Also pass active_transaction_ and | |
351 // ERR_CACHE_WRITE_FAILURE to DoneWritingToEntry on integration so that it can | |
352 // be passed back in ProcessFailure. | |
353 cache_->DoneWritingToEntry(entry_, false); | |
354 } | |
355 | |
356 bool HttpCache::Writers::IsResponseCompleted() { | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
I'm a bit uncomfortable with this function's name;
shivanisha
2017/05/19 13:49:57
Moved it to its call site.
| |
357 int current_size = entry_->disk_entry->GetDataSize(kResponseContentIndex); | |
358 const HttpResponseInfo* response_info = | |
359 network_transaction_->GetResponseInfo(); | |
360 int64_t content_length = response_info->headers->GetContentLength(); | |
361 if ((content_length >= 0 && content_length <= current_size) || | |
362 content_length < 0) | |
363 return true; | |
364 return false; | |
365 } | |
366 | |
367 int HttpCache::Writers::WriteResponseInfo(bool truncated) { | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
Again, I'd vote in favor of hoisting this into the
shivanisha
2017/05/19 13:49:57
Moved to calling site. I had this as separate func
| |
368 // When writing headers, we normally only write the non-transient headers. | |
369 const HttpResponseInfo* response = network_transaction_->GetResponseInfo(); | |
370 bool skip_transient_headers = true; | |
371 scoped_refptr<PickledIOBuffer> data(new PickledIOBuffer()); | |
372 response->Persist(data->pickle(), skip_transient_headers, truncated); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
nit, suggestion: I'd just write this as
response
shivanisha
2017/05/19 13:49:57
done
| |
373 data->Done(); | |
374 io_buf_len_ = data->pickle()->size(); | |
375 return entry_->disk_entry->WriteData(kResponseInfoIndex, 0, data.get(), | |
376 io_buf_len_, io_callback_, truncated); | |
377 } | |
378 | |
379 void HttpCache::Writers::ProcessWaitingForReadTransactions(int result) { | |
380 for (auto it = waiting_for_read_.begin(); it != waiting_for_read_.end(); | |
381 it++) { | |
382 Transaction* transaction = it->transaction; | |
383 if (result > 0) { // success | |
384 result = it->write_len; | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:55
(Just an addition to an earlier comment: Setting r
shivanisha
2017/05/19 13:49:57
Moved copying to this function.
| |
385 } else { | |
386 // If its response completion or failure, this transaction needs to be | |
387 // removed. | |
388 all_writers_.erase(transaction); | |
Randy Smith (Not in Mondays)
2017/05/18 01:02:54
Caption: This comment is an extended babble with m
shivanisha
2017/05/19 13:49:58
In case transaction B comes back , reads from netw
shivanisha
2017/05/19 17:12:58
Actually, keeping all_writers_ is important from a
Randy Smith (Not in Mondays)
2017/05/22 01:56:55
Acknowledged.
| |
389 } | |
390 // Post task to notify transaction. | |
391 base::ThreadTaskRunnerHandle::Get()->PostTask( | |
392 FROM_HERE, base::Bind(it->callback, result)); | |
393 } | |
394 waiting_for_read_.clear(); | |
395 } | |
396 | |
397 void HttpCache::Writers::SetIdleWritersFailState(int result) { | |
398 // Since this is only for idle transactions, all waiting_for_read_ and | |
399 // active_transaction_ should be empty. | |
400 DCHECK(waiting_for_read_.empty()); | |
401 DCHECK(!active_transaction_); | |
402 for (auto* transaction : all_writers_) | |
403 transaction->SetSharedWritingFailState(result); | |
404 all_writers_.clear(); | |
405 } | |
406 | |
407 RequestPriority HttpCache::Writers::GetCurrentHighestPriority() { | |
408 RequestPriority priority = MINIMUM_PRIORITY; | |
409 for (auto* transaction : all_writers_) | |
410 priority = std::max(transaction->priority(), priority); | |
411 return priority; | |
412 } | |
413 | |
414 void HttpCache::Writers::OnIOComplete(int result) { | |
415 DoLoop(result); | |
416 } | |
417 | |
418 } // namespace net | |
OLD | NEW |