Fixes the cache lock issue.

net/http/http_cache.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 a HttpTransactionFactory implementation that can be
 // layered on top of another HttpTransactionFactory to add HTTP caching.  The
 // caching logic follows RFC 7234 (any exceptions are called out in the code).
 //
 // The HttpCache takes a disk_cache::Backend as a parameter, and uses that for
 // the cache storage.
 //
 // See HttpTransactionFactory and HttpTransaction for more details.
 
 #ifndef NET_HTTP_HTTP_CACHE_H_
 #define NET_HTTP_HTTP_CACHE_H_
 
 #include <list>
 #include <map>
 #include <memory>
 #include <string>
 #include <unordered_map>
+#include <unordered_set>
+#include <utility>

[Randy Smith (Not in Mondays), 2017/01/19 00:53:42]

What is this include for?  I think of <utility> as being for algorithms, which I
wouldn't think would be needed in the header.

[shivanisha, 2017/01/19 21:13:06]

Included this as per git cl lint. I agree it's not needed, though. Do you
suggest to ignore those lint messages?

[Randy Smith (Not in Mondays), 2017/01/20 19:54:31]

Weird.  No, I don't think it's overriding an automated tool, and hey, maybe it
knows something I don't.  If you feel inspired, you could file a bug on the
tool, but that's up to you :-}.

[shivanisha, 2017/01/25 19:46:12]

Removed the include.

 
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/time/clock.h"
 #include "base/time/time.h"
 #include "net/base/cache_type.h"
 #include "net/base/completion_callback.h"
 #include "net/base/load_states.h"
@@ skipping 163 matching lines @@
   }
 
   // Causes all transactions created after this point to generate a failure
   // when attempting to conditionalize a network request.
   void FailConditionalizationForTest() {
     fail_conditionalization_for_test_ = true;
   }
 
   // HttpTransactionFactory implementation:
   int CreateTransaction(RequestPriority priority,
-                        std::unique_ptr<HttpTransaction>* trans) override;
+                        std::unique_ptr<HttpTransaction>* transaction) override;
   HttpCache* GetCache() override;
   HttpNetworkSession* GetSession() override;
 
   base::WeakPtr<HttpCache> GetWeakPtr() { return weak_factory_.GetWeakPtr(); }
 
   // Resets the network layer to allow for tests that probe
   // network changes (e.g. host unreachable).  The old network layer is
   // returned to allow for filter patterns that only intercept
   // some creation requests.  Note ownership exchange.
   std::unique_ptr<HttpTransactionFactory>
   SetHttpNetworkTransactionFactoryForTesting(
       std::unique_ptr<HttpTransactionFactory> new_network_layer);
 
  private:
   // Types --------------------------------------------------------------------
 
   // Disk cache entry data indices.
   enum {
     kResponseInfoIndex = 0,
     kResponseContentIndex,
     kMetadataIndex,
 
     // Must remain at the end of the enum.
     kNumCacheEntryDataIndices
   };
 
+  class DataAccess;
+  class SharedWriters;
+  class Transaction;
+  typedef std::list<Transaction*> TransactionList;
+  typedef std::unordered_set<Transaction*> TransactionSet;
   class MetadataWriter;
   class QuicServerInfoFactoryAdaptor;
-  class Transaction;
   class WorkItem;
-  friend class Transaction;
   friend class ViewCacheHelper;
   struct PendingOp;  // Info for an entry under construction.
 
-  typedef std::list<Transaction*> TransactionList;
   typedef std::list<std::unique_ptr<WorkItem>> WorkItemList;
 
   struct ActiveEntry {
     explicit ActiveEntry(disk_cache::Entry* entry);
     ~ActiveEntry();
 
     disk_cache::Entry* disk_entry;
+    // Either writer or shared_writers will be non-null at any point in time.
     Transaction*       writer;
-    TransactionList    readers;
+    std::unique_ptr<SharedWriters> shared_writers;
+    TransactionSet readers;
     TransactionList    pending_queue;
     bool               will_process_pending_queue;
     bool               doomed;
   };
 
   using ActiveEntriesMap =
       std::unordered_map<std::string, std::unique_ptr<ActiveEntry>>;
   using PendingOpsMap = std::unordered_map<std::string, PendingOp*>;
   using ActiveEntriesSet = std::map<ActiveEntry*, std::unique_ptr<ActiveEntry>>;
   using PlaybackCacheMap = std::unordered_map<std::string, int>;
@@ skipping 35 matching lines @@
   // Closes a previously doomed entry.
   void FinalizeDoomedEntry(ActiveEntry* entry);
 
   // Returns an entry that is currently in use and not doomed, or NULL.
   ActiveEntry* FindActiveEntry(const std::string& key);
 
   // Creates a new ActiveEntry and starts tracking it. |disk_entry| is the disk
   // cache entry.
   ActiveEntry* ActivateEntry(disk_cache::Entry* disk_entry);
 
-  // Deletes an ActiveEntry.
+  // Deletes an ActiveEntry. Expects all of the transaction pointers to be
+  // null and containers to be empty: writer, shared_writers, pending_queue
+  // and readers.
   void DeactivateEntry(ActiveEntry* entry);
 
   // Deletes an ActiveEntry using an exhaustive search.
   void SlowDeactivateEntry(ActiveEntry* entry);
 
   // Returns the PendingOp for the desired |key|. If an entry is not under
   // construction already, a new PendingOp structure is created.
   PendingOp* GetPendingOp(const std::string& key);
 
   // Deletes a PendingOp.
