gpu shader cache: Clarify lifetime/ownership/threadiness of objects.

content/browser/gpu/shader_disk_cache.cc

Index: content/browser/gpu/shader_disk_cache.cc
diff --git a/content/browser/gpu/shader_disk_cache.cc b/content/browser/gpu/shader_disk_cache.cc
index f86c4d1f3c87cc677573522a647b77fe9eb317fe..3e4535efcba2669f84f2a39428d263d05eb4e4f2 100644
--- a/content/browser/gpu/shader_disk_cache.cc
+++ b/content/browser/gpu/shader_disk_cache.cc
@@ -5,8 +5,8 @@
 #include "content/browser/gpu/shader_disk_cache.h"
 
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/threading/thread_checker.h"
-#include "content/public/browser/browser_thread.h"
 #include "gpu/command_buffer/common/constants.h"
 #include "net/base/cache_type.h"
 #include "net/base/io_buffer.h"
@@ -19,67 +19,56 @@ namespace {
 static const base::FilePath::CharType kGpuCachePath[] =
     FILE_PATH_LITERAL("GPUCache");
 
-void EntryCloser(disk_cache::Entry* entry) {
-  entry->Close();
-}
-
-void FreeDiskCacheIterator(
-    std::unique_ptr<disk_cache::Backend::Iterator> iterator) {}
+static ShaderCacheFactory* factory_instance = nullptr;
 
 }  // namespace
 
 // ShaderDiskCacheEntry handles the work of caching/updating the cached
 // shaders.
