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

net/http/http_cache.cc

Index: net/http/http_cache.cc
diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index 796aeea501414bdacb9efa9aecd625a6702a85aa..94511a5e69b2ef8b8d48f359154f8e7e1ff2ee1b 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -839,33 +839,6 @@ int HttpCache::AddTransactionToEntry(ActiveEntry* entry, Transaction* trans) {
   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());