Fixes the cache lock issue.

net/http/http_cache_transaction.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file declares HttpCache::Transaction, a private class of HttpCache so
 // it should only be included by http_cache.cc
 
 #ifndef NET_HTTP_HTTP_CACHE_TRANSACTION_H_
 #define NET_HTTP_HTTP_CACHE_TRANSACTION_H_
 
@@ skipping 89 matching lines @@
   int WriteMetadata(IOBuffer* buf,
                     int buf_len,
                     const CompletionCallback& callback);
 
   // This transaction is being deleted and we are not done writing to the cache.
   // We need to indicate that the response data was truncated.  Returns true on
   // success. Keep in mind that this operation may have side effects, such as
   // 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
   // http cache, because this transaction should be the one currently writing
   // to the cache entry.
   LoadState GetWriterLoadState() const;
 
   const CompletionCallback& io_callback() { return io_callback_; }
 
+  base::WeakPtr<Transaction> GetWeakPtr() { return weak_factory_.GetWeakPtr(); }

[jkarlin, 2017/02/03 18:26:20]

Chrome convention is to call it AsWeakPtr()

[shivanisha, 2017/02/06 21:14:11]

In net/ I see many instances of GetWeakPtr but none for AsWeakPtr, although in
the rest of the chrome code base there are many instances of AsWeakPtr. Keeping
it as GetWeakPtr for consistency within net.

+
   const NetLogWithSource& net_log() const;
 
   // Bypasses the cache lock whenever there is lock contention.
   void BypassLockForTest() {
     bypass_lock_for_test_ = true;
   }
 
   // Generates a failure when attempting to conditionalize a network request.
   void FailConditionalizationForTest() {
     fail_conditionalization_for_test_ = true;
@@ skipping 26 matching lines @@
   void PopulateNetErrorDetails(NetErrorDetails* details) const override;
   void SetPriority(RequestPriority priority) override;
   void SetWebSocketHandshakeStreamCreateHelper(
       WebSocketHandshakeStreamBase::CreateHelper* create_helper) override;
   void SetBeforeNetworkStartCallback(
       const BeforeNetworkStartCallback& callback) override;
   void SetBeforeHeadersSentCallback(
       const BeforeHeadersSentCallback& callback) override;
   int ResumeNetworkStart() override;
   void GetConnectionAttempts(ConnectionAttempts* out) const override;
+  RequestPriority priority() const { return priority_; }
+
+  // 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;
+

[jkarlin, 2017/02/03 18:26:20]

Note that methods in .cc file should be defined in the same order that they're
declared in the header. I see that they don't match here (e.g., I see
ResetShared is defined earlier in the cc than SetShared).

[shivanisha, 2017/02/06 21:14:11]

Re-ordered

+  // Sets shared_ to true.
+  void SetShared();
+
+  // Sets shared_ to false. Arguments tell if the transaction will continue to
+  // read from the network 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);

[jkarlin, 2017/02/03 18:26:20]

It sounds like (from the comment) that this is a setter for next_state_, but it
does more than that.

[shivanisha, 2017/02/06 21:14:11]

Updated the comment.

