Index: net/http/http_cache.h |
diff --git a/net/http/http_cache.h b/net/http/http_cache.h |
index 635cb92d557d17c6c60ca459f4117deb075f71f7..75ae5fb7e2c6104a9a45c79376dd13a84421b961 100644 |
--- a/net/http/http_cache.h |
+++ b/net/http/http_cache.h |
@@ -238,8 +238,8 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory, |
class MetadataWriter; |
class QuicServerInfoFactoryAdaptor; |
- class Transaction; |
class WorkItem; |
jkarlin
2017/03/07 16:45:41
This isn't alphabetical.
jkarlin
2017/03/07 17:22:59
I actually don't know that they need to be alphabe
shivanisha
2017/03/08 21:42:12
Done.
|
+ class Transaction; |
friend class Transaction; |
friend class ViewCacheHelper; |
struct PendingOp; // Info for an entry under construction. |
@@ -248,7 +248,8 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory, |
using TransactionSet = std::unordered_set<Transaction*>; |
typedef std::list<std::unique_ptr<WorkItem>> WorkItemList; |
- struct ActiveEntry { |
+ class ActiveEntry { |
jkarlin
2017/03/07 16:45:42
If it's a class the members need to be private and
shivanisha
2017/03/08 21:42:12
Reverted it to being a struct. I initially thought
|
+ public: |
explicit ActiveEntry(disk_cache::Entry* entry); |
~ActiveEntry(); |
size_t EstimateMemoryUsage() const; |
@@ -256,12 +257,32 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory, |
// Returns true if no transactions are associated with this entry. |
bool HasNoTransactions(); |
- disk_cache::Entry* disk_entry; |
- Transaction* writer; |
+ // Returns true if the given transaction is a reader. |
+ bool IsReader(Transaction* transaction); |
+ |
+ disk_cache::Entry* disk_entry = nullptr; |
+ |
+ // Transaction currently reading from the network and writing to the cache. |
+ Transaction* writer = nullptr; |
+ |
+ // Transactions that can only read from the cache. Only one of writer or |
+ // readers can exist at a time. |
TransactionSet readers; |
- TransactionList pending_queue; |
- bool will_process_pending_queue; |
- bool doomed; |
+ |
+ // Transactions waiting to be added to entry. |
Randy Smith (Not in Mondays)
2017/03/07 22:54:30
nit: Thank you for the extra comments; they help!
Randy Smith (Not in Mondays)
2017/03/08 19:11:23
minor additional nit/suggestion: Ordering the memb
shivanisha
2017/03/08 21:42:12
Done
|
+ TransactionList pending_queue; |
jkarlin
2017/03/07 16:45:41
pending_queue seems a bit vague now. Perhaps add_t
shivanisha
2017/03/08 21:42:12
Changed. add_to_entry_queue seems more consistent
|
+ |
+ // Transaction currently validating the response. This can exist |
+ // simultaneously with writer or readers. |
+ Transaction* validating_transaction = nullptr; |
+ |
+ // Transactions that have completed their validation phase and are waiting |
+ // to read the response body. |
+ TransactionList validated_queue; |
+ |
+ bool will_process_pending_queue = false; |
+ bool will_process_parallel_validation = false; |
+ bool doomed = false; |
}; |
using ActiveEntriesMap = |
@@ -310,6 +331,9 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory, |
// Returns an entry that is currently in use and not doomed, or NULL. |
ActiveEntry* FindActiveEntry(const std::string& key); |
+ // Returns true if given entry is still active. |
jkarlin
2017/03/07 16:45:41
What does it mean for an entry to be active? It's
shivanisha
2017/03/08 21:42:12
Actually, it needs to search if the entry is alive
|
+ bool IsEntryActive(const std::string& key, const ActiveEntry* entry); |
+ |
// Creates a new ActiveEntry and starts tracking it. |disk_entry| is the disk |
// cache entry. |
ActiveEntry* ActivateEntry(disk_cache::Entry* disk_entry); |
@@ -346,8 +370,28 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory, |
// the transaction will be notified about completion via its IO callback. This |
// method returns ERR_CACHE_RACE to signal the transaction that it cannot be |
jkarlin
2017/03/07 16:45:41
I don't think this actually does return ERR_CACHE_
shivanisha
2017/03/08 21:42:12
done
|
// added to the provided entry, and it should retry the process with another |
- // one (in this case, the entry is no longer valid). |
- int AddTransactionToEntry(ActiveEntry* entry, Transaction* trans); |
+ // one (in this case, the entry is no longer valid). validation_done should |
jkarlin
2017/03/07 16:45:41
|validation_done|. We wrap variable names in ||'s.
shivanisha
2017/03/08 21:42:12
N/A now after refactoring.
|
+ // be passed as true if this transaction has already passed the validation |
+ // phase, new_transaction should be passed as true if this transaction is a |
jkarlin
2017/03/07 16:45:41
same with |new_transaction|
shivanisha
2017/03/08 21:42:12
N/A now after refactoring. Removed these from the
|
+ // new transaction and not already added to queued in the entry. |
+ int AddTransactionToEntry(ActiveEntry* entry, |
+ Transaction* transaction, |
+ bool validation_done = false, |
jkarlin
2017/03/07 16:45:41
nit: s/validation_done/validation_needed/
shivanisha
2017/03/08 21:42:12
N/A now after refactoring.
|
+ bool new_transaction = false); |
jkarlin
2017/03/07 16:45:41
nit: is_new_transaction to make it clear that it's
jkarlin
2017/03/07 16:45:41
This method has few enough existing callers that I
Randy Smith (Not in Mondays)
2017/03/08 19:11:23
The style guide generally recommends against defau
shivanisha
2017/03/08 21:42:12
Thanks for the link.
Removed default arguments as
shivanisha
2017/03/08 21:42:12
N/A now after refactoring.
|
+ |
+ // Transaction invokes this when its validation matches the entry so another |
+ // transaction can now start validation. write_this_response should be passed |
+ // as true if this transaction is also responsible for writing the response |
+ // body to the cache. Returns OK if the transaction is an active writer or |
+ // reader and ERR_IO_PENDING if the transaction should wait to be able to read |
+ // from the network or cache. |
+ int OnValidationMatch(ActiveEntry* entry, |
jkarlin
2017/03/07 16:45:41
In keeping with the naming scheme, perhaps DoneWit
shivanisha
2017/03/08 21:42:12
Done. Created DoneValidationWithEntry() and merged
|
+ Transaction* transaction, |
+ bool write_this_response); |
+ |
+ // Transaction invokes this when its validation does not match the entry. |
+ // Leads to dooming the entry and restarting the pending transactions. |
+ void OnValidationNoMatch(ActiveEntry* entry, Transaction* transaction); |
// Called when the transaction has finished working with this entry. |cancel| |
// is true if the operation was cancelled by the caller instead of running |
@@ -356,14 +400,22 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory, |
// Called when the transaction has finished writing to this entry. |success| |
// is false if the cache entry should be deleted. |
- void DoneWritingToEntry(ActiveEntry* entry, bool success); |
+ void DoneWritingToEntry(ActiveEntry* entry, |
+ bool success, |
+ Transaction* transaction); |
+ |
+ // Processes other transactions when a transaction is done writing to the |
+ // entry, based on success or failure of this transaction. |
+ void DoneWritingToEntryProcessOtherTransactions(ActiveEntry* entry, |
+ bool success); |
// Called when the transaction has finished reading from this entry. |
- void DoneReadingFromEntry(ActiveEntry* entry, Transaction* trans); |
+ void DoneReadingFromEntry(ActiveEntry* entry, Transaction* transaction); |
- // Converts the active writer transaction to a reader so that other |
- // transactions can start reading from this entry. |
- void ConvertWriterToReader(ActiveEntry* entry); |
+ // Converts the active writer/validating transaction to a reader so that other |
+ // transactions can start reading from this entry. Returns true if the |
+ // transaction was added to readers. |
Randy Smith (Not in Mondays)
2017/03/07 22:54:30
It's not clear to me what this documentation means
shivanisha
2017/03/08 21:42:12
-- We want to preserve their original mode till we
|
+ bool ConvertWriterToReader(ActiveEntry* entry, Transaction* transaction); |
// Returns the LoadState of the provided pending transaction. |
LoadState GetLoadStateForPendingTransaction(const Transaction* trans); |
@@ -379,12 +431,32 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory, |
// Removes the transaction |trans|, from the pending list of |pending_op|. |
bool RemovePendingTransactionFromPendingOp(PendingOp* pending_op, |
Transaction* trans); |
- // Resumes processing the pending list of |entry|. |
+ // Resumes processing the pending list of |entry|. This should always be |
+ // called when the response is already written to the cache. |
void ProcessPendingQueue(ActiveEntry* entry); |
+ // Invoked when a transaction can start validating the entry when there may be |
+ // a writer/readers writing/reading to/from the entry. |
+ void ProcessParallelValidation(ActiveEntry* entry); |
+ |
// Events (called via PostTask) --------------------------------------------- |
- void OnProcessPendingQueue(ActiveEntry* entry); |
+ void OnProcessPendingQueue(const std::string& key, ActiveEntry* entry); |
+ |
+ void OnProcessParallelValidation(const std::string& key, ActiveEntry* entry); |
+ |
+ // Dooms the entry and restarts all the pending transactions. |
+ void DoomEntryRestartPendingTransactions(ActiveEntry* entry); |
+ |
+ // Destroys the entry and restarts all the pending transactions. |
+ void DestroyEntryRestartPendingTransactions(ActiveEntry* entry); |
+ |
+ // Retrieves all pending transactions from entry in a single list. |
+ TransactionList GetAllPendingTransactions(ActiveEntry* entry); |
+ |
+ // Helper function for restarting all pending transactions by invoking their |
+ // IO callbacks with ERR_CACHE_RACE. |
jkarlin
2017/03/07 16:45:41
Is this actually a cache race though? Seems like a
shivanisha
2017/03/08 21:42:12
This error code is used whenever we want the HCT s
|
+ void RestartPendingTransactions(const TransactionList& list); |
// Callbacks ---------------------------------------------------------------- |