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

Issue 2886483002: Adds a new class HttpCache::Writers for multiple cache transactions reading from the network. (Closed)

Created:
3 years, 7 months ago by shivanisha
Modified:
3 years, 5 months ago
CC:
cbentzel+watch_chromium.org, chromium-reviews, gavinp+disk_chromium.org, net-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

This CL adds a new class HttpCache::Writers which will implement multiple cache transactions reading from the network and writing to the cache enabling each of those transactions to drive reading the response body from the network. This ensures that a slow consumer does not starve other consumers of the same resource. This CL implements the Writers class in isolation and does not create an instance or integrates it with HttpCache/HttpCache::Transaction. In a follow up CL which will be dependent on parallel validation CL (https://codereview.chromium.org/2721933002/), a Writer object will replace entry- >writer. In case of transaction types that do not support shared writing like partial transactions, Writers will only contain one transaction at a time and will have is_exclusive set to true. The flow of Read() call will be: URLRequestHttpJob -> HttpCache::Transaction::Read() -> HttpCache::Writers::Read() -> HttpNetworkTransaction::Read() HttpCache::Writers will invoke HC::T's callback after writing the read buffer to the cache successfully or on network read failure/ cache write failure. So, in essence, HttpCache::Writers::Read() abstracts away the DoNetworkRead* and DoCacheWriteData* functions of HC::T HttpCache::Transaction that has invoked Read() is the |active_transaction_| and may be killed at any time by it's consumer. When a transaction completes writing to the cache, either failure or success, it invokes HttpCache::DoneWritingToEntry as it does now which will invoke the required functionality in Writers. But If |active_transaction_| does not exist, Writers object invokes HttpCache::DoneWritingToEntry itself. BUG=472740 Review-Url: https://codereview.chromium.org/2886483002 Cr-Commit-Position: refs/heads/master@{#486907} Committed: https://chromium.googlesource.com/chromium/src/+/c6582e1924c76f68e2b8873712cdf26a0b0c1edb

Patch Set 1 : Initial patch #

Patch Set 2 : copyright + include + lint suggestions #

Patch Set 3 : Unit tests added + minor changes #

Total comments: 24

Patch Set 4 : rdsmith feedback #

Total comments: 1

Patch Set 5 : Windows compile error fixed with NET_EXPORT_PRIVATE #

Patch Set 6 : AddTransaction to take unique_ptr of network transaction. #

Total comments: 92

Patch Set 7 : Feedback addressed #

Patch Set 8 : Feedback addressed #

Total comments: 4

Patch Set 9 : rdsmith feedback #

Patch Set 10 : Rebased till refs/heads/master@{#475981} #

Total comments: 50

Patch Set 11 : Feedback addressed #

Total comments: 52

Patch Set 12 : rdsmith feedback addressed #

Patch Set 13 : Addressed all feedback (includes new tests) #

Patch Set 14 : Rebased with refs/heads/master@{#479258} #

Total comments: 32

Patch Set 15 : Feedback addressed #

Total comments: 12

Patch Set 16 : Rebased with refs/heads/master@{#482382} #

Patch Set 17 : Tests feedback addressed #

Patch Set 18 : Win compile errors fixed #

Patch Set 19 : Converted test class into a test fixture class. #

Patch Set 20 : Fix test class memory leak #

Total comments: 110

Patch Set 21 : Test memory leak + rdsmith feedback addressed #

Patch Set 22 : jkarlin feebdack addressed #

Total comments: 12

Patch Set 23 : rdsmith@ feedback addressed #

Patch Set 24 : jkarlin@ feedback addressed. #

Total comments: 26

Patch Set 25 : Rebased with refs/heads/master@{#484325} #

Patch Set 26 : Added MockHttpCacheTransaction #

Patch Set 27 : Feedback addressed. #

Total comments: 36

Patch Set 28 : Windows compile error fixed. #

Patch Set 29 : Feedback addressed #

Total comments: 2

Patch Set 30 : Rebased with refs/heads/master@{#486443} #

Patch Set 31 : Comment changed. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1514 lines, -52 lines) Patch
M net/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 2 chunks +3 lines, -0 lines 0 comments Download
M net/http/http_cache.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 27 28 29 1 chunk +3 lines, -0 lines 0 comments Download
M net/http/http_cache_lookup_manager_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +1 line, -1 line 0 comments Download
M net/http/http_cache_transaction.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 2 chunks +7 lines, -1 line 0 comments Download
M net/http/http_cache_transaction.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 2 chunks +7 lines, -2 lines 0 comments Download
M net/http/http_cache_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 27 28 29 9 chunks +10 lines, -10 lines 0 comments Download
A net/http/http_cache_writers.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +227 lines, -0 lines 0 comments Download
A net/http/http_cache_writers.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +443 lines, -0 lines 0 comments Download
A net/http/http_cache_writers_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 1 chunk +664 lines, -0 lines 0 comments Download
M net/http/http_transaction_test_util.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +5 lines, -1 line 0 comments Download
M net/http/http_transaction_test_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 5 chunks +56 lines, -21 lines 0 comments Download
M net/url_request/url_request_job_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +88 lines, -16 lines 0 comments Download

Messages

Total messages: 156 (99 generated)
shivanisha
Josh, Randy, PTAL, Thanks! This CL adds a new class HttpCache::Writers which will implement multiple ...
3 years, 7 months ago (2017-05-15 17:25:02 UTC) #6
jkarlin
On 2017/05/15 17:25:02, shivanisha wrote: > Josh, Randy, PTAL, Thanks! > > This CL adds ...
3 years, 7 months ago (2017-05-15 17:44:11 UTC) #7
shivanisha
On 2017/05/15 at 17:44:11, jkarlin wrote: > On 2017/05/15 17:25:02, shivanisha wrote: > > Josh, ...
3 years, 7 months ago (2017-05-15 18:21:40 UTC) #9
shivanisha
PTAL, Thanks! The latest patch includes unit tests. Tests cover the independent aspects of this ...
3 years, 7 months ago (2017-05-16 20:30:44 UTC) #17
Randy Smith (Not in Mondays)
Initial comments, primarily focussed on the interface contract. https://codereview.chromium.org/2886483002/diff/100001/net/http/http_cache_writers.h File net/http/http_cache_writers.h (right): https://codereview.chromium.org/2886483002/diff/100001/net/http/http_cache_writers.h#newcode18 net/http/http_cache_writers.h:18: // ...
3 years, 7 months ago (2017-05-16 22:35:29 UTC) #20
shivanisha
PTAL, Thanks! The latest patch has Randy's feedback addressed. https://codereview.chromium.org/2886483002/diff/100001/net/http/http_cache_writers.h File net/http/http_cache_writers.h (right): https://codereview.chromium.org/2886483002/diff/100001/net/http/http_cache_writers.h#newcode18 net/http/http_cache_writers.h:18: ...
3 years, 7 months ago (2017-05-17 13:02:31 UTC) #24
Randy Smith (Not in Mondays)
Next step of comments, this primarily being about the implementation. I may need to talk ...
3 years, 7 months ago (2017-05-18 01:02:56 UTC) #31
shivanisha
PTAL, Thanks! Addressed some of the feedback. Will be responding to the remaining in the ...
3 years, 7 months ago (2017-05-18 20:59:44 UTC) #33
shivanisha
PTAL, Thanks! Latest patch has all of the feedback addressed. https://codereview.chromium.org/2886483002/diff/100001/net/http/http_cache_writers.h File net/http/http_cache_writers.h (right): https://codereview.chromium.org/2886483002/diff/100001/net/http/http_cache_writers.h#newcode93 ...
3 years, 7 months ago (2017-05-19 13:49:58 UTC) #36
shivanisha
https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc#newcode388 net/http/http_cache_writers.cc:388: all_writers_.erase(transaction); On 2017/05/19 at 13:49:58, shivanisha wrote: > On ...
3 years, 7 months ago (2017-05-19 17:12:59 UTC) #39
Randy Smith (Not in Mondays)
Sorry, I had intended to get this code review done before the end of the ...
3 years, 7 months ago (2017-05-22 01:56:56 UTC) #40
Randy Smith (Not in Mondays)
Shivani: Full followup on existing comments. Let's run through these, then I'll do another from ...
3 years, 7 months ago (2017-05-24 23:09:19 UTC) #41
shivanisha
PTAL, Thanks! Latest patch has Randy's feedback addressed. https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc#newcode39 net/http/http_cache_writers.cc:39: DCHECK_EQ(next_state_, ...
3 years, 6 months ago (2017-05-31 19:21:27 UTC) #44
Randy Smith (Not in Mondays)
Next round of comments. https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc#newcode39 net/http/http_cache_writers.cc:39: DCHECK_EQ(next_state_, State::NONE); On 2017/05/31 19:21:26, ...
3 years, 6 months ago (2017-06-06 12:54:58 UTC) #45
Randy Smith (Not in Mondays)
Next round of comments.
3 years, 6 months ago (2017-06-06 12:55:33 UTC) #46
Randy Smith (Not in Mondays)
On 2017/06/06 12:55:33, Randy Smith (Not in Mondays) wrote: > Next round of comments. Sorry, ...
3 years, 6 months ago (2017-06-06 13:54:19 UTC) #47
shivanisha
PTAL, Thanks! Has the latest feedback addressed. https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc#newcode39 net/http/http_cache_writers.cc:39: DCHECK_EQ(next_state_, State::NONE); ...
3 years, 6 months ago (2017-06-07 15:19:19 UTC) #55
shivanisha
https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc#newcode154 net/http/http_cache_writers.cc:154: SetIdleWritersFailState(error); On 2017/06/07 at 15:19:18, shivanisha wrote: > On ...
3 years, 6 months ago (2017-06-08 13:38:41 UTC) #58
Randy Smith (Not in Mondays)
The basic functionality is looking good modulo integration (it's hard to evaluate interface contracts without ...
3 years, 6 months ago (2017-06-08 17:10:01 UTC) #59
shivanisha
Responding to some of the comments that did not need a code change. https://codereview.chromium.org/2886483002/diff/160001/net/http/http_cache_writers.cc File ...
3 years, 6 months ago (2017-06-08 17:55:56 UTC) #60
jkarlin
https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_transaction.h File net/http/http_cache_transaction.h (right): https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_transaction.h#newcode170 net/http/http_cache_transaction.h:170: RequestPriority priority() { return priority_; } this should be ...
3 years, 6 months ago (2017-06-12 18:30:33 UTC) #61
shivanisha
Patch 12 addresses Randy's rest of the feedback. https://codereview.chromium.org/2886483002/diff/260001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/260001/net/http/http_cache_writers.cc#newcode238 net/http/http_cache_writers.cc:238: read_buf_ ...
3 years, 6 months ago (2017-06-12 18:51:15 UTC) #64
Randy Smith (Not in Mondays)
I'm happy with the non-test code in this CL, modulo WriteToEntry(offset, ...) documentation/elimination. It sounds ...
3 years, 6 months ago (2017-06-13 17:45:05 UTC) #67
shivanisha
PTAL, Thanks! Addressed all feedback and included new tests. https://codereview.chromium.org/2886483002/diff/260001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/260001/net/http/http_cache_writers.cc#newcode169 net/http/http_cache_writers.cc:169: ...
3 years, 6 months ago (2017-06-14 02:33:17 UTC) #68
Randy Smith (Not in Mondays)
Initial comments on tests. https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc#newcode20 net/http/http_cache_writers_unittest.cc:20: class HttpCache::WritersTest { On 2017/06/14 ...
3 years, 6 months ago (2017-06-14 19:49:18 UTC) #73
jkarlin
https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers.h File net/http/http_cache_writers.h (right): https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers.h#newcode149 net/http/http_cache_writers.h:149: On 2017/06/14 02:33:17, shivanisha wrote: > On 2017/06/12 at ...
3 years, 6 months ago (2017-06-14 20:03:14 UTC) #74
Randy Smith (Not in Mondays)
https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc#newcode34 net/http/http_cache_writers_unittest.cc:34: writers_ = base::MakeUnique<Writers>(cache, entry); On 2017/06/14 20:03:14, jkarlin wrote: ...
3 years, 6 months ago (2017-06-14 20:10:32 UTC) #75
jkarlin
https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc#newcode34 net/http/http_cache_writers_unittest.cc:34: writers_ = base::MakeUnique<Writers>(cache, entry); > If writers_ could exist ...
3 years, 6 months ago (2017-06-14 20:28:37 UTC) #76
shivanisha
PTAL, Thanks! Feedback addressed. https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc#newcode20 net/http/http_cache_writers_unittest.cc:20: class HttpCache::WritersTest { On 2017/06/14 ...
3 years, 6 months ago (2017-06-15 20:17:46 UTC) #82
shivanisha
https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc#newcode34 net/http/http_cache_writers_unittest.cc:34: writers_ = base::MakeUnique<Writers>(cache, entry); On 2017/06/14 at 20:28:37, jkarlin ...
3 years, 6 months ago (2017-06-15 20:46:27 UTC) #83
Randy Smith (Not in Mondays)
Next round of comments on tests. Converging :-}. https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc#newcode20 net/http/http_cache_writers_unittest.cc:20: class ...
3 years, 6 months ago (2017-06-22 21:45:06 UTC) #86
jkarlin
https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc#newcode34 net/http/http_cache_writers_unittest.cc:34: writers_ = base::MakeUnique<Writers>(cache, entry); On 2017/06/22 21:45:05, Randy Smith ...
3 years, 5 months ago (2017-06-26 16:58:42 UTC) #87
shivanisha
PTAL, Thanks! Last 2 patches rebase and address test feedback respectively. https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): ...
3 years, 5 months ago (2017-06-26 20:45:35 UTC) #91
shivanisha
https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc#newcode20 net/http/http_cache_writers_unittest.cc:20: class HttpCache::WritersTest { On 2017/06/22 at 21:45:05, Randy Smith ...
3 years, 5 months ago (2017-06-26 20:47:02 UTC) #92
shivanisha
Converted the test class to a test fixture class. https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/340001/net/http/http_cache_writers_unittest.cc#newcode20 net/http/http_cache_writers_unittest.cc:20: ...
3 years, 5 months ago (2017-06-27 04:42:20 UTC) #99
shivanisha
3 years, 5 months ago (2017-06-27 14:51:34 UTC) #102
Randy Smith (Not in Mondays)
Next round of comments. https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc#newcode115 net/http/http_cache_writers_unittest.cc:115: rv = writers_->Read(bufs[i].get(), 256, callbacks[i].callback(), ...
3 years, 5 months ago (2017-06-27 18:49:38 UTC) #103
Randy Smith (Not in Mondays)
https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers_unittest.cc#newcode36 net/http/http_cache_writers_unittest.cc:36: writers_ = base::MakeUnique<HttpCache::Writers>(disk_entry); Sorry, missed this on the last ...
3 years, 5 months ago (2017-06-27 18:50:56 UTC) #104
shivanisha
https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers_unittest.cc#newcode36 net/http/http_cache_writers_unittest.cc:36: writers_ = base::MakeUnique<HttpCache::Writers>(disk_entry); On 2017/06/27 at 18:50:56, Randy Smith ...
3 years, 5 months ago (2017-06-27 19:30:41 UTC) #105
Randy Smith (Not in Mondays)
https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers_unittest.cc#newcode36 net/http/http_cache_writers_unittest.cc:36: writers_ = base::MakeUnique<HttpCache::Writers>(disk_entry); On 2017/06/27 19:30:41, shivanisha wrote: > ...
3 years, 5 months ago (2017-06-28 15:48:28 UTC) #106
jkarlin
https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache.h File net/http/http_cache.h (right): https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache.h#newcode71 net/http/http_cache.h:71: // class WritersTest; What's this? https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache.h#newcode253 net/http/http_cache.h:253: friend class ...
3 years, 5 months ago (2017-06-28 15:50:11 UTC) #107
shivanisha
https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/400001/net/http/http_cache_writers_unittest.cc#newcode115 net/http/http_cache_writers_unittest.cc:115: rv = writers_->Read(bufs[i].get(), 256, callbacks[i].callback(), On 2017/06/27 at 18:49:37, ...
3 years, 5 months ago (2017-06-28 18:02:33 UTC) #110
shivanisha
Latest patch addresses Josh's feedback so all feedback till now is addressed. Also, it shows ...
3 years, 5 months ago (2017-06-28 19:38:21 UTC) #113
Randy Smith (Not in Mondays)
Next round. This is converging, but I don't know if I'll end up with time ...
3 years, 5 months ago (2017-06-29 00:53:50 UTC) #114
jkarlin
https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers.cc#newcode98 net/http/http_cache_writers.cc:98: DCHECK(it != all_writers_.end()); On 2017/06/28 19:38:19, shivanisha wrote: > ...
3 years, 5 months ago (2017-06-30 18:20:39 UTC) #115
shivanisha
Patch 23 addresses Randy's latest feedback. https://codereview.chromium.org/2886483002/diff/600001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/600001/net/http/http_cache_writers_unittest.cc#newcode48 net/http/http_cache_writers_unittest.cc:48: net::RequestPriority priority = ...
3 years, 5 months ago (2017-07-05 17:44:25 UTC) #118
shivanisha
Patch 24 addresses Josh's latest feedback. Thanks! https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers.cc#newcode365 net/http/http_cache_writers.cc:365: void HttpCache::Writers::OnCacheWriteFailure() ...
3 years, 5 months ago (2017-07-05 18:55:42 UTC) #126
jkarlin
https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers.cc#newcode365 net/http/http_cache_writers.cc:365: void HttpCache::Writers::OnCacheWriteFailure() { On 2017/07/05 18:55:42, shivanisha wrote: > ...
3 years, 5 months ago (2017-07-06 18:50:04 UTC) #127
jkarlin
https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/560001/net/http/http_cache_writers.cc#newcode416 net/http/http_cache_writers.cc:416: transaction->SetSharedWritingFailState(result); On 2017/07/06 18:50:04, jkarlin_slow wrote: > On 2017/07/05 ...
3 years, 5 months ago (2017-07-06 21:17:49 UTC) #128
shivanisha
PTAL, Thanks! https://codereview.chromium.org/2886483002/diff/660001/net/http/http_cache_writers.h File net/http/http_cache_writers.h (right): https://codereview.chromium.org/2886483002/diff/660001/net/http/http_cache_writers.h#newcode22 net/http/http_cache_writers.h:22: On 2017/07/06 at 18:50:04, jkarlin_slow wrote: > ...
3 years, 5 months ago (2017-07-11 02:10:27 UTC) #137
Randy Smith (Not in Mondays)
LGTM conditional on notes below on checking data read and one resolution or another to ...
3 years, 5 months ago (2017-07-11 18:51:33 UTC) #138
jkarlin
https://codereview.chromium.org/2886483002/diff/720001/net/http/http_cache_transaction.cc File net/http/http_cache_transaction.cc (right): https://codereview.chromium.org/2886483002/diff/720001/net/http/http_cache_transaction.cc#newcode1163 net/http/http_cache_transaction.cc:1163: // first transaction of that new entry and thus ...
3 years, 5 months ago (2017-07-12 15:05:26 UTC) #139
shivanisha
Thanks Randy! Addressed all feedback and rebased with main (in separate patches). https://codereview.chromium.org/2886483002/diff/720001/net/http/http_cache_transaction.cc File net/http/http_cache_transaction.cc ...
3 years, 5 months ago (2017-07-13 18:35:51 UTC) #146
jkarlin
lgtm with comments https://codereview.chromium.org/2886483002/diff/720001/net/http/http_cache_writers.cc File net/http/http_cache_writers.cc (right): https://codereview.chromium.org/2886483002/diff/720001/net/http/http_cache_writers.cc#newcode245 net/http/http_cache_writers.cc:245: } On 2017/07/13 18:35:50, shivanisha wrote: ...
3 years, 5 months ago (2017-07-14 18:30:48 UTC) #149
shivanisha
Thanks Josh! https://codereview.chromium.org/2886483002/diff/720001/net/http/http_cache_writers_unittest.cc File net/http/http_cache_writers_unittest.cc (right): https://codereview.chromium.org/2886483002/diff/720001/net/http/http_cache_writers_unittest.cc#newcode24 net/http/http_cache_writers_unittest.cc:24: class MockHttpCacheTransaction : public HttpCache::Transaction { On ...
3 years, 5 months ago (2017-07-14 20:50:28 UTC) #150
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2886483002/800001
3 years, 5 months ago (2017-07-14 20:51:37 UTC) #153
commit-bot: I haz the power
3 years, 5 months ago (2017-07-14 22:18:46 UTC) #156
Message was sent while issue was closed.
Committed patchset #31 (id:800001) as
https://chromium.googlesource.com/chromium/src/+/c6582e1924c76f68e2b8873712cd...

Powered by Google App Engine
This is Rietveld 408576698