[CacheStorage] Use backend size to determine change in cache size in put and delete

content/browser/cache_storage/cache_storage_cache.cc

 // Copyright 2014 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/cache_storage/cache_storage_cache.h"
 
 #include <stddef.h>
 #include <string>
 #include <utility>
 
@@ skipping 162 matching lines @@
   scoped_ptr<CacheMetadata> metadata(new CacheMetadata());
 
   if (!metadata->ParseFromArray(buffer->data(), buffer->size())) {
     callback.Run(scoped_ptr<CacheMetadata>());
     return;
   }
 
   callback.Run(std::move(metadata));
 }
 
-void SizeDidGetCacheSize(const CacheStorageCache::SizeCallback& callback,
-                         int size) {
-  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
-                                                base::Bind(callback, size));
-}
-
 }  // namespace
 
 // The state needed to iterate all entries in the cache.
 struct CacheStorageCache::OpenAllEntriesContext {
   OpenAllEntriesContext() : enumerated_entry(nullptr) {}
   ~OpenAllEntriesContext() {
     for (size_t i = 0, max = entries.size(); i < max; ++i) {
       if (entries[i])
         entries[i]->Close();
     }
@@ skipping 48 matching lines @@
 
   // The context holding open entries.
   scoped_ptr<OpenAllEntriesContext> entries_context;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(KeysContext);
 };
 
 // The state needed to pass between CacheStorageCache::Put callbacks.
 struct CacheStorageCache::PutContext {
-  PutContext(
-      const GURL& origin,
-      scoped_ptr<ServiceWorkerFetchRequest> request,
-      scoped_ptr<ServiceWorkerResponse> response,
-      scoped_ptr<storage::BlobDataHandle> blob_data_handle,
-      const CacheStorageCache::ErrorCallback& callback,
-      const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
-      const scoped_refptr<storage::QuotaManagerProxy>& quota_manager_proxy)
-      : origin(origin),
-        request(std::move(request)),
+  PutContext(scoped_ptr<ServiceWorkerFetchRequest> request,
+             scoped_ptr<ServiceWorkerResponse> response,
+             scoped_ptr<storage::BlobDataHandle> blob_data_handle,
+             const CacheStorageCache::ErrorCallback& callback)
+      : request(std::move(request)),
         response(std::move(response)),
         blob_data_handle(std::move(blob_data_handle)),
-        callback(callback),
-        request_context_getter(request_context_getter),
-        quota_manager_proxy(quota_manager_proxy) {}
+        callback(callback) {}
 
   // Input parameters to the Put function.
-  GURL origin;
   scoped_ptr<ServiceWorkerFetchRequest> request;
   scoped_ptr<ServiceWorkerResponse> response;
   scoped_ptr<storage::BlobDataHandle> blob_data_handle;
   CacheStorageCache::ErrorCallback callback;
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter;
-  scoped_refptr<storage::QuotaManagerProxy> quota_manager_proxy;
   disk_cache::ScopedEntryPtr cache_entry;
   int64_t available_bytes = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PutContext);
 };
 
 // static
 scoped_refptr<CacheStorageCache> CacheStorageCache::CreateMemoryCache(
     const GURL& origin,
@@ skipping 169 matching lines @@
     const GURL& origin,
     const base::FilePath& path,
     const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
     const scoped_refptr<storage::QuotaManagerProxy>& quota_manager_proxy,
     base::WeakPtr<storage::BlobStorageContext> blob_context)
     : origin_(origin),
       path_(path),
       request_context_getter_(request_context_getter),
       quota_manager_proxy_(quota_manager_proxy),
       blob_storage_context_(blob_context),
-      backend_state_(BACKEND_UNINITIALIZED),
       scheduler_(new CacheStorageScheduler()),
