Track entries pending Doom in SimpleCache backend.

net/disk_cache/simple/simple_backend_impl.cc

 // Copyright (c) 2013 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/disk_cache/simple/simple_backend_impl.h"
 
 #include <algorithm>
 #include <cstdlib>
 
 #if defined(OS_POSIX)
@@ skipping 16 matching lines @@
 #include "net/base/net_errors.h"
 #include "net/disk_cache/backend_impl.h"
 #include "net/disk_cache/simple/simple_entry_format.h"
 #include "net/disk_cache/simple/simple_entry_impl.h"
 #include "net/disk_cache/simple/simple_histogram_macros.h"
 #include "net/disk_cache/simple/simple_index.h"
 #include "net/disk_cache/simple/simple_index_file.h"
 #include "net/disk_cache/simple/simple_synchronous_entry.h"
 #include "net/disk_cache/simple/simple_util.h"
 
+using base::Callback;
 using base::Closure;
 using base::FilePath;
 using base::MessageLoopProxy;
 using base::SequencedWorkerPool;
 using base::SingleThreadTaskRunner;
 using base::Time;
 using base::DirectoryExists;
 using file_util::CreateDirectory;
 
 namespace {
@@ skipping 146 matching lines @@
 }
 
 // A short bindable thunk that can call a completion callback. Intended to be
 // used to post a task to run a callback after an operation completes.
 void CallCompletionCallback(const net::CompletionCallback& callback,
                             int error_code) {
   DCHECK(!callback.is_null());
   callback.Run(error_code);
 }
 