@@ skipping 69 matching lines @@
   // TODO(ajwong): The PendingOp lifetime management is very tricky.  It might
   // be possible to simplify it using either base::Owned() or base::Passed()
   // with the callback.
   static void OnPendingOpComplete(const base::WeakPtr<HttpCache>& cache,
                                   PendingOp* pending_op,
                                   int result);
 
   // Processes the backend creation notification.
   void OnBackendCreated(int result, PendingOp* pending_op);
 
+  // Destroys the entry and any pending queue transactions' callbacks are
+  // invoked with ERR_CACHE_RACE that restarts their state machine.
+  void DestroyEntryAndRestartPendingQueueTransactions(ActiveEntry* entry);
+
+  // Writes response info to entry.
+  int WriteResponseInfo(ActiveEntry* entry,
+                        const HttpResponseInfo* response,
+                        CompletionCallback& callback,
+                        bool truncated,
+                        int* io_buf_len);
+
+  // Checks if the response is completed based on content length.
+  bool IsResponseCompleted(const ActiveEntry* entry,
+                           const HttpResponseInfo* response);
+
+  // 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 CanResumeEntry(bool has_data,
+                      const std::string& method,
+                      const HttpResponseInfo* response,
+                      ActiveEntry* entry);
+
+  // Creates a SharedWriters object. The ownership of the network_transaction is
+  // transferred to a new DataAccess object owned by the SharedWriters object.
+  void CreateSharedWriters(Transaction* cacheTransaction,
+                           std::unique_ptr<HttpTransaction> network_transaction,
+                           RequestPriority priority);
+
+  // Shared Writing scenarios requiring processing of transactions in the entry
+  // or removal of entry.
+  //
+  // These are invoked from SharedWriters IO callback and will lead to shared
+  // writers' ownership being released from entry which will be deleted at the
+  // end of the callback processing by the SharedWriters object itself.
+  //

[Randy Smith (Not in Mondays), 2017/01/19 00:53:42]

To me, it would make it more clear that these comments apply to all the
following routines if there was a blank line between the and the first one; my
gut instinct about this formatting is that the comment applies just to the next
routine.

[shivanisha, 2017/01/25 19:46:12]

N/A due to recent changes.

+  // Response data completely written to the cache successfully.
+  void ResponseCompleteSharedWriters(ActiveEntry* entry);
+
+  // Response data could not be written to the cache successfully, or
+  // Response data could not be read from the network successfully.
+  // Success is true if the entry was marked as truncated successfully.
+  void ResponseFailedSharedWriters(bool success, ActiveEntry* entry);
+
+  // Resets or releases if entry contains an empty SharedWriters object and
+  // calls ProcessPendingQueue. It will release if SharedWriters is about to
+  // delete itself at the end of the flow, else resets.
+  void ResetSharedWritersProcessPendingQueue(ActiveEntry* entry);
+
+  // These are invoked from HttpCache::Transaction and might lead to shared
+  // writers being destroyed.
+  //
+  // DoneReading() API invoked on the transaction.
+  void DoneReadingSharedWriters(Transaction* transaction, ActiveEntry* entry);
+
+  // StopCaching() API invoked on the transaction.
+  void StopCachingSharedWriters(Transaction* transaction, ActiveEntry* entry);
+
+  // Validating transaction receives a non-304 response.
+  std::unique_ptr<HttpTransaction> ValidationNoMatchSharedWriters(
+      const std::string& key,
+      Transaction* transaction,
+      std::unique_ptr<HttpTransaction> network_transaction,
+      RequestPriority priority,
+      ActiveEntry* entry);
+
+  // Current Writer is destroyed by its consumer.
+  void RemoveCurrentSharedWriter(Transaction* transaction, ActiveEntry* entry);
+
+  // An idle writer is destroyed by its consumer.
+  void RemoveIdleSharedWriter(Transaction* transaction, ActiveEntry* entry);
+
+  // Validating transaction is destroyed by its consumer.
+  void RemoveValidatingTransSharedWriters(Transaction* transaction,
+                                          ActiveEntry* entry);
+
+  // Invokes the io callback of the transaction with the result, if the
+  // transaction is still alive.
+  void NotifyTransaction(base::WeakPtr<Transaction> transaction, int result);
+
+  // Reset SharedWriters except in the scenario given here: SharedWriters may
+  // be in a flow where it is meant to destroy itself at the end of the flow
+  // and we are here as a result of a callback. Do not reset in that case and
+  // just release the ownership of SharedWriters.
+  void ResetOrReleaseSharedWriters(ActiveEntry* entry);
+
   // Variables ----------------------------------------------------------------
 
   NetLog* net_log_;
 
   // Used when lazily constructing the disk_cache_.
   std::unique_ptr<BackendFactory> backend_factory_;
   bool building_backend_;
   bool bypass_lock_for_test_;
   bool fail_conditionalization_for_test_;
 
@@ skipping 18 matching lines @@
   std::unique_ptr<base::Clock> clock_;
 
   base::WeakPtrFactory<HttpCache> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(HttpCache);
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_CACHE_H_