-      initializing_(false),
       memory_only_(path.empty()),
       weak_ptr_factory_(this) {
 }
 
 bool CacheStorageCache::LazyInitialize() {
   switch (backend_state_) {
     case BACKEND_UNINITIALIZED:
       InitBackend();
       return true;
     case BACKEND_CLOSED:
@@ skipping 273 matching lines @@
       callback.Run(CACHE_STORAGE_ERROR_STORAGE);
       return;
     }
   }
 
   ErrorCallback pending_callback =
       base::Bind(&CacheStorageCache::PendingErrorCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback);
 
   scoped_ptr<PutContext> put_context(
-      new PutContext(origin_, std::move(request), std::move(response),
-                     std::move(blob_data_handle), pending_callback,
-                     request_context_getter_, quota_manager_proxy_));
+      new PutContext(std::move(request), std::move(response),
+                     std::move(blob_data_handle), pending_callback));
 
   scheduler_->ScheduleOperation(
       base::Bind(&CacheStorageCache::PutImpl, weak_ptr_factory_.GetWeakPtr(),
                  base::Passed(std::move(put_context))));
 }
 
 void CacheStorageCache::PutImpl(scoped_ptr<PutContext> put_context) {
   DCHECK_NE(BACKEND_UNINITIALIZED, backend_state_);
   if (backend_state_ != BACKEND_OPEN) {
     put_context->callback.Run(CACHE_STORAGE_ERROR_STORAGE);
@@ skipping 134 matching lines @@
   if (rv != expected_bytes) {
     put_context->cache_entry->Doom();
     put_context->callback.Run(CACHE_STORAGE_ERROR_STORAGE);
     return;
   }
 
   // The metadata is written, now for the response content. The data is streamed
   // from the blob into the cache entry.
 
   if (put_context->response->blob_uuid.empty()) {
-    if (put_context->quota_manager_proxy.get()) {
-      put_context->quota_manager_proxy->NotifyStorageModified(
-          storage::QuotaClient::kServiceWorkerCache, put_context->origin,
-          storage::kStorageTypeTemporary,
-          put_context->cache_entry->GetDataSize(INDEX_HEADERS));
-    }
-
-    put_context->callback.Run(CACHE_STORAGE_OK);
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(put_context->callback, CACHE_STORAGE_OK));

[michaeln, 2016/02/12 20:16:53]

at first i didn't understand why you wanted the PostTask here, i think i do now,
|this| could be deleted during |callback| execution, is that right?

if so, you might be able to reverse the order instead of going thru the msgloop

  UpdateCacheSize();
  callback.Run(OK);

the update method will have safely taken the ref it needs before the callback
has a chance to drop any the callers

[jkarlin, 2016/02/12 20:25:49]

yes
MsgLoop is safer.. but yes, this is faster. Done.

[jkarlin, 2016/02/12 20:28:29]

By that I meant generally safer. In this case I do believe it's safe.

+    UpdateCacheSize();
     return;
   }
 
   DCHECK(put_context->blob_data_handle);
 
   disk_cache::ScopedEntryPtr entry(std::move(put_context->cache_entry));
   put_context->cache_entry = NULL;
 
   CacheStorageBlobToDiskCache* blob_to_cache =
       new CacheStorageBlobToDiskCache();
   BlobToDiskCacheIDMap::KeyType blob_to_cache_key =
       active_blob_to_disk_cache_writers_.Add(blob_to_cache);
 
-  // Grab some pointers before passing put_context in Bind.
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter =
-      put_context->request_context_getter;
   scoped_ptr<storage::BlobDataHandle> blob_data_handle =
       std::move(put_context->blob_data_handle);
 
   blob_to_cache->StreamBlobToCache(
-      std::move(entry), INDEX_RESPONSE_BODY, request_context_getter,
+      std::move(entry), INDEX_RESPONSE_BODY, request_context_getter_,
       std::move(blob_data_handle),
       base::Bind(&CacheStorageCache::PutDidWriteBlobToCache,
                  weak_ptr_factory_.GetWeakPtr(),
                  base::Passed(std::move(put_context)), blob_to_cache_key));
 }
 
 void CacheStorageCache::PutDidWriteBlobToCache(
     scoped_ptr<PutContext> put_context,
     BlobToDiskCacheIDMap::KeyType blob_to_cache_key,
     disk_cache::ScopedEntryPtr entry,
     bool success) {
   DCHECK(entry);
   put_context->cache_entry = std::move(entry);
 
   active_blob_to_disk_cache_writers_.Remove(blob_to_cache_key);
 
   if (!success) {
     put_context->cache_entry->Doom();
     put_context->callback.Run(CACHE_STORAGE_ERROR_STORAGE);
     return;
   }
 
-  if (put_context->quota_manager_proxy.get()) {
-    put_context->quota_manager_proxy->NotifyStorageModified(
-        storage::QuotaClient::kServiceWorkerCache, put_context->origin,
-        storage::kStorageTypeTemporary,
-        put_context->cache_entry->GetDataSize(INDEX_HEADERS) +
-            put_context->cache_entry->GetDataSize(INDEX_RESPONSE_BODY));
-  }
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(put_context->callback, CACHE_STORAGE_OK));
+  UpdateCacheSize();
+}
 
