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

Unified Diff: net/http/http_cache_transaction.h

Issue 2519473002: Fixes the cache lock issue. (Closed)
Patch Set: Redesigned the fix using DataAccess class for eliminating Orphan API.(Rebased till refs/heads/master@{#442607}) Created 3 years, 11 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 side-by-side diff with in-line comments
Download patch
Index: net/http/http_cache_transaction.h
diff --git a/net/http/http_cache_transaction.h b/net/http/http_cache_transaction.h
index 32bb23571f48a0a868288b492a1f55dacf48f9cd..d3396e83e40b4a7aed35ff3e5eb9022f06c4a8ee 100644
--- a/net/http/http_cache_transaction.h
+++ b/net/http/http_cache_transaction.h
@@ -40,6 +40,7 @@ class PartialData;
struct HttpRequestInfo;
struct LoadTimingInfo;
class SSLPrivateKey;
+class DataAccess;
// This is the transaction that is returned by the HttpCache transaction
// factory.
@@ -107,7 +108,7 @@ class HttpCache::Transaction : public HttpTransaction {
// deleting the active entry.
bool AddTruncatedFlag();
- HttpCache::ActiveEntry* entry() { return entry_; }
+ HttpCache::ActiveEntry* entry() const { return entry_; }
// Returns the LoadState of the writer transaction of a given ActiveEntry. In
// other words, returns the LoadState of this transaction without asking the
@@ -117,6 +118,8 @@ class HttpCache::Transaction : public HttpTransaction {
const CompletionCallback& io_callback() { return io_callback_; }
+ base::WeakPtr<Transaction> GetWeakPtr() { return weak_factory_.GetWeakPtr(); }
+
const NetLogWithSource& net_log() const;
// Bypasses the cache lock whenever there is lock contention.
@@ -164,6 +167,35 @@ class HttpCache::Transaction : public HttpTransaction {
int ResumeNetworkStart() override;
void GetConnectionAttempts(ConnectionAttempts* out) const override;
+ // Returns true if the transaction is eligible for reading from the
+ // network and writing to the cache along with other transactions.
+ // It is eligible if the following conditions are true:
+ // - Request’s mode is READ_WRITE. If mode is only READ then this
+ // transaction should wait until the whole response is in the cache.
+ // - Method is GET.
+ // - It is not a range request.
+ // - It is not a no-store data.
+ bool IsEligibleForSharedWriting() const;
+
+ // Sets shared_ to true.
+ void SetShared();
+
+ // Sets shared_ to false. Arguments tell if the transaction will continue to
+ // read from the network with its own data_access_ object and whether the
+ // transaction will continue to read from the cache.
+ void ResetShared(bool continue_network_reading = false,
+ bool continue_cache_reading = false);
+
+ bool shared() const { return shared_; }
+
+ // Sets next_state_ such that any subsequent Read or IO callback should fail.
+ void SetSharedWritingFailState(int result);
+
+ // The transaction should now continue without shared writing and take the
+ // ownership of the data_access_ object.
+ void ContinueWithoutSharedWriting(std::unique_ptr<DataAccess> data_access,
+ bool needs_entry);
+
private:
static const size_t kNumValidationHeaders = 2;
// Helper struct to pair a header name with its value, for
@@ -215,16 +247,34 @@ class HttpCache::Transaction : public HttpTransaction {
STATE_PARTIAL_HEADERS_RECEIVED,
STATE_CACHE_READ_METADATA,
STATE_CACHE_READ_METADATA_COMPLETE,
+ // Update the value if another state becomes the last state of the start
+ // state machine.
+ STATE_START_STATE_MACHINE_FINAL = STATE_CACHE_READ_METADATA_COMPLETE,
+ // Start states should always be before Read states.
// These states are entered from Read/AddTruncatedFlag.
STATE_NETWORK_READ,
STATE_NETWORK_READ_COMPLETE,
- STATE_CACHE_READ_DATA,
- STATE_CACHE_READ_DATA_COMPLETE,
STATE_CACHE_WRITE_DATA,
STATE_CACHE_WRITE_DATA_COMPLETE,
STATE_CACHE_WRITE_TRUNCATED_RESPONSE,
- STATE_CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE
+ STATE_CACHE_WRITE_TRUNCATED_RESPONSE_COMPLETE,
+
+ // Shared Writing states.
+ STATE_SHARED_NETWORK_READ,
+ STATE_SHARED_NETWORK_READ_COMPLETE,
+ STATE_SHARED_NETWORK_READ_WAIT_COMPLETE,
+ STATE_SHARED_CACHE_WRITE_DATA,
+ STATE_SHARED_CACHE_WRITE_DATA_COMPLETE,
+
+ // Cache read states, also used for shared writing transactions when they
+ // are behind the data already written to the cache.
+ STATE_CACHE_READ_DATA,
+ STATE_CACHE_READ_DATA_COMPLETE,
+
+ // This state is reached for a shared writing transaction when there is a
+ // failure and any subsequent Read or IO callback should fail.
+ STATE_SHARED_READ_WRITE_FAILED
};
// Used for categorizing validation triggers in histograms.
@@ -291,6 +341,12 @@ class HttpCache::Transaction : public HttpTransaction {
int DoCacheWriteDataComplete(int result);
int DoCacheWriteTruncatedResponse();
int DoCacheWriteTruncatedResponseComplete(int result);
+ int DoSharedNetworkRead();
+ int DoSharedNetworkReadComplete(int result);
+ int DoSharedNetworkReadWaitComplete(int result);
+ int DoSharedCacheWriteData(int num_bytes);
+ int DoSharedCacheWriteDataComplete(int result);
+ int DoSharedReadWriteFailed();
// Sets request_ and fields derived from it.
void SetRequest(const NetLogWithSource& net_log,
@@ -374,7 +430,10 @@ class HttpCache::Transaction : public HttpTransaction {
int OnWriteResponseInfoToEntryComplete(int result);
// Called when we are done writing to the cache entry.
- void DoneWritingToEntry(bool success);
+ // perform_entry_cleanup will be false if we just want to set the internal
+ // state of the transaction but do not want to invoke any cleanup operation on
+ // entry_ e.g. when it's already done by a SharedWriter callback function.
+ void DoneWritingToEntry(bool success, bool perform_entry_cleanup = true);
// Returns an error to signal the caller that the current read failed. The
// current operation |result| is also logged. If |restart| is true, the
@@ -409,10 +468,21 @@ class HttpCache::Transaction : public HttpTransaction {
// |old_network_trans_load_timing_|, which must be NULL when this is called.
void ResetNetworkTransaction();
- // Returns true if we should bother attempting to resume this request if it is
- // aborted while in progress. If |has_data| is true, the size of the stored
- // data is considered for the result.
- bool CanResume(bool has_data);
+ // Resets |data_access_|. Also updates |old_network_trans_load_timing_|, which
+ // must be NULL when this is called.
+ void ResetDataAccess();
+
+ void SaveNetworkTransactionInfo(const HttpTransaction* transaction);
+ void SaveSharedNetworkTransactionInfo();
+
+ // Returns the currently active network transaction.
+ // At any given point, at most one of the following can be non-null:
+ // - network_trans_(network transaction for headers),
+ // - data_access_->network_transaction_(network transaction for response
+ // body for non-shared transactions)
+ // - entry_->shared_writers->data_access_->network_transaction_ (network
+ // transaction for shared writing of response body).
+ HttpTransaction* GetCurrentNetworkTransaction() const;
// Setter for response_ and auth_response_. It updates its cache entry status,
// if needed.
@@ -426,6 +496,18 @@ class HttpCache::Transaction : public HttpTransaction {
void SyncCacheEntryStatusToResponse();
void RecordHistograms();
+ // Checks if the transaction should create a SharedWriters object or not. If
+ // yes, creates one. Also checks if the transaction should continue to be part
+ // of an already created SharedWriters object or not based on its validation
+ // result.
+ void ProcessForSharedWriting();
+
+ // Returns true if the transaction is shared and still in the headers phase.
+ bool validating_shared() const;
+
+ // Destructor helper to remove the transaction from SharedWriters.
+ void RemoveTransactionFromSharedWriters();
+
// Called to signal completion of asynchronous IO.
void OnIOComplete(int result);
@@ -435,6 +517,7 @@ class HttpCache::Transaction : public HttpTransaction {
NetLogWithSource net_log_;
std::unique_ptr<HttpRequestInfo> custom_request_;
HttpRequestHeaders request_headers_copy_;
+
// If extra_headers specified a "if-modified-since" or "if-none-match",
// |external_validation_| contains the value of those headers.
ValidationHeaders external_validation_;
@@ -442,6 +525,7 @@ class HttpCache::Transaction : public HttpTransaction {
HttpCache::ActiveEntry* entry_;
HttpCache::ActiveEntry* new_entry_;
std::unique_ptr<HttpTransaction> network_trans_;
+ std::unique_ptr<DataAccess> data_access_;
CompletionCallback callback_; // Consumer's callback.
HttpResponseInfo response_;
HttpResponseInfo auth_response_;
@@ -460,6 +544,18 @@ class HttpCache::Transaction : public HttpTransaction {
bool couldnt_conditionalize_request_;
bool bypass_lock_for_test_; // A test is exercising the cache lock.
bool fail_conditionalization_for_test_; // Fail ConditionalizeRequest.
+
+ // Reads from the network and writes to the cache using a SharedWriters
+ // object.
+ bool shared_;
+
+ // Used for histograms.
+ bool initiate_shared_writing_;
+
+ // Result to be returned to the consumer on Read invocation when state is
+ // STATE_SHARED_READ_WRITE_FAILED.
+ int shared_read_write_failure_result_;
+
scoped_refptr<IOBuffer> read_buf_;
int io_buf_len_;
int read_offset_;
@@ -503,6 +599,9 @@ class HttpCache::Transaction : public HttpTransaction {
BeforeNetworkStartCallback before_network_start_callback_;
BeforeHeadersSentCallback before_headers_sent_callback_;
+ bool have_full_request_headers_;
+ HttpRequestHeaders full_request_headers_;
+
base::WeakPtrFactory<Transaction> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(Transaction);

Powered by Google App Engine
This is Rietveld 408576698