+
+  // The transaction should now continue without shared writing and take the
+  // ownership of the network transaction object.
+  void ContinueWithoutSharedWriting(
+      std::unique_ptr<HttpTransaction> network_transaction,
+      bool needs_entry);
 
  private:
   static const size_t kNumValidationHeaders = 2;
   // Helper struct to pair a header name with its value, for
   // headers used to validate cache entries.
   struct ValidationHeaders {
     ValidationHeaders() : initialized(false) {}
 
     std::string values[kNumValidationHeaders];
     bool initialized;
@@ skipping 32 matching lines @@
     STATE_OVERWRITE_CACHED_RESPONSE,
     STATE_CACHE_WRITE_RESPONSE,
     STATE_CACHE_WRITE_RESPONSE_COMPLETE,
     STATE_TRUNCATE_CACHED_DATA,
     STATE_TRUNCATE_CACHED_DATA_COMPLETE,
     STATE_TRUNCATE_CACHED_METADATA,
     STATE_TRUNCATE_CACHED_METADATA_COMPLETE,
     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

[jkarlin, 2017/02/03 18:26:20]

Better to put this comment on top of the enum. People are more likely to look
there.

[shivanisha, 2017/02/06 21:14:11]

Don't need this anymore since RemoveTransactionFromSharedWriters is now using a
switch statement.

+    // 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.
   // NOTE: This enumeration is used in histograms, so please do not add entries
   // in the middle.
   enum ValidationCause {
     VALIDATION_CAUSE_UNDEFINED,
     VALIDATION_CAUSE_VARY_MISMATCH,
     VALIDATION_CAUSE_VALIDATE_FLAG,
     VALIDATION_CAUSE_STALE,
@@ skipping 46 matching lines @@
   int DoCacheReadMetadata();
   int DoCacheReadMetadataComplete(int result);
   int DoNetworkRead();
   int DoNetworkReadComplete(int result);
   int DoCacheReadData();
   int DoCacheReadDataComplete(int result);
   int DoCacheWriteData(int num_bytes);
   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,
                   const HttpRequestInfo* request);
 
   // Returns true if the request should be handled exclusively by the network
   // layer (skipping the cache entirely).
   bool ShouldPassThrough();
 
   // Called to begin reading from the cache.  Returns network error code.
@@ skipping 63 matching lines @@
 
   // Helper function, should be called with result of WriteResponseInfoToEntry
   // (or the result of the callback, when WriteResponseInfoToEntry returns
   // ERR_IO_PENDING). Calls DoneWritingToEntry if |result| is not the right
   // number of bytes. It is expected that the state that calls this will
   // return whatever net error code this function returns, which currently
   // is always "OK".
   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
   // transaction should be restarted.
   int OnCacheReadError(int result, bool restart);
 
   // Called when the cache lock timeout fires.
   void OnAddToEntryTimeout(base::TimeTicks start_time);
 
   // Deletes the current partial cache entry (sparse), and optionally removes
@@ skipping 14 matching lines @@
   int DoRestartPartialRequest();
 
   // Resets the relavant internal state to remove traces of internal processing
   // related to range requests. Deletes |partial_| if |delete_object| is true.
   void ResetPartialState(bool delete_object);
 
   // Resets |network_trans_|, which must be non-NULL.  Also updates
   // |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);
+  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),
+  // - entry_->shared_writers->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.
   void SetResponse(const HttpResponseInfo& new_response);
   void SetAuthResponse(const HttpResponseInfo& new_response);
 
   void UpdateCacheEntryStatus(
       HttpResponseInfo::CacheEntryStatus new_cache_entry_status);
 
   // Sets the response.cache_entry_status to the current cache_entry_status_.
   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();
+
+  // Helper function.
+  void OnCacheWriteDataComplete(bool was_shared, int* result);
+
   // Called to signal completion of asynchronous IO.
   void OnIOComplete(int result);
 
   State next_state_;
   const HttpRequestInfo* request_;
   RequestPriority priority_;
   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_;
   base::WeakPtr<HttpCache> cache_;
   HttpCache::ActiveEntry* entry_;
   HttpCache::ActiveEntry* new_entry_;
   std::unique_ptr<HttpTransaction> network_trans_;
   CompletionCallback callback_;  // Consumer's callback.
   HttpResponseInfo response_;
   HttpResponseInfo auth_response_;
   const HttpResponseInfo* new_response_;
   std::string cache_key_;
   Mode mode_;
   bool reading_;  // We are already reading. Never reverts to false once set.
   bool invalid_range_;  // We may bypass the cache for this request.
   bool truncated_;  // We don't have all the response data.
   bool is_sparse_;  // The data is stored in sparse byte ranges.
   bool range_requested_;  // The user requested a byte range.
   bool handling_206_;  // We must deal with this 206 response.
   bool cache_pending_;  // We are waiting for the HttpCache.
   bool done_reading_;  // All available data was read.
   bool vary_mismatch_;  // The request doesn't match the stored vary data.
   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_;
   int effective_load_flags_;
   int write_len_;
   std::unique_ptr<PartialData> partial_;  // We are dealing with range requests.
   CompletionCallback io_callback_;
 
   // Members used to track data for histograms.
   // This cache_entry_status_ takes precedence over
@@ skipping 23 matching lines @@
   // The helper object to use to create WebSocketHandshakeStreamBase
   // objects. Only relevant when establishing a WebSocket connection.
   // This is passed to the underlying network transaction. It is stored here in
   // case the transaction does not exist yet.
   WebSocketHandshakeStreamBase::CreateHelper*
       websocket_handshake_stream_base_create_helper_;
 
   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);
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_CACHE_TRANSACTION_H_