HttpCache - Writers and Readers should be mutually exclusive.

There is a combination of simultaneous transactions for an entry, namely
a READ transaction followed by a READ-WRITE transaction that was
violating the mutual exclusiveness of Readers and Writers. This violation
was happening even less frequently because the second transaction being
READ-WRITE can only happen for a range request. This CL fixes the
issue.
HttpCache.RangeGET_DoNotCreateWritersWhenReaderExists

TEST: net_unittests --gtest_filter=
Bug: 792818
Change-Id: I3a98fb8de2cd521210077e4baf9460fe91232eec
Reviewed-on: https://chromium-review.googlesource.com/825802Reviewed-by: Randy Smith <rdsmith@chromium.org>
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524229}

net/http/http_cache.cc

@@ -839,7 +839,8 @@ int HttpCache::DoneWithResponseHeaders(ActiveEntry* entry,
   // through done_headers_queue for performance benefit. (Also, in case of
   // writer transaction, the consumer sometimes depend on synchronous behaviour
   // e.g. while computing raw headers size. (crbug.com/711766))
-  if ((transaction->mode() & Transaction::WRITE) && !entry->writers) {
+  if ((transaction->mode() & Transaction::WRITE) && !entry->writers &&
+      entry->readers.empty()) {
     AddTransactionToWriters(entry, transaction,
                             CanTransactionJoinExistingWriters(transaction));
     ProcessQueuedTransactions(entry);
@@ -1095,7 +1096,10 @@ void HttpCache::ProcessDoneHeadersQueue(ActiveEntry* entry) {
   } else {  // no writing in progress
     if (transaction->mode() & Transaction::WRITE) {
       if (transaction->partial()) {
-        AddTransactionToWriters(entry, transaction, parallel_writing_pattern);
+        if (entry->readers.empty())
+          AddTransactionToWriters(entry, transaction, parallel_writing_pattern);
+        else
+          return;
       } else {
         // Add the transaction to readers since the response body should have
         // already been written. (If it was the first writer about to start

net/http/http_cache_unittest.cc

@@ -1841,6 +1841,50 @@ TEST(HttpCache, RangeGET_ParallelValidationDifferentRanges) {
       histogram_name, static_cast<int>(HttpCache::PARALLEL_WRITING_CREATE), 2);
 }
 
+// Tests that a request does not create Writers when readers is not empty.
+TEST(HttpCache, RangeGET_DoNotCreateWritersWhenReaderExists) {
+  MockHttpCache cache;
+
+  // Save a request in the cache so that the next request can become a
+  // reader.
+  MockTransaction transaction(kRangeGET_Transaction);
+  transaction.request_headers = EXTRA_HEADER;
+  AddMockTransaction(&transaction);
+  RunTransactionTest(cache.http_cache(), transaction);
+
+  // Let this request be a reader since it doesn't need validation as per its
+  // load flag.
+  transaction.load_flags |= LOAD_SKIP_CACHE_VALIDATION;
+  MockHttpRequest request(transaction);
+  Context context;
+  context.result = cache.CreateTransaction(&context.trans);
+  ASSERT_THAT(context.result, IsOk());
+  context.result = context.trans->Start(&request, context.callback.callback(),
+                                        NetLogWithSource());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, cache.GetCountReaders(transaction.url));
+  RemoveMockTransaction(&transaction);
+
+  // A range request should now "not" create Writers while readers is still
+  // non-empty.
+  MockTransaction range_transaction(kRangeGET_Transaction);
+  range_transaction.request_headers = "Range: bytes = 0-9\r\n" EXTRA_HEADER;
+  AddMockTransaction(&range_transaction);
+  MockHttpRequest range_request(range_transaction);
+  Context range_context;
+  range_context.result = cache.CreateTransaction(&range_context.trans);
+  ASSERT_THAT(range_context.result, IsOk());
+  range_context.result = range_context.trans->Start(
+      &range_request, range_context.callback.callback(), NetLogWithSource());
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(1, cache.GetCountReaders(transaction.url));
+  EXPECT_FALSE(cache.IsWriterPresent(transaction.url));
+  EXPECT_EQ(1, cache.GetCountDoneHeadersQueue(transaction.url));
+
+  RemoveMockTransaction(&range_transaction);
+}
+
 // Tests parallel validation on range requests can be successfully restarted
 // when there is a cache lock timeout.
 TEST(HttpCache, RangeGET_ParallelValidationCacheLockTimeout) {