Remove Writers::ShouldKeepEntry and pass as an argument to cache callback.

Bug: 472740
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I959c0e461b1388ab3d812569ca90be3b68571098
Reviewed-on: https://chromium-review.googlesource.com/738531
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515440}

net/http/http_cache.cc

@@ -923,12 +923,12 @@ void HttpCache::DoneWithEntry(ActiveEntry* entry,
 
 void HttpCache::WritersDoomEntryRestartTransactions(ActiveEntry* entry) {
   DCHECK(!entry->writers->IsEmpty());
-  DCHECK(!entry->writers->ShouldKeepEntry());
   ProcessEntryFailure(entry);
 }
 
 void HttpCache::WritersDoneWritingToEntry(ActiveEntry* entry,
                                           bool success,
+                                          bool should_keep_entry,
                                           TransactionSet make_readers) {
   // Impacts the queued transactions in one of the following ways:
   // - restart them but do not doom the entry since entry can be saved in
@@ -941,7 +941,7 @@ void HttpCache::WritersDoneWritingToEntry(ActiveEntry* entry,
   DCHECK(entry->writers->IsEmpty());
   DCHECK(success || make_readers.empty());
 
-  if (!success && entry->writers->ShouldKeepEntry()) {
+  if (!success && should_keep_entry) {
     // Restart already validated transactions so that they are able to read
     // the truncated status of the entry.
     RestartHeadersPhaseTransactions(entry);

net/http/http_cache.h

@@ -435,9 +435,11 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory {
   // cache. It may be successful completion of the response or failure as given
   // by |success|.
   // |entry| is the owner of writers.
+  // |should_keep_entry| indicates if the entry should be doomed/destroyed.
   // Virtual so that it can be extended in tests.
   virtual void WritersDoneWritingToEntry(ActiveEntry* entry,
                                          bool success,
+                                         bool should_keep_entry,
                                          TransactionSet make_readers);
 
   // Called when the transaction has received a non-matching response to

net/http/http_cache_writers.cc

@@ -169,7 +169,8 @@ void HttpCache::Writers::RemoveTransaction(Transaction* transaction,
     }
   }
 
-  cache_->WritersDoneWritingToEntry(entry_, success, TransactionSet());
+  cache_->WritersDoneWritingToEntry(entry_, success, should_keep_entry_,
+                                    TransactionSet());
 }
 
 void HttpCache::Writers::EraseTransaction(Transaction* transaction,
@@ -628,9 +629,9 @@ void HttpCache::Writers::SetIdleWritersFailState(int result) {
 void HttpCache::Writers::SetCacheCallback(bool success,
                                           const TransactionSet& make_readers) {
   DCHECK(!cache_callback_);
-  cache_callback_ =
-      base::BindOnce(&HttpCache::WritersDoneWritingToEntry,
-                     cache_->GetWeakPtr(), entry_, success, make_readers);
+  cache_callback_ = base::BindOnce(&HttpCache::WritersDoneWritingToEntry,
+                                   cache_->GetWeakPtr(), entry_, success,
+                                   should_keep_entry_, make_readers);
 }
 
 void HttpCache::Writers::OnIOComplete(int result) {

net/http/http_cache_writers.h

@@ -137,11 +137,6 @@ class NET_EXPORT_PRIVATE HttpCache::Writers {
   // Returns if response is only being read from the network.
   bool network_read_only() const { return network_read_only_; }
 
-  // Returns true if entry is worth keeping in its current state. It may be
-  // truncated or complete. Returns false if entry is incomplete and cannot be
-  // resumed, it will not be marked as truncated and will not be preserved.
-  bool ShouldKeepEntry() const { return should_keep_entry_; }
-
   int GetTransactionsCount() const { return all_writers_.size(); }
 
  private:

net/http/http_cache_writers_unittest.cc

@@ -47,6 +47,7 @@ class TestHttpCache : public HttpCache {
 
   void WritersDoneWritingToEntry(ActiveEntry* entry,
                                  bool success,
+                                 bool should_keep_entry,
                                  TransactionSet) override {}
 
   void WritersDoomEntryRestartTransactions(ActiveEntry* entry) override {}
@@ -429,9 +430,11 @@ class WritersTest : public testing::Test {
     EXPECT_EQ(priority, writers_->priority_);
   }
 
+  bool ShouldKeepEntry() { return writers_->should_keep_entry_; }
+
   void TruncateEntryNoStrongValidators() {
     writers_->InitiateTruncateEntry();
-    EXPECT_FALSE(writers_->ShouldKeepEntry());
+    EXPECT_FALSE(ShouldKeepEntry());
   }
 
   MockHttpCache cache_;
@@ -456,7 +459,7 @@ TEST_F(WritersTest, AddTransaction) {
   EXPECT_FALSE(writers_->IsEmpty());
 
   // Verify keep_entry_ is true by default.
-  EXPECT_TRUE(writers_->ShouldKeepEntry());
+  EXPECT_TRUE(ShouldKeepEntry());
 }
 
 // Tests successful addition of multiple transactions.
@@ -692,7 +695,7 @@ TEST_F(WritersTest, StopCachingWithKeepEntry) {
 
   writers_->StopCaching(true /* keep_entry */);
   EXPECT_TRUE(writers_->network_read_only());
-  EXPECT_TRUE(writers_->ShouldKeepEntry());
+  EXPECT_TRUE(ShouldKeepEntry());
 }
 
 TEST_F(WritersTest, StopCachingWithNotKeepEntry) {
@@ -701,7 +704,7 @@ TEST_F(WritersTest, StopCachingWithNotKeepEntry) {
 
   writers_->StopCaching(false /* keep_entry */);
   EXPECT_TRUE(writers_->network_read_only());
-  EXPECT_FALSE(writers_->ShouldKeepEntry());
+  EXPECT_FALSE(ShouldKeepEntry());
 }
 
 }  // namespace net