Fixes the cache lock issue.

net/http/http_cache_data_access.h

+// Copyright (c) 2016 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.
+
+#ifndef NET_HTTP_HTTP_CACHE_DATA_ACCESS_H_
+#define NET_HTTP_HTTP_CACHE_DATA_ACCESS_H_
+
+#include <net/http/http_cache.h>
+#include <memory>
+
+namespace net {
+
+// This class encapsulates the functionality for reading from the
+// network and writing to the cache. It acts as a helper class to its consumers,
+// which are currently HttpCache::Transaction and HttpCache::SharedWriters.
+//
+// Having the functionality separated in this helper class allows the DataAccess
+// object to out-live its consumer transaction object. Thus the network
+// transaction can out-live its original creator transaction. This is required
+// for cases where the network transaction is being shared across multiple
+// HttpCache::Transactions. For this reason this class should not contain
+// any specific cache transaction's information.
+//
+// Its ownership might be transferred from an HttpCache::Transaction to
+// HttpCache::SharedWriters for shared writing. The ownership can also be
+// transferred back in some failure cases when we can no longer continue with
+// shared writing e.g. cache access failure.
+class HttpCache::DataAccess {
+ public:
+  DataAccess(std::unique_ptr<HttpTransaction> network_transaction,
+             ActiveEntry* entry);

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

I think it's important to document the lifetime requirements on the consumer of
objects passed by raw pointers and stored in a persistent object.  "Consumer
must ensure that |*entry| outlives this." is plenty.

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

N/A here since DataAccess class no longer exists. 
Added similar comment in SharedWriters constructor.

+  ~DataAccess();
+  int Read(scoped_refptr<IOBuffer> buf,
+           int buf_len,
+           const CompletionCallback& callback);
+  int CacheWrite(scoped_refptr<IOBuffer> buf,
+                 int write_len,
+                 const CompletionCallback& callback);
+
+  // Called to signal completion of asynchronous IO.
+  void OnIOComplete(int result);
+
+  friend class Transaction;
+  friend class SharedWriters;
+
+ private:
+  enum State {
+    STATE_NONE,
+    STATE_NETWORK_READ,
+    STATE_NETWORK_READ_COMPLETE,
+    STATE_CACHE_WRITE_DATA,
+    STATE_CACHE_WRITE_DATA_COMPLETE,
+  };
+
+  int DoLoop(int result);
+  int DoNetworkRead();
+  int DoNetworkReadComplete(int result);
+  int DoCacheWriteData(int num_bytes);
+  int DoCacheWriteDataComplete(int result);
+
+  State next_state_ = STATE_NONE;
+  CompletionCallback callback_;  // Consumer's callback.
+  std::unique_ptr<HttpTransaction> network_transaction_;
+  CompletionCallback io_callback_;

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

Suggestion: This is currently set in the constructor and not changed for the
lifetime of the class, so a) it can be const (though that might require setting
it in the constructor through construction rather than assignment), and b) it
could just be eliminated and written out at each location.  I've vote in favor
of the latter; it's more text, but it's less lookup for future readers of the
code.

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

N/A since DataAccess class no longer exists.

+  scoped_refptr<IOBuffer> read_buf_;
+  int io_buf_len_ = 0;
+  int write_len_ = 0;
+  ActiveEntry* entry_ = nullptr;
+  base::WeakPtrFactory<DataAccess> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(DataAccess);
+};
+
+}  // namespace net
+
+#endif  // NET_HTTP_HTTP_CACHE_DATA_ACCESS_H_