[net] Better StopCaching() handling for HttpCache::Transaction.

net/http/http_cache.cc

 // 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.
 
 #include "net/http/http_cache.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 821 matching lines @@
 
   // We do this before calling EntryAvailable to force any further calls to
   // AddTransactionToEntry to add their transaction to the pending queue, which
   // ensures FIFO ordering.
   if (!entry->writer && !entry->pending_queue.empty())
     ProcessPendingQueue(entry);
 
   return OK;
 }
 
-void HttpCache::DoneWithEntry(ActiveEntry* entry, Transaction* trans,
-                              bool cancel) {
-  // If we already posted a task to move on to the next transaction and this was
-  // the writer, there is nothing to cancel.
-  if (entry->will_process_pending_queue && entry->readers.empty())

[rvargas (doing something else), 2015/08/19 23:46:37]

I worry about removing this method.

This logic should not be moved to the transaction because it is tightly tied to
the queues (which are managed by the cache, not the transactions)[*]. That means
that a transaction may have an incomplete knowledge about what is going with the
ActiveEntry (there may be a task already posted to update the writer/readers).

I know it is weird to wave writers call DoneWriting and readers DoneReading by
themselves, and then saying that they should let the cache itself decide if this
should be treated as a writer or reader (DoneWithEntry), but I feel that if we
want to be more consistent (which is good), we should remove the other two
methods instead.

[*] On the same line, the transaction mostly cares about a single member of
ActiveEntry (disk_entry), except for a few places where we have slipped extra
knowledge. It's better to try to keep responsibilities as clear as possible.

-    return;
-
-  if (entry->writer) {
-    DCHECK(trans == entry->writer);
-
-    // Assume there was a failure.
-    bool success = false;
-    if (cancel) {
-      DCHECK(entry->disk_entry);
-      // This is a successful operation in the sense that we want to keep the
-      // entry.
-      success = trans->AddTruncatedFlag();
-      // The previous operation may have deleted the entry.
-      if (!trans->entry())
-        return;
-    }
-    DoneWritingToEntry(entry, success);
-  } else {
-    DoneReadingFromEntry(entry, trans);
-  }
-}
-
 void HttpCache::DoneWritingToEntry(ActiveEntry* entry, bool success) {
   DCHECK(entry->readers.empty());
 
   entry->writer = NULL;
 
   if (success) {
     ProcessPendingQueue(entry);
   } else {
     DCHECK(!entry->will_process_pending_queue);
 
@@ skipping 302 matching lines @@
     building_backend_ = false;
     DeletePendingOp(pending_op);
   }
 
   // The cache may be gone when we return from the callback.
   if (!item->DoCallback(result, disk_cache_.get()))
     item->NotifyTransaction(result, NULL);
 }
 
 }  // namespace net