HttpCache::Transaction layer allowing parallel validation

net/http/http_cache_transaction.h

Index: net/http/http_cache_transaction.h
diff --git a/net/http/http_cache_transaction.h b/net/http/http_cache_transaction.h
index 51c0db70256cbd31505ca20ffba6486ab4c36aad..3603755a0b33a566fccda65df144478c4bfde563 100644
--- a/net/http/http_cache_transaction.h
+++ b/net/http/http_cache_transaction.h
@@ -164,6 +164,10 @@ class HttpCache::Transaction : public HttpTransaction {
   int ResumeNetworkStart() override;
   void GetConnectionAttempts(ConnectionAttempts* out) const override;
 
+  // Invoked when parallel validation cannot proceed due to response failure
+  // and this transaction needs to be restarted.
+  void SetValidatingCannotProceed();
+
   // Returns the estimate of dynamically allocated memory in bytes.
   size_t EstimateMemoryUsage() const;
 
@@ -220,6 +224,9 @@ class HttpCache::Transaction : public HttpTransaction {
     STATE_PARTIAL_HEADERS_RECEIVED,
     STATE_CACHE_READ_METADATA,
     STATE_CACHE_READ_METADATA_COMPLETE,
+    STATE_HEADERS_PHASE_CANNOT_PROCEED,
+    STATE_FINISH_HEADERS,
+    STATE_FINISH_HEADERS_COMPLETE,
 
     // These states are entered from Read/AddTruncatedFlag.
     STATE_NETWORK_READ,
@@ -251,6 +258,11 @@ class HttpCache::Transaction : public HttpTransaction {
   // Each of these methods corresponds to a State value.  If there is an
   // argument, the value corresponds to the return of the previous state or
   // corresponding callback.
+  //
+  // Note that since headers and response body phases might be going on in

[jkarlin, 2017/04/20 15:05:29]

s/might be going on in parallel/can occur in parallel/

[shivanisha, 2017/04/21 23:06:00]

N/A now that this comment is not needed here.

+  // parallel for two transactions, a transaction in one phase should not be
+  // accessing disk entry fields of the other index as that might lead to data

[Randy Smith (Not in Mondays), 2017/04/19 17:26:34]

I'm not sure what "of the other index" means in this context?

[shivanisha, 2017/04/21 23:06:00]

N/A.

+  // races in some backends (like block file cache).

[Randy Smith (Not in Mondays), 2017/04/19 17:26:34]

I'd rephrase this with to talk about what the cache backend API contract does or
does not guarantee, with a comment that the disk cache backend appears not to
guarantee it and a pointer to the new bug.

I'd also suggest that this comment probably belongs somewhere else; at least, I
don't see the conceptual linkage to this area of the source.  Maybe try to make
the summary as pithy as possible (with a reference to the bug) and include it at
both places IsWritingInProgress is used?

[jkarlin, 2017/04/20 15:05:29]

I think what we really need is a DCHECK in the entry that verifies that there
are no ongoing write operations when a synchronous method is called.

[shivanisha, 2017/04/21 23:06:00]

Added new functions in ActiveEntry for data size, last used time and last
modified time that asserts the transaction's state with respect to other
transactions and used those functions in everywhere in the .cc file. Removed
this comment from here and included the explanation in the .cc file along with
the crbug number as to why we are checking if a writer is in progress.

[jkarlin, 2017/04/25 16:00:51]

Sorry, I meant meant putting the checks in the disk entry (e.g., EntryImpl). But
that can be for another CL.

   int DoGetBackend();
   int DoGetBackendComplete(int result);
   int DoInitEntry();
@@ -288,6 +300,9 @@ class HttpCache::Transaction : public HttpTransaction {
   int DoPartialHeadersReceived();
   int DoCacheReadMetadata();
   int DoCacheReadMetadataComplete(int result);
+  int DoHeadersPhaseCannotProceed();
+  int DoFinishHeaders(int result);
+  int DoFinishHeadersComplete(int result);
   int DoNetworkRead();
   int DoNetworkReadComplete(int result);
   int DoCacheReadData();
@@ -430,7 +445,12 @@ class HttpCache::Transaction : public HttpTransaction {
   void SyncCacheEntryStatusToResponse();
   void RecordHistograms();
 
-  // Called to signal completion of asynchronous IO.
+  // Called to signal completion of asynchronous IO. Note that this callback is
+  // used in the conventional sense where one layer calls the callback of the
+  // layer above it e.g. this callback gets called from the network transaction
+  // layer. In addition, it is also used for HttpCache layer to let this
+  // transaction know when it is out of a queued state in ActiveEntry and can
+  // continue its processing.
   void OnIOComplete(int result);
 
   // When in a DoLoop, use this to set the next state as it verifies that the
@@ -456,6 +476,7 @@ class HttpCache::Transaction : public HttpTransaction {
   const HttpResponseInfo* new_response_;
   std::string cache_key_;
   Mode mode_;
+  Mode original_mode_;  // Used when restarting the transaction.
   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.