gpu shader cache: Make ShaderClearHelper non-refcounted.

content/browser/gpu/shader_disk_cache.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 "content/browser/gpu/shader_disk_cache.h"
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_checker.h"
@@ skipping 80 matching lines @@
   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>,
-                          public base::ThreadChecker {
+class ShaderClearHelper : public base::ThreadChecker {
  public:
   ShaderClearHelper(scoped_refptr<ShaderDiskCache> cache,
                     const base::FilePath& path,
                     const base::Time& delete_begin,
                     const base::Time& delete_end,
                     const base::Closure& callback);
+  ~ShaderClearHelper();
+
   void Clear();
 
  private:
-  friend class base::RefCounted<ShaderClearHelper>;
-
   enum OpType {
     TERMINATE,
     VERIFY_CACHE_SETUP,
     DELETE_CACHE
   };
 
-  ~ShaderClearHelper();
-
   void DoClearShaderCache(int rv);
 
   scoped_refptr<ShaderDiskCache> cache_;
   OpType op_type_;
   base::FilePath path_;
   base::Time delete_begin_;
   base::Time delete_end_;
   base::Closure callback_;
+  base::WeakPtrFactory<ShaderClearHelper> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ShaderClearHelper);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // ShaderDiskCacheEntry
 
 ShaderDiskCacheEntry::ShaderDiskCacheEntry(ShaderDiskCache* cache,
                                            const std::string& key,
                                            const std::string& shader)
@@ skipping 117 matching lines @@
       case READ_COMPLETE:
         rv = ReadComplete(rv);
         break;
       case ITERATION_FINISHED:
         rv = IterationComplete(rv);
         break;
       case TERMINATE:
         cache_->ReadComplete();
         rv = net::ERR_IO_PENDING;  // break the loop
         break;
-      default:
-        NOTREACHED();  // Invalid state for read helper
-        rv = net::ERR_FAILED;
-        break;
     }
   } while (rv != net::ERR_IO_PENDING);
 }
 
 int ShaderDiskReadHelper::OpenNextEntry() {
   DCHECK(CalledOnValidThread());
   op_type_ = OPEN_NEXT_COMPLETE;
   if (!iter_)
     iter_ = cache_->backend()->CreateIterator();
   return iter_->OpenNextEntry(&entry_,
@@ skipping 42 matching lines @@
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ShaderClearHelper
 
 ShaderClearHelper::ShaderClearHelper(scoped_refptr<ShaderDiskCache> cache,
                     const base::FilePath& path,
                     const base::Time& delete_begin,
                     const base::Time& delete_end,
                     const base::Closure& callback)
-    : cache_(cache),
+    : cache_(std::move(cache)),
       op_type_(VERIFY_CACHE_SETUP),
       path_(path),
       delete_begin_(delete_begin),
       delete_end_(delete_end),
-      callback_(callback) {
+      callback_(callback),
+      weak_ptr_factory_(this) {
 }
 
 ShaderClearHelper::~ShaderClearHelper() {
   DCHECK(CalledOnValidThread());
 }
 
 void ShaderClearHelper::Clear() {
   DCHECK(CalledOnValidThread());
   DoClearShaderCache(net::OK);
 }
 
 void ShaderClearHelper::DoClearShaderCache(int rv) {
   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.
-  scoped_refptr<ShaderClearHelper> helper = this;
-
   while (rv != net::ERR_IO_PENDING) {
     switch (op_type_) {
       case VERIFY_CACHE_SETUP:
         rv = cache_->SetAvailableCallback(
-            base::Bind(&ShaderClearHelper::DoClearShaderCache, AsWeakPtr()));
+            base::Bind(&ShaderClearHelper::DoClearShaderCache,
+                       weak_ptr_factory_.GetWeakPtr()));
         op_type_ = DELETE_CACHE;
         break;
       case DELETE_CACHE:
         rv = cache_->Clear(
             delete_begin_, delete_end_,
-            base::Bind(&ShaderClearHelper::DoClearShaderCache, AsWeakPtr()));
+            base::Bind(&ShaderClearHelper::DoClearShaderCache,
+                       weak_ptr_factory_.GetWeakPtr()));
         op_type_ = TERMINATE;
         break;
       case TERMINATE:
+        callback_.Run();
+        // Calling CacheCleared() destroys |this|.
         ShaderCacheFactory::GetInstance()->CacheCleared(path_);
-        callback_.Run();
         rv = net::ERR_IO_PENDING;  // Break the loop.
         break;
-      default:
-        NOTREACHED();  // Invalid state provided.
-        op_type_ = TERMINATE;
-        break;
     }
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ShaderCacheFactory
 
 // static
 void ShaderCacheFactory::InitInstance(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
@@ skipping 33 matching lines @@
   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);
+  ClientIdToPathMap::iterator iter = client_id_to_path_map_.find(client_id);
   if (iter == client_id_to_path_map_.end())
     return NULL;
   return ShaderCacheFactory::GetByPath(iter->second);
 }
 
 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())
@@ skipping 15 matching lines @@
   shader_cache_map_.erase(key);
 }
 
 void ShaderCacheFactory::ClearByPath(const base::FilePath& path,
                                      const base::Time& delete_begin,
                                      const base::Time& delete_end,
                                      const base::Closure& callback) {
   DCHECK(CalledOnValidThread());
   DCHECK(!callback.is_null());
 
-  scoped_refptr<ShaderClearHelper> helper = new ShaderClearHelper(
+  auto helper = base::MakeUnique<ShaderClearHelper>(
       GetByPath(path), path, delete_begin, delete_end, callback);
 
   // We could receive requests to clear the same path with different
   // begin/end times. So, we keep a list of requests. If we haven't seen this
   // path before we kick off the clear and add it to the list. If we have see it
   // already, then we already have a clear running. We add this clear to the
   // list and wait for any previous clears to finish.
   ShaderClearMap::iterator iter = shader_clear_map_.find(path);
   if (iter != shader_clear_map_.end()) {
-    iter->second.push(helper);
+    iter->second.push(std::move(helper));
     return;
   }
 
+  helper->Clear();

[piman, 2016/11/03 18:01:38]

Is it possible that Clear() will reenter CacheCleared() ? It seems like the code
allows it. If so, we want to keep it after adding the helper to the map.

[sadrul, 2016/11/03 19:31:13]

Oh nice! You are right. Done. Thanks!

   shader_clear_map_.insert(
       std::pair<base::FilePath, ShaderClearQueue>(path, ShaderClearQueue()));
-  shader_clear_map_[path].push(helper);
-  helper->Clear();
+  shader_clear_map_[path].push(std::move(helper));
 }
 
 void ShaderCacheFactory::CacheCleared(const base::FilePath& path) {
   DCHECK(CalledOnValidThread());
 
   ShaderClearMap::iterator iter = shader_clear_map_.find(path);
   if (iter == shader_clear_map_.end()) {
     LOG(ERROR) << "Completed clear but missing clear helper.";
     return;
   }
 
   iter->second.pop();
 
   // If there are remaining items in the list we trigger the Clear on the
   // next one.
   if (!iter->second.empty()) {
     iter->second.front()->Clear();
     return;
   }
 
-  shader_clear_map_.erase(path);
+  shader_clear_map_.erase(iter);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ShaderDiskCache
 
 ShaderDiskCache::ShaderDiskCache(const base::FilePath& cache_path)
     : cache_available_(false),
       cache_path_(cache_path),
       is_initialized_(false) {
   ShaderCacheFactory::GetInstance()->AddToCache(cache_path_, this);
@@ skipping 91 matching lines @@
     const net::CompletionCallback& callback) {
   if (entries_.empty()) {
     return net::OK;
   }
   cache_complete_callback_ = callback;
   return net::ERR_IO_PENDING;
 }
 
 }  // namespace content