-  put_context->callback.Run(CACHE_STORAGE_OK);
+void CacheStorageCache::UpdateCacheSize() {
+  if (backend_state_ != BACKEND_OPEN)
+    return;
+
+  // Note that the callback holds a refptr to |this| since UpdateCacheSize is
+  // often called after an operation completes and the cache might be freed.
+  int rv = backend_->CalculateSizeOfAllEntries(

[michaeln, 2016/02/12 20:16:53]

i was wondering how expensive this was in the backend

uint64_t SimpleIndex::GetCacheSize() const {
  DCHECK(initialized_);
  return cache_size_;
}

nice :)

[jkarlin, 2016/02/12 20:25:49]

Definitely. It does posttask to get there though.

+      base::Bind(&CacheStorageCache::UpdateCacheSizeGotSize, this));
+
+  if (rv != net::ERR_IO_PENDING)
+    UpdateCacheSizeGotSize(rv);
+}
+
+void CacheStorageCache::UpdateCacheSizeGotSize(int current_cache_size) {
+  int64_t old_cache_size = cache_size_;
+  cache_size_ = current_cache_size;
+
+  quota_manager_proxy_->NotifyStorageModified(
+      storage::QuotaClient::kServiceWorkerCache, origin_,
+      storage::kStorageTypeTemporary, current_cache_size - old_cache_size);
 }
 
 void CacheStorageCache::Delete(const CacheStorageBatchOperation& operation,
                                const ErrorCallback& callback) {
   DCHECK(BACKEND_OPEN == backend_state_ || initializing_);
   DCHECK_EQ(CACHE_STORAGE_CACHE_OPERATION_TYPE_DELETE,
             operation.operation_type);
 
   scoped_ptr<ServiceWorkerFetchRequest> request(new ServiceWorkerFetchRequest(
       operation.request.url, operation.request.method,
@@ skipping 18 matching lines @@
   }
   scoped_ptr<disk_cache::Entry*> entry(new disk_cache::Entry*);
 
   disk_cache::Entry** entry_ptr = entry.get();
 
   ServiceWorkerFetchRequest* request_ptr = request.get();
 
   net::CompletionCallback open_entry_callback = base::Bind(
       &CacheStorageCache::DeleteDidOpenEntry, weak_ptr_factory_.GetWeakPtr(),
       origin_, base::Passed(std::move(request)), callback,
-      base::Passed(std::move(entry)), quota_manager_proxy_);
+      base::Passed(std::move(entry)));
 
   int rv = backend_->OpenEntry(request_ptr->url.spec(), entry_ptr,
                                open_entry_callback);
   if (rv != net::ERR_IO_PENDING)
     open_entry_callback.Run(rv);
 }
 
 void CacheStorageCache::DeleteDidOpenEntry(
     const GURL& origin,
     scoped_ptr<ServiceWorkerFetchRequest> request,
     const CacheStorageCache::ErrorCallback& callback,
     scoped_ptr<disk_cache::Entry*> entry_ptr,
-    const scoped_refptr<storage::QuotaManagerProxy>& quota_manager_proxy,
     int rv) {
   if (rv != net::OK) {
     callback.Run(CACHE_STORAGE_ERROR_NOT_FOUND);
     return;
   }
 
   DCHECK(entry_ptr);
   disk_cache::ScopedEntryPtr entry(*entry_ptr);
 
-  if (quota_manager_proxy.get()) {
-    quota_manager_proxy->NotifyStorageModified(
-        storage::QuotaClient::kServiceWorkerCache, origin,
-        storage::kStorageTypeTemporary,
-        -1 * (entry->GetDataSize(INDEX_HEADERS) +
-              entry->GetDataSize(INDEX_RESPONSE_BODY)));
-  }
+  entry->Doom();
+  entry.reset();
 
-  entry->Doom();
-  callback.Run(CACHE_STORAGE_OK);
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(callback, CACHE_STORAGE_OK));
+  UpdateCacheSize();
 }
 
 void CacheStorageCache::KeysImpl(const RequestsCallback& callback) {
   DCHECK_NE(BACKEND_UNINITIALIZED, backend_state_);
   if (backend_state_ != BACKEND_OPEN) {
     callback.Run(CACHE_STORAGE_ERROR_STORAGE, scoped_ptr<Requests>());
     return;
   }
 
   // 1. Iterate through all of the entries, open them, and add them to a vector.
@@ skipping 71 matching lines @@
   DCHECK_NE(BACKEND_CLOSED, backend_state_);
 
   backend_state_ = BACKEND_CLOSED;
   backend_.reset();
   callback.Run();
 }
 
 void CacheStorageCache::SizeImpl(const SizeCallback& callback) {
   DCHECK_NE(BACKEND_UNINITIALIZED, backend_state_);
 
-  if (backend_state_ != BACKEND_OPEN) {
-    // Backend is closed for deletion, don't count its size.
-    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
-                                                  base::Bind(callback, 0));
-    return;
-  }
-
-  int rv = backend_->CalculateSizeOfAllEntries(
-      base::Bind(&SizeDidGetCacheSize, callback));
-  if (rv != net::ERR_IO_PENDING)
-    SizeDidGetCacheSize(callback, rv);
+  int64_t size = backend_state_ == BACKEND_OPEN ? cache_size_ : 0;
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                base::Bind(callback, size));
 }
 
 void CacheStorageCache::CreateBackend(const ErrorCallback& callback) {
   DCHECK(!backend_);
 
   // Use APP_CACHE as opposed to DISK_CACHE to prevent cache eviction.
   net::CacheType cache_type = memory_only_ ? net::MEMORY_CACHE : net::APP_CACHE;
 
   scoped_ptr<ScopedBackendPtr> backend_ptr(new ScopedBackendPtr());
 
@@ skipping 33 matching lines @@
   DCHECK_EQ(BACKEND_UNINITIALIZED, backend_state_);
 
   if (initializing_)
     return;
 
   DCHECK(!scheduler_->ScheduledOperations());
   initializing_ = true;
 
   scheduler_->ScheduleOperation(base::Bind(
       &CacheStorageCache::CreateBackend, weak_ptr_factory_.GetWeakPtr(),
-      base::Bind(&CacheStorageCache::InitDone,
+      base::Bind(&CacheStorageCache::InitDidCreateBackend,
                  weak_ptr_factory_.GetWeakPtr())));
 }
 
-void CacheStorageCache::InitDone(CacheStorageError error) {
+void CacheStorageCache::InitDidCreateBackend(
+    CacheStorageError cache_create_error) {
+  if (cache_create_error != CACHE_STORAGE_OK) {
+    InitGotCacheSize(cache_create_error, 0);
+    return;
+  }
+
+  int rv = backend_->CalculateSizeOfAllEntries(
+      base::Bind(&CacheStorageCache::InitGotCacheSize,
+                 weak_ptr_factory_.GetWeakPtr(), cache_create_error));
+
+  if (rv != net::ERR_IO_PENDING)
+    InitGotCacheSize(cache_create_error, rv);
+}
+
+void CacheStorageCache::InitGotCacheSize(CacheStorageError cache_create_error,
+                                         int cache_size) {
+  cache_size_ = cache_size;
   initializing_ = false;
-  backend_state_ = (error == CACHE_STORAGE_OK && backend_ &&
+  backend_state_ = (cache_create_error == CACHE_STORAGE_OK && backend_ &&
                     backend_state_ == BACKEND_UNINITIALIZED)
                        ? BACKEND_OPEN
                        : BACKEND_CLOSED;
 
-  UMA_HISTOGRAM_ENUMERATION("ServiceWorkerCache.InitBackendResult", error,
-                            CACHE_STORAGE_ERROR_LAST + 1);
+  UMA_HISTOGRAM_ENUMERATION("ServiceWorkerCache.InitBackendResult",
+                            cache_create_error, CACHE_STORAGE_ERROR_LAST + 1);
 
   scheduler_->CompleteOperationAndRunNext();
 }
 
 void CacheStorageCache::PendingClosure(const base::Closure& callback) {
   base::WeakPtr<CacheStorageCache> cache = weak_ptr_factory_.GetWeakPtr();
 
   callback.Run();
   if (cache)
     scheduler_->CompleteOperationAndRunNext();
@@ skipping 81 matching lines @@
   storage::BlobDataBuilder blob_data(response->blob_uuid);
 
   disk_cache::Entry* temp_entry = entry.get();
   blob_data.AppendDiskCacheEntry(
       new CacheStorageCacheDataHandle(this, std::move(entry)), temp_entry,
       INDEX_RESPONSE_BODY);
   return blob_storage_context_->AddFinishedBlob(&blob_data);
 }
 
 }  // namespace content