+// A short bindable thunk that ensures a completion callback is always called
+// after running an operation asynchronously.
+void RunOperationAndCallback(
+    const Callback<int(const net::CompletionCallback&)>& operation,
+    const net::CompletionCallback& operation_callback) {
+  const int operation_result = operation.Run(operation_callback);
+  if (operation_result != net::ERR_IO_PENDING)
+    operation_callback.Run(operation_result);
+}
+
 void RecordIndexLoad(net::CacheType cache_type,
                      base::TimeTicks constructed_since,
                      int result) {
   const base::TimeDelta creation_to_index = base::TimeTicks::Now() -
                                             constructed_since;
   if (result == net::OK) {
     SIMPLE_CACHE_UMA(TIMES, "CreationToIndex", cache_type, creation_to_index);
   } else {
     SIMPLE_CACHE_UMA(TIMES,
                      "CreationToIndexFail", cache_type, creation_to_index);
@@ skipping 54 matching lines @@
 }
 
 int SimpleBackendImpl::GetMaxFileSize() const {
   return index_->max_size() / kMaxFileRatio;
 }
 
 void SimpleBackendImpl::OnDeactivated(const SimpleEntryImpl* entry) {
   active_entries_.erase(entry->entry_hash());
 }
 
+void SimpleBackendImpl::OnDoomStart(uint64 entry_hash) {
+  DCHECK_EQ(0u, entries_pending_doom_.count(entry_hash));
+  entries_pending_doom_.insert(
+      std::make_pair(entry_hash, std::vector<Closure>()));
+}
+
+void SimpleBackendImpl::OnDoomComplete(uint64 entry_hash) {
+  DCHECK_EQ(1u, entries_pending_doom_.count(entry_hash));
+  base::hash_map<uint64, std::vector<Closure> >::iterator it =
+      entries_pending_doom_.find(entry_hash);
+  std::vector<Closure> to_run_closures;
+  to_run_closures.swap(it->second);
+  entries_pending_doom_.erase(it);
+
+  std::for_each(to_run_closures.begin(), to_run_closures.end(),
+                std::mem_fun_ref(&Closure::Run));
+}
+
 net::CacheType SimpleBackendImpl::GetCacheType() const {
   return net::DISK_CACHE;
 }
 
 int32 SimpleBackendImpl::GetEntryCount() const {
   // TODO(pasko): Use directory file count when index is not ready.
   return index_->GetEntryCount();
 }
 
 int SimpleBackendImpl::OpenEntry(const std::string& key,
                                  Entry** entry,
                                  const CompletionCallback& callback) {
-  scoped_refptr<SimpleEntryImpl> simple_entry = CreateOrFindActiveEntry(key);
+  const uint64 entry_hash = simple_util::GetEntryHashKey(key);
+
+  base::hash_map<uint64, std::vector<Closure> >::iterator it =

[Philippe, 2013/09/11 15:41:31]

I wonder if it would make sense to factor out this part from here to line 329?
The immediate downside I see is the fact that you would have to call the
factored out function by passing it a callback that would not be lazily created
as it is done right now (only when the hash was found in
|entries_pending_doom_|).

Wdyt?

[Deprecated (see juliatuttle), 2013/09/11 19:03:21]

I think it's a good idea.

[gavinp, 2013/09/11 19:20:53]

I agree, but doing it is subtle enough that I didn't want to stop right now and
do it; so I've added a TODO(). Creating a redundant closure each time is too
expensive, but the methods that are ultimately called vary, too.

+      entries_pending_doom_.find(entry_hash);
+  if (it != entries_pending_doom_.end()) {
+    Callback<int(const net::CompletionCallback&)> operation =
+        base::Bind(&SimpleBackendImpl::OpenEntry,
+                   base::Unretained(this), key, entry);
+    it->second.push_back(base::Bind(&RunOperationAndCallback,
+                                    operation, callback));
+    return net::ERR_IO_PENDING;
+  }
+  scoped_refptr<SimpleEntryImpl> simple_entry =
+      CreateOrFindActiveEntry(entry_hash, key);
   CompletionCallback backend_callback =
       base::Bind(&SimpleBackendImpl::OnEntryOpenedFromKey,
                  AsWeakPtr(),
                  key,
                  entry,
                  simple_entry,
                  callback);
   return simple_entry->OpenEntry(entry, backend_callback);
 }
 
 int SimpleBackendImpl::CreateEntry(const std::string& key,
                                    Entry** entry,
                                    const CompletionCallback& callback) {
   DCHECK_LT(0u, key.size());
-  scoped_refptr<SimpleEntryImpl> simple_entry = CreateOrFindActiveEntry(key);
+  const uint64 entry_hash = simple_util::GetEntryHashKey(key);
+
+  base::hash_map<uint64, std::vector<Closure> >::iterator it =
+      entries_pending_doom_.find(entry_hash);
+  if (it != entries_pending_doom_.end()) {
+    Callback<int(const net::CompletionCallback&)> operation =
+        base::Bind(&SimpleBackendImpl::CreateEntry,
+                   base::Unretained(this), key, entry);
+    it->second.push_back(base::Bind(&RunOperationAndCallback,
+                                    operation, callback));
+    return net::ERR_IO_PENDING;
+  }
+  scoped_refptr<SimpleEntryImpl> simple_entry =
+      CreateOrFindActiveEntry(entry_hash, key);
   return simple_entry->CreateEntry(entry, callback);
 }
 
 int SimpleBackendImpl::DoomEntry(const std::string& key,
                                  const net::CompletionCallback& callback) {
-  scoped_refptr<SimpleEntryImpl> simple_entry = CreateOrFindActiveEntry(key);
-  return simple_entry->DoomEntry(callback);
+  const uint64 entry_hash = simple_util::GetEntryHashKey(key);
+
+  base::hash_map<uint64, std::vector<Closure> >::iterator it =
+      entries_pending_doom_.find(entry_hash);
+  if (it != entries_pending_doom_.end()) {
+    Callback<int(const net::CompletionCallback&)> operation =
+        base::Bind(&SimpleBackendImpl::DoomEntry, base::Unretained(this), key);
+    it->second.push_back(base::Bind(&RunOperationAndCallback,
+                                    operation, callback));
+    return net::ERR_IO_PENDING;
+  }
+  scoped_refptr<SimpleEntryImpl> simple_entry =
+      CreateOrFindActiveEntry(entry_hash, key);
+return simple_entry->DoomEntry(callback);
 }
 
 int SimpleBackendImpl::DoomAllEntries(const CompletionCallback& callback) {
   return DoomEntriesBetween(Time(), Time(), callback);
 }
 
 void SimpleBackendImpl::IndexReadyForDoom(Time initial_time,
                                           Time end_time,
                                           const CompletionCallback& callback,
                                           int result) {
@@ skipping 100 matching lines @@
         // TODO(pasko): Move PreferedCacheSize() to cache_util.h. Also fix the
         // spelling.
         result.max_size = disk_cache::PreferedCacheSize(available);
     }
     DCHECK(result.max_size);
   }
   return result;
 }
 
 scoped_refptr<SimpleEntryImpl> SimpleBackendImpl::CreateOrFindActiveEntry(
+    const uint64 entry_hash,
     const std::string& key) {
-  const uint64 entry_hash = simple_util::GetEntryHashKey(key);
-
+  DCHECK_EQ(entry_hash, simple_util::GetEntryHashKey(key));
   std::pair<EntryMap::iterator, bool> insert_result =
       active_entries_.insert(std::make_pair(entry_hash,
                                             base::WeakPtr<SimpleEntryImpl>()));
   EntryMap::iterator& it = insert_result.first;
   if (insert_result.second)
     DCHECK(!it->second.get());
   if (!it->second.get()) {
     SimpleEntryImpl* entry = new SimpleEntryImpl(
         cache_type_, path_, entry_hash, entry_operations_mode_, this, net_log_);
     entry->SetKey(key);
     it->second = entry->AsWeakPtr();
   }
   DCHECK(it->second.get());
   // It's possible, but unlikely, that we have an entry hash collision with a
   // currently active entry.
   if (key != it->second->key()) {
     it->second->Doom();
     DCHECK_EQ(0U, active_entries_.count(entry_hash));
-    return CreateOrFindActiveEntry(key);
+    return CreateOrFindActiveEntry(entry_hash, key);
   }
   return make_scoped_refptr(it->second.get());
 }
 
 int SimpleBackendImpl::OpenEntryFromHash(uint64 hash,
                                          Entry** entry,
                                          const CompletionCallback& callback) {
   EntryMap::iterator has_active = active_entries_.find(hash);
   if (has_active != active_entries_.end())
     return OpenEntry(has_active->second->key(), entry, callback);
@@ skipping 110 matching lines @@
   }
   callback.Run(error_code);
 }
 
 void SimpleBackendImpl::FlushWorkerPoolForTesting() {
   if (g_sequenced_worker_pool)
     g_sequenced_worker_pool->FlushForTesting();
 }
 
 }  // namespace disk_cache