-class ShaderDiskCacheEntry
-    : public base::ThreadChecker,
-      public base::RefCounted<ShaderDiskCacheEntry> {
+class ShaderDiskCacheEntry : public base::ThreadChecker {
  public:
-  ShaderDiskCacheEntry(base::WeakPtr<ShaderDiskCache> cache,
+  ShaderDiskCacheEntry(ShaderDiskCache* cache,
                        const std::string& key,
                        const std::string& shader);
+  ~ShaderDiskCacheEntry();
+
   void Cache();
 
  private:
-  friend class base::RefCounted<ShaderDiskCacheEntry>;
-
   enum OpType {
-    TERMINATE,
     OPEN_ENTRY,
     WRITE_DATA,
     CREATE_ENTRY,
   };
 
-  ~ShaderDiskCacheEntry();
-
   void OnOpComplete(int rv);
 
   int OpenCallback(int rv);
   int WriteCallback(int rv);
   int IOComplete(int rv);
 
-  base::WeakPtr<ShaderDiskCache> cache_;
+  ShaderDiskCache* cache_;
   OpType op_type_;
   std::string key_;
   std::string shader_;
   disk_cache::Entry* entry_;
+  base::WeakPtrFactory<ShaderDiskCacheEntry> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ShaderDiskCacheEntry);
 };
 
 // ShaderDiskReadHelper is used to load all of the cached shaders from the
 // disk cache and send to the memory cache.
-class ShaderDiskReadHelper
-    : public base::ThreadChecker,
-      public base::RefCounted<ShaderDiskReadHelper> {
+class ShaderDiskReadHelper : public base::ThreadChecker {
  public:
   using ShaderLoadedCallback = ShaderDiskCache::ShaderLoadedCallback;
-  ShaderDiskReadHelper(base::WeakPtr<ShaderDiskCache> cache,
+  ShaderDiskReadHelper(ShaderDiskCache* cache,
                        const ShaderLoadedCallback& callback);
+  ~ShaderDiskReadHelper();
+
   void LoadCache();
 
  private:
-  friend class base::RefCounted<ShaderDiskReadHelper>;
-
   enum OpType {
     TERMINATE,
     OPEN_NEXT,
@@ -89,8 +78,6 @@ class ShaderDiskReadHelper
   };
 
 
-  ~ShaderDiskReadHelper();
-
   void OnOpComplete(int rv);
 
   int OpenNextEntry();
@@ -98,19 +85,20 @@ class ShaderDiskReadHelper
   int ReadComplete(int rv);
   int IterationComplete(int rv);
 
-  base::WeakPtr<ShaderDiskCache> cache_;
+  ShaderDiskCache* cache_;
   ShaderLoadedCallback shader_loaded_callback_;
   OpType op_type_;
   std::unique_ptr<disk_cache::Backend::Iterator> iter_;
   scoped_refptr<net::IOBufferWithSize> buf_;
   disk_cache::Entry* entry_;
+  base::WeakPtrFactory<ShaderDiskReadHelper> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ShaderDiskReadHelper);
 };
 
-class ShaderClearHelper
-    : public base::RefCounted<ShaderClearHelper>,
-      public base::SupportsWeakPtr<ShaderClearHelper> {
+class ShaderClearHelper : public base::RefCounted<ShaderClearHelper>,
+                          public base::SupportsWeakPtr<ShaderClearHelper>,
+                          public base::ThreadChecker {
  public:
   ShaderClearHelper(scoped_refptr<ShaderDiskCache> cache,
                     const base::FilePath& path,
@@ -142,40 +130,36 @@ class ShaderClearHelper
   DISALLOW_COPY_AND_ASSIGN(ShaderClearHelper);
 };
 
-ShaderDiskCacheEntry::ShaderDiskCacheEntry(base::WeakPtr<ShaderDiskCache> cache,
+////////////////////////////////////////////////////////////////////////////////
+// ShaderDiskCacheEntry
+
+ShaderDiskCacheEntry::ShaderDiskCacheEntry(ShaderDiskCache* cache,
                                            const std::string& key,
                                            const std::string& shader)
-  : cache_(cache),
-    op_type_(OPEN_ENTRY),
-    key_(key),
-    shader_(shader),
-    entry_(NULL) {
-}
+    : cache_(cache),
+      op_type_(OPEN_ENTRY),
+      key_(key),
+      shader_(shader),
+      entry_(nullptr),
+      weak_ptr_factory_(this) {}
 
 ShaderDiskCacheEntry::~ShaderDiskCacheEntry() {
+  DCHECK(CalledOnValidThread());
   if (entry_)
-    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-                            base::Bind(&EntryCloser, entry_));
+    entry_->Close();
 }
 
 void ShaderDiskCacheEntry::Cache() {
   DCHECK(CalledOnValidThread());
-  if (!cache_.get())
-    return;
-
   int rv = cache_->backend()->OpenEntry(
-      key_,
-      &entry_,
-      base::Bind(&ShaderDiskCacheEntry::OnOpComplete, this));
+      key_, &entry_, base::Bind(&ShaderDiskCacheEntry::OnOpComplete,
+                                weak_ptr_factory_.GetWeakPtr()));
   if (rv != net::ERR_IO_PENDING)
     OnOpComplete(rv);
 }
 
 void ShaderDiskCacheEntry::OnOpComplete(int rv) {
   DCHECK(CalledOnValidThread());
-  if (!cache_.get())
-    return;
-
   do {
     switch (op_type_) {
       case OPEN_ENTRY:
@@ -187,82 +171,72 @@ void ShaderDiskCacheEntry::OnOpComplete(int rv) {
       case WRITE_DATA:
         rv = IOComplete(rv);
         break;
-      case TERMINATE:
-        rv = net::ERR_IO_PENDING;  // break the loop.
-        break;
-      default:
-        NOTREACHED();  // Invalid op_type_ provided.
-        break;
     }
   } while (rv != net::ERR_IO_PENDING);
 }
 
 int ShaderDiskCacheEntry::OpenCallback(int rv) {
   DCHECK(CalledOnValidThread());
-  // Called through OnOpComplete, so we know |cache_| is valid.
   if (rv == net::OK) {
     cache_->backend()->OnExternalCacheHit(key_);
     cache_->EntryComplete(this);
-    op_type_ = TERMINATE;
-    return rv;
+    return net::ERR_IO_PENDING;

[piman, 2016/11/02 18:11:15]

I'm not a huge fan of this transformation, because it makes the code less
readable IMO. net::ERR_IO_PENDING is an implementation detail of the loop in
OnOpComplete, but looking here, it's not obvious what's pending (actually,
nothing is).

In particular if we want to handle error cases differently than success, we
would need this rv to correctly flow to OnOpComplete, I think.

[sadrul, 2016/11/02 19:24:53]

I have reverted this part of the code. Instead of this, OnOpComplete() holds one
a WeakPtr<> and inspects that to determine if it has been destroyed.

   }
 
   op_type_ = CREATE_ENTRY;
   return cache_->backend()->CreateEntry(
-      key_,
-      &entry_,
-      base::Bind(&ShaderDiskCacheEntry::OnOpComplete, this));
+      key_, &entry_, base::Bind(&ShaderDiskCacheEntry::OnOpComplete,
+                                weak_ptr_factory_.GetWeakPtr()));
 }
 
 int ShaderDiskCacheEntry::WriteCallback(int rv) {
   DCHECK(CalledOnValidThread());
-  // Called through OnOpComplete, so we know |cache_| is valid.
   if (rv != net::OK) {
     LOG(ERROR) << "Failed to create shader cache entry: " << rv;
     cache_->EntryComplete(this);
-    op_type_ = TERMINATE;
-    return rv;
+    return net::ERR_IO_PENDING;
   }
 
   op_type_ = WRITE_DATA;
   scoped_refptr<net::StringIOBuffer> io_buf = new net::StringIOBuffer(shader_);
-  return entry_->WriteData(
-      1,
-      0,
-      io_buf.get(),
-      shader_.length(),
-      base::Bind(&ShaderDiskCacheEntry::OnOpComplete, this),
-      false);
+  return entry_->WriteData(1, 0, io_buf.get(), shader_.length(),
+                           base::Bind(&ShaderDiskCacheEntry::OnOpComplete,
+                                      weak_ptr_factory_.GetWeakPtr()),
+                           false);
 }
 
 int ShaderDiskCacheEntry::IOComplete(int rv) {
   DCHECK(CalledOnValidThread());
-  // Called through OnOpComplete, so we know |cache_| is valid.
   cache_->EntryComplete(this);
-  op_type_ = TERMINATE;
-  return rv;
+  return net::ERR_IO_PENDING;
 }
 
-ShaderDiskReadHelper::ShaderDiskReadHelper(base::WeakPtr<ShaderDiskCache> cache,
+////////////////////////////////////////////////////////////////////////////////
+// ShaderDiskReadHelper
+
+ShaderDiskReadHelper::ShaderDiskReadHelper(ShaderDiskCache* cache,
                                            const ShaderLoadedCallback& callback)
     : cache_(cache),
       shader_loaded_callback_(callback),
       op_type_(OPEN_NEXT),
       buf_(NULL),
-      entry_(NULL) {}
+      entry_(NULL),
+      weak_ptr_factory_(this) {}
+
+ShaderDiskReadHelper::~ShaderDiskReadHelper() {
+  DCHECK(CalledOnValidThread());
+  if (entry_)
+    entry_->Close();
+  iter_ = nullptr;
+}
 
 void ShaderDiskReadHelper::LoadCache() {
   DCHECK(CalledOnValidThread());
-  if (!cache_.get())
-    return;
   OnOpComplete(net::OK);
 }
 
 void ShaderDiskReadHelper::OnOpComplete(int rv) {
   DCHECK(CalledOnValidThread());
-  if (!cache_.get())
-    return;
-
   do {
     switch (op_type_) {
       case OPEN_NEXT:
@@ -291,17 +265,16 @@ void ShaderDiskReadHelper::OnOpComplete(int rv) {
 
 int ShaderDiskReadHelper::OpenNextEntry() {
   DCHECK(CalledOnValidThread());
-  // Called through OnOpComplete, so we know |cache_| is valid.
   op_type_ = OPEN_NEXT_COMPLETE;
   if (!iter_)
     iter_ = cache_->backend()->CreateIterator();
-  return iter_->OpenNextEntry(
-      &entry_, base::Bind(&ShaderDiskReadHelper::OnOpComplete, this));
+  return iter_->OpenNextEntry(&entry_,
+                              base::Bind(&ShaderDiskReadHelper::OnOpComplete,
+                                         weak_ptr_factory_.GetWeakPtr()));
 }
 
 int ShaderDiskReadHelper::OpenNextEntryComplete(int rv) {
   DCHECK(CalledOnValidThread());
-  // Called through OnOpComplete, so we know |cache_| is valid.
   if (rv == net::ERR_FAILED) {
     iter_.reset();
     op_type_ = ITERATION_FINISHED;
@@ -313,17 +286,13 @@ int ShaderDiskReadHelper::OpenNextEntryComplete(int rv) {
 
   op_type_ = READ_COMPLETE;
   buf_ = new net::IOBufferWithSize(entry_->GetDataSize(1));
-  return entry_->ReadData(
-      1,
-      0,
-      buf_.get(),
-      buf_->size(),
-      base::Bind(&ShaderDiskReadHelper::OnOpComplete, this));
+  return entry_->ReadData(1, 0, buf_.get(), buf_->size(),
+                          base::Bind(&ShaderDiskReadHelper::OnOpComplete,
+                                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 int ShaderDiskReadHelper::ReadComplete(int rv) {
   DCHECK(CalledOnValidThread());
-  // Called through OnOpComplete, so we know |cache_| is valid.
   if (rv && rv == buf_->size() && !shader_loaded_callback_.is_null()) {
     shader_loaded_callback_.Run(entry_->GetKey(),
                                 std::string(buf_->data(), buf_->size()));
@@ -339,23 +308,13 @@ int ShaderDiskReadHelper::ReadComplete(int rv) {
 
 int ShaderDiskReadHelper::IterationComplete(int rv) {
   DCHECK(CalledOnValidThread());
-  // Called through OnOpComplete, so we know |cache_| is valid.
   iter_.reset();
   op_type_ = TERMINATE;
   return net::OK;
 }
 
-ShaderDiskReadHelper::~ShaderDiskReadHelper() {
-  if (entry_) {
-    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-                            base::Bind(&EntryCloser, entry_));
-  }
-  if (iter_) {
-    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-                            base::Bind(&FreeDiskCacheIterator,
-                                       base::Passed(&iter_)));
-  }
-}
+////////////////////////////////////////////////////////////////////////////////
+// ShaderClearHelper
 
 ShaderClearHelper::ShaderClearHelper(scoped_refptr<ShaderDiskCache> cache,
                     const base::FilePath& path,
@@ -371,16 +330,16 @@ ShaderClearHelper::ShaderClearHelper(scoped_refptr<ShaderDiskCache> cache,
 }
 
 ShaderClearHelper::~ShaderClearHelper() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(CalledOnValidThread());
 }
 
 void ShaderClearHelper::Clear() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(CalledOnValidThread());
   DoClearShaderCache(net::OK);
 }
 
 void ShaderClearHelper::DoClearShaderCache(int rv) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(CalledOnValidThread());
 
   // Hold a ref to ourselves so when we do the CacheCleared call we don't get
   // auto-deleted when our ref count drops to zero.
@@ -412,10 +371,29 @@ void ShaderClearHelper::DoClearShaderCache(int rv) {
   }
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// ShaderCacheFactory
+
+// static
+void ShaderCacheFactory::InitInstance(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    scoped_refptr<base::SingleThreadTaskRunner> cache_task_runner) {
+  if (task_runner->BelongsToCurrentThread()) {
+    CreateFactoryInstance(std::move(cache_task_runner));
+  } else {
+    task_runner->PostTask(FROM_HERE,
+                          base::Bind(&ShaderCacheFactory::CreateFactoryInstance,
+                                     std::move(cache_task_runner)));
+  }
+}
+
 // static
 ShaderCacheFactory* ShaderCacheFactory::GetInstance() {
-  return base::Singleton<ShaderCacheFactory,
-                         base::LeakySingletonTraits<ShaderCacheFactory>>::get();
+  factory_instance =
+      base::Singleton<ShaderCacheFactory,
+                      base::LeakySingletonTraits<ShaderCacheFactory>>::get();

[piman, 2016/11/02 18:11:15]

Can we instantiate the singleton in CreateFactoryInstance, and instead here
DCHECK(factory_instance)?
This is to ensure no-one tries to call ShaderCacheFactory::GetInstance before it
gets initialized.

[sadrul, 2016/11/02 19:24:53]

Done.

[sadrul, 2016/11/02 20:36:39]

This change ended up catching a few places that calls GetInstance() directly
without initializing it first. But a number of those tests don't actually do
anything with the cache/factory. So would it make sense to have GetInstance()
return null if it hasn't been initialized, instead of being fatal? Or maybe have
a separate 'bool HasInstance()'?

The atlernative is to create the ShaderCacheFactory in
content::UnitTestTestSuite so that it's always there. The problem with this is
though that there are no task-runner when UnitTestTestSuite runs. So
ShaderCacheFactory will have to deal with null task-runners.

[piman, 2016/11/02 20:56:17]

I'm ok with returning null from GetInstance() if it hasn't been initialized.
Maybe add a comment that it's the case?

[sadrul, 2016/11/03 03:57:51]

Done.

+  DCHECK(factory_instance->CalledOnValidThread());
+  return factory_instance;
 }
 
 ShaderCacheFactory::ShaderCacheFactory() {
@@ -424,16 +402,26 @@ ShaderCacheFactory::ShaderCacheFactory() {
 ShaderCacheFactory::~ShaderCacheFactory() {
 }
 
+// static
+void ShaderCacheFactory::CreateFactoryInstance(
+    scoped_refptr<base::SingleThreadTaskRunner> cache_task_runner) {
+  DCHECK(!factory_instance);
+  GetInstance()->cache_task_runner_ = std::move(cache_task_runner);
+}
+
 void ShaderCacheFactory::SetCacheInfo(int32_t client_id,
                                       const base::FilePath& path) {
+  DCHECK(CalledOnValidThread());
   client_id_to_path_map_[client_id] = path;
 }
 
 void ShaderCacheFactory::RemoveCacheInfo(int32_t client_id) {
+  DCHECK(CalledOnValidThread());
   client_id_to_path_map_.erase(client_id);
 }
 
 scoped_refptr<ShaderDiskCache> ShaderCacheFactory::Get(int32_t client_id) {
+  DCHECK(CalledOnValidThread());
   ClientIdToPathMap::iterator iter =
       client_id_to_path_map_.find(client_id);
   if (iter == client_id_to_path_map_.end())
@@ -443,21 +431,24 @@ scoped_refptr<ShaderDiskCache> ShaderCacheFactory::Get(int32_t client_id) {
 
 scoped_refptr<ShaderDiskCache> ShaderCacheFactory::GetByPath(
     const base::FilePath& path) {
+  DCHECK(CalledOnValidThread());
   ShaderCacheMap::iterator iter = shader_cache_map_.find(path);
   if (iter != shader_cache_map_.end())
     return iter->second;
 
   ShaderDiskCache* cache = new ShaderDiskCache(path);
-  cache->Init();
+  cache->Init(cache_task_runner_);
   return cache;
 }
 
 void ShaderCacheFactory::AddToCache(const base::FilePath& key,
                                     ShaderDiskCache* cache) {
+  DCHECK(CalledOnValidThread());
   shader_cache_map_[key] = cache;
 }
 
 void ShaderCacheFactory::RemoveFromCache(const base::FilePath& key) {
+  DCHECK(CalledOnValidThread());
   shader_cache_map_.erase(key);
 }
 
@@ -465,7 +456,7 @@ void ShaderCacheFactory::ClearByPath(const base::FilePath& path,
                                      const base::Time& delete_begin,
                                      const base::Time& delete_end,
                                      const base::Closure& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(CalledOnValidThread());
   DCHECK(!callback.is_null());
 
   scoped_refptr<ShaderClearHelper> helper = new ShaderClearHelper(
@@ -489,7 +480,7 @@ void ShaderCacheFactory::ClearByPath(const base::FilePath& path,
 }
 
 void ShaderCacheFactory::CacheCleared(const base::FilePath& path) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(CalledOnValidThread());
 
   ShaderClearMap::iterator iter = shader_clear_map_.find(path);
   if (iter == shader_clear_map_.end()) {
@@ -509,6 +500,9 @@ void ShaderCacheFactory::CacheCleared(const base::FilePath& path) {
   shader_clear_map_.erase(path);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// ShaderDiskCache
+
 ShaderDiskCache::ShaderDiskCache(const base::FilePath& cache_path)
     : cache_available_(false),
       cache_path_(cache_path),
@@ -520,7 +514,8 @@ ShaderDiskCache::~ShaderDiskCache() {
   ShaderCacheFactory::GetInstance()->RemoveFromCache(cache_path_);
 }
 
-void ShaderDiskCache::Init() {
+void ShaderDiskCache::Init(
+    scoped_refptr<base::SingleThreadTaskRunner> cache_task_runner) {
   if (is_initialized_) {
     NOTREACHED();  // can't initialize disk cache twice.
     return;
@@ -530,8 +525,7 @@ void ShaderDiskCache::Init() {
   int rv = disk_cache::CreateCacheBackend(
       net::SHADER_CACHE, net::CACHE_BACKEND_DEFAULT,
       cache_path_.Append(kGpuCachePath),
-      gpu::kDefaultMaxProgramCacheMemoryBytes, true,
-      BrowserThread::GetTaskRunnerForThread(BrowserThread::CACHE).get(), NULL,
+      gpu::kDefaultMaxProgramCacheMemoryBytes, true, cache_task_runner, NULL,
       &backend_, base::Bind(&ShaderDiskCache::CacheCreatedCallback, this));
 
   if (rv == net::OK)
@@ -542,11 +536,9 @@ void ShaderDiskCache::Cache(const std::string& key, const std::string& shader) {
   if (!cache_available_)
     return;
 
-  scoped_refptr<ShaderDiskCacheEntry> shim =
-      new ShaderDiskCacheEntry(AsWeakPtr(), key, shader);
+  auto shim = base::MakeUnique<ShaderDiskCacheEntry>(this, key, shader);
   shim->Cache();
-
-  entry_map_[shim.get()] = shim;
+  entries_.push_back(std::move(shim));
 }
 
 int ShaderDiskCache::Clear(
@@ -581,19 +573,24 @@ void ShaderDiskCache::CacheCreatedCallback(int rv) {
     LOG(ERROR) << "Shader Cache Creation failed: " << rv;
     return;
   }
-  helper_ = new ShaderDiskReadHelper(AsWeakPtr(), shader_loaded_callback_);
+  helper_ =
+      base::MakeUnique<ShaderDiskReadHelper>(this, shader_loaded_callback_);
   helper_->LoadCache();
 }
 
-void ShaderDiskCache::EntryComplete(void* entry) {
-  entry_map_.erase(entry);
+void ShaderDiskCache::EntryComplete(ShaderDiskCacheEntry* entry) {
+  entries_.erase(
+      std::remove_if(entries_.begin(), entries_.end(),
+                     [entry](const std::unique_ptr<ShaderDiskCacheEntry>& e) {
+                       return e.get() == entry;
+                     }));

[piman, 2016/11/02 18:11:15]

This changes from O(log n) to O(n) - is it really ok? I think we can have a lot
of entries.

Could we use a std::set (std::unordered_set?) instead of std::vector to keep the
O(log n) (or O(1)) behavior?

[sadrul, 2016/11/02 19:24:53]

I couldn't find a clean way of using std::[unordered_]set. So I went back to
using std::unordered_map<>.

 
-  if (entry_map_.empty() && !cache_complete_callback_.is_null())
+  if (entries_.empty() && !cache_complete_callback_.is_null())
     cache_complete_callback_.Run(net::OK);
 }
 
 void ShaderDiskCache::ReadComplete() {
-  helper_ = NULL;
+  helper_ = nullptr;
 
   // The cache is considered available after we have finished reading any
   // of the old cache values off disk. This prevents a potential race where we
@@ -607,7 +604,7 @@ void ShaderDiskCache::ReadComplete() {
 
 int ShaderDiskCache::SetCacheCompleteCallback(
     const net::CompletionCallback& callback) {
-  if (entry_map_.empty()) {
+  if (entries_.empty()) {
     return net::OK;
   }
   cache_complete_callback_ = callback;