Create blobs from Disk Cache entries.

content/browser/cache_storage/cache_storage_cache.cc

Index: content/browser/cache_storage/cache_storage_cache.cc
diff --git a/content/browser/cache_storage/cache_storage_cache.cc b/content/browser/cache_storage/cache_storage_cache.cc
index b7c92d757a44f1b7f2cf2359e953912b342589f3..87a5e643fcd55255f003cfcfbfd3b6a23e4a7da2 100644
--- a/content/browser/cache_storage/cache_storage_cache.cc
+++ b/content/browser/cache_storage/cache_storage_cache.cc
@@ -31,8 +31,24 @@ namespace content {
 
 namespace {
 
-typedef base::Callback<void(disk_cache::ScopedEntryPtr, bool)>
-    EntryBoolCallback;
+// This class ensures that the cache and the entry have a lifetime as long as
+// the blob that is created to contain them.
+class CacheStorageCacheDataHandle
+    : public storage::BlobDataBuilder::DataHandle {
+ public:
+  CacheStorageCacheDataHandle(const scoped_refptr<CacheStorageCache>& cache,
+                              disk_cache::ScopedEntryPtr entry)
+      : cache_(cache), entry_(entry.Pass()) {}
+
+ private:
+  ~CacheStorageCacheDataHandle() override {}
+
+  scoped_refptr<CacheStorageCache> cache_;
+  disk_cache::ScopedEntryPtr entry_;
+
+  DISALLOW_COPY_AND_ASSIGN(CacheStorageCacheDataHandle);
+};
+
 typedef base::Callback<void(scoped_ptr<CacheMetadata>)> MetadataCallback;
 
 enum EntryIndex { INDEX_HEADERS = 0, INDEX_RESPONSE_BODY };
@@ -306,38 +322,6 @@ struct CacheStorageCache::KeysContext {
   DISALLOW_COPY_AND_ASSIGN(KeysContext);
 };
 
-struct CacheStorageCache::MatchContext {
-  MatchContext(scoped_ptr<ServiceWorkerFetchRequest> request,
-               const CacheStorageCache::ResponseCallback& callback,
-               base::WeakPtr<storage::BlobStorageContext> blob_storage_context)
-      : request(request.Pass()),
-        original_callback(callback),
-        blob_storage_context(blob_storage_context),
-        entry(nullptr),
-        total_bytes_read(0) {}
-
-  ~MatchContext() {
-    if (entry)
-      entry->Close();
-  }
-
-  // Input
-  scoped_ptr<ServiceWorkerFetchRequest> request;
-  CacheStorageCache::ResponseCallback original_callback;
-  base::WeakPtr<storage::BlobStorageContext> blob_storage_context;
-  disk_cache::Entry* entry;
-
-  // Output
-  scoped_ptr<ServiceWorkerResponse> response;
-  scoped_ptr<storage::BlobDataBuilder> blob_data;
-
-  // For reading the cache entry data into a blob.
-  scoped_refptr<net::IOBufferWithSize> response_body_buffer;
-  size_t total_bytes_read;
-
-  DISALLOW_COPY_AND_ASSIGN(MatchContext);
-};
-
 // The state needed to pass between CacheStorageCache::Put callbacks.
 struct CacheStorageCache::PutContext {
   PutContext(
@@ -354,12 +338,7 @@ struct CacheStorageCache::PutContext {
         blob_data_handle(blob_data_handle.Pass()),
         callback(callback),
         request_context_getter(request_context_getter),
-        quota_manager_proxy(quota_manager_proxy),
-        cache_entry(NULL) {}
-  ~PutContext() {
-    if (cache_entry)
-      cache_entry->Close();
-  }
+        quota_manager_proxy(quota_manager_proxy) {}
 
   // Input parameters to the Put function.
   GURL origin;
@@ -369,10 +348,7 @@ struct CacheStorageCache::PutContext {
   CacheStorageCache::ErrorCallback callback;
   scoped_refptr<net::URLRequestContextGetter> request_context_getter;
   scoped_refptr<storage::QuotaManagerProxy> quota_manager_proxy;
-
-  // This isn't a scoped_ptr because the disk_cache needs an Entry** as input to
-  // CreateEntry.
-  disk_cache::Entry* cache_entry;
+  disk_cache::ScopedEntryPtr cache_entry;
 
   DISALLOW_COPY_AND_ASSIGN(PutContext);
 };
@@ -584,15 +560,14 @@ void CacheStorageCache::MatchImpl(scoped_ptr<ServiceWorkerFetchRequest> request,
     return;
   }
 
-  scoped_ptr<MatchContext> match_context(
-      new MatchContext(request.Pass(), callback, blob_storage_context_));
-
-  disk_cache::Entry** entry_ptr = &match_context->entry;
-  ServiceWorkerFetchRequest* request_ptr = match_context->request.get();
+  scoped_ptr<disk_cache::Entry*> scoped_entry_ptr(new disk_cache::Entry*());
+  disk_cache::Entry** entry_ptr = scoped_entry_ptr.get();
+  ServiceWorkerFetchRequest* request_ptr = request.get();
 
-  net::CompletionCallback open_entry_callback = base::Bind(
-      &CacheStorageCache::MatchDidOpenEntry, weak_ptr_factory_.GetWeakPtr(),
-      base::Passed(match_context.Pass()));
+  net::CompletionCallback open_entry_callback =
+      base::Bind(&CacheStorageCache::MatchDidOpenEntry,
+                 weak_ptr_factory_.GetWeakPtr(), base::Passed(request.Pass()),
+                 callback, base::Passed(scoped_entry_ptr.Pass()));
 
   int rv = backend_->OpenEntry(request_ptr->url.spec(), entry_ptr,
                                open_entry_callback);
@@ -601,44 +576,43 @@ void CacheStorageCache::MatchImpl(scoped_ptr<ServiceWorkerFetchRequest> request,
 }
 
 void CacheStorageCache::MatchDidOpenEntry(
-    scoped_ptr<MatchContext> match_context,
+    scoped_ptr<ServiceWorkerFetchRequest> request,
+    const ResponseCallback& callback,
+    scoped_ptr<disk_cache::Entry*> entry_ptr,
     int rv) {
   if (rv != net::OK) {
-    match_context->original_callback.Run(CACHE_STORAGE_ERROR_NOT_FOUND,
-                                         scoped_ptr<ServiceWorkerResponse>(),
-                                         scoped_ptr<storage::BlobDataHandle>());
+    callback.Run(CACHE_STORAGE_ERROR_NOT_FOUND,
+                 scoped_ptr<ServiceWorkerResponse>(),
+                 scoped_ptr<storage::BlobDataHandle>());
     return;
   }
-
-  // Copy the entry pointer before passing it in base::Bind.
-  disk_cache::Entry* tmp_entry_ptr = match_context->entry;
-  DCHECK(tmp_entry_ptr);
+  disk_cache::ScopedEntryPtr entry(*entry_ptr);
 
   MetadataCallback headers_callback = base::Bind(
       &CacheStorageCache::MatchDidReadMetadata, weak_ptr_factory_.GetWeakPtr(),
-      base::Passed(match_context.Pass()));
+      base::Passed(request.Pass()), callback, base::Passed(entry.Pass()));
 
-  ReadMetadata(tmp_entry_ptr, headers_callback);
+  ReadMetadata(*entry_ptr, headers_callback);
 }
 
 void CacheStorageCache::MatchDidReadMetadata(
-    scoped_ptr<MatchContext> match_context,
+    scoped_ptr<ServiceWorkerFetchRequest> request,
+    const ResponseCallback& callback,
+    disk_cache::ScopedEntryPtr entry,
     scoped_ptr<CacheMetadata> metadata) {
   if (!metadata) {
-    match_context->original_callback.Run(CACHE_STORAGE_ERROR_STORAGE,
-                                         scoped_ptr<ServiceWorkerResponse>(),
-                                         scoped_ptr<storage::BlobDataHandle>());
+    callback.Run(CACHE_STORAGE_ERROR_STORAGE,
+                 scoped_ptr<ServiceWorkerResponse>(),
+                 scoped_ptr<storage::BlobDataHandle>());
     return;
   }
 
-  match_context->response.reset(new ServiceWorkerResponse(
-      match_context->request->url, metadata->response().status_code(),
+  scoped_ptr<ServiceWorkerResponse> response(new ServiceWorkerResponse(
+      request->url, metadata->response().status_code(),
       metadata->response().status_text(),
       ProtoResponseTypeToWebResponseType(metadata->response().response_type()),
       ServiceWorkerHeaderMap(), "", 0, GURL()));
 
-  ServiceWorkerResponse* response = match_context->response.get();
-
   if (metadata->response().has_url())
     response->url = GURL(metadata->response().url());
 
@@ -657,107 +631,40 @@ void CacheStorageCache::MatchDidReadMetadata(
     cached_request_headers[header.name()] = header.value();
   }
 
-  if (!VaryMatches(match_context->request->headers, cached_request_headers,
+  if (!VaryMatches(request->headers, cached_request_headers,
                    response->headers)) {
-    match_context->original_callback.Run(CACHE_STORAGE_ERROR_NOT_FOUND,
-                                         scoped_ptr<ServiceWorkerResponse>(),
-                                         scoped_ptr<storage::BlobDataHandle>());
+    callback.Run(CACHE_STORAGE_ERROR_NOT_FOUND,
+                 scoped_ptr<ServiceWorkerResponse>(),
+                 scoped_ptr<storage::BlobDataHandle>());
     return;
   }
 
-  if (match_context->entry->GetDataSize(INDEX_RESPONSE_BODY) == 0) {
-    match_context->original_callback.Run(CACHE_STORAGE_OK,
-                                         match_context->response.Pass(),
-                                         scoped_ptr<storage::BlobDataHandle>());
+  if (entry->GetDataSize(INDEX_RESPONSE_BODY) == 0) {
+    callback.Run(CACHE_STORAGE_OK, response.Pass(),
+                 scoped_ptr<storage::BlobDataHandle>());
     return;
   }
 
-  // Stream the response body into a blob.
-  if (!match_context->blob_storage_context) {
-    match_context->original_callback.Run(CACHE_STORAGE_ERROR_STORAGE,
-                                         scoped_ptr<ServiceWorkerResponse>(),
-                                         scoped_ptr<storage::BlobDataHandle>());
+  if (!blob_storage_context_) {
+    callback.Run(CACHE_STORAGE_ERROR_STORAGE,
+                 scoped_ptr<ServiceWorkerResponse>(),
+                 scoped_ptr<storage::BlobDataHandle>());
     return;
   }
 
+  // Create a blob with the response body data.
+  response->blob_size = entry->GetDataSize(INDEX_RESPONSE_BODY);
   response->blob_uuid = base::GenerateGUID();
-
-  match_context->blob_data.reset(
+  scoped_ptr<storage::BlobDataBuilder> blob_data(

[michaeln, 2015/06/12 23:43:06]

looks like BlobDataBuilder could be stack allocated instead of heap allocated
now?

[gavinp, 2015/06/15 14:01:19]

Done.

       new storage::BlobDataBuilder(response->blob_uuid));
-  match_context->response_body_buffer = new net::IOBufferWithSize(kBufferSize);
-
-  disk_cache::Entry* tmp_entry_ptr = match_context->entry;
-  net::IOBufferWithSize* response_body_buffer =
-      match_context->response_body_buffer.get();
-
-  net::CompletionCallback read_callback = base::Bind(
-      &CacheStorageCache::MatchDidReadResponseBodyData,
-      weak_ptr_factory_.GetWeakPtr(), base::Passed(match_context.Pass()));
-
-  int read_rv =
-      tmp_entry_ptr->ReadData(INDEX_RESPONSE_BODY, 0, response_body_buffer,
-                              response_body_buffer->size(), read_callback);
-
-  if (read_rv != net::ERR_IO_PENDING)
-    read_callback.Run(read_rv);
-}
-
-void CacheStorageCache::MatchDidReadResponseBodyData(
-    scoped_ptr<MatchContext> match_context,
-    int rv) {
-  if (rv < 0) {
-    match_context->original_callback.Run(CACHE_STORAGE_ERROR_STORAGE,
-                                         scoped_ptr<ServiceWorkerResponse>(),
-                                         scoped_ptr<storage::BlobDataHandle>());
-    return;
-  }
-
-  if (rv == 0) {
-    match_context->response->blob_uuid = match_context->blob_data->uuid();
-    match_context->response->blob_size = match_context->total_bytes_read;
-    MatchDoneWithBody(match_context.Pass());
-    return;
-  }
-
-  // TODO(jkarlin): This copying of the the entire cache response into memory is
-  // awful. Create a new interface around SimpleCache that provides access the
-  // data directly from the file. See bug http://crbug.com/403493.
-  match_context->blob_data->AppendData(
-      match_context->response_body_buffer->data(), rv);
-  match_context->total_bytes_read += rv;
-  int total_bytes_read = match_context->total_bytes_read;
-
-  // Grab some pointers before passing match_context in bind.
-  net::IOBufferWithSize* buffer = match_context->response_body_buffer.get();
-  disk_cache::Entry* tmp_entry_ptr = match_context->entry;
-
-  net::CompletionCallback read_callback = base::Bind(
-      &CacheStorageCache::MatchDidReadResponseBodyData,
-      weak_ptr_factory_.GetWeakPtr(), base::Passed(match_context.Pass()));
-
-  int read_rv = tmp_entry_ptr->ReadData(INDEX_RESPONSE_BODY, total_bytes_read,
-                                        buffer, buffer->size(), read_callback);
-
-  if (read_rv != net::ERR_IO_PENDING)
-    read_callback.Run(read_rv);
-}
-
-void CacheStorageCache::MatchDoneWithBody(
-    scoped_ptr<MatchContext> match_context) {
-  if (!match_context->blob_storage_context) {
-    match_context->original_callback.Run(CACHE_STORAGE_ERROR_STORAGE,
-                                         scoped_ptr<ServiceWorkerResponse>(),
-                                         scoped_ptr<storage::BlobDataHandle>());
-    return;
-  }
 
+  disk_cache::Entry* tmp_entry = entry.get();
+  blob_data->AppendDiskCacheEntry(
+      new CacheStorageCacheDataHandle(this, entry.Pass()), tmp_entry,
+      INDEX_RESPONSE_BODY);
   scoped_ptr<storage::BlobDataHandle> blob_data_handle(
-      match_context->blob_storage_context->AddFinishedBlob(
-          match_context->blob_data.get()));
-
-  match_context->original_callback.Run(CACHE_STORAGE_OK,
-                                       match_context->response.Pass(),
-                                       blob_data_handle.Pass());
+      blob_storage_context_->AddFinishedBlob(blob_data.get()));
+  callback.Run(CACHE_STORAGE_OK, response.Pass(), blob_data_handle.Pass());
 }
 
 void CacheStorageCache::Put(const CacheStorageBatchOperation& operation,
@@ -828,13 +735,14 @@ void CacheStorageCache::PutDidDelete(scoped_ptr<PutContext> put_context,
     return;
   }
 
-  disk_cache::Entry** entry_ptr = &put_context->cache_entry;
+  scoped_ptr<disk_cache::Entry*> scoped_entry_ptr(new disk_cache::Entry*());
+  disk_cache::Entry** entry_ptr = scoped_entry_ptr.get();
   ServiceWorkerFetchRequest* request_ptr = put_context->request.get();
   disk_cache::Backend* backend_ptr = backend_.get();
 
   net::CompletionCallback create_entry_callback = base::Bind(
       &CacheStorageCache::PutDidCreateEntry, weak_ptr_factory_.GetWeakPtr(),
-      base::Passed(put_context.Pass()));
+      base::Passed(scoped_entry_ptr.Pass()), base::Passed(put_context.Pass()));
 
   int create_rv = backend_ptr->CreateEntry(request_ptr->url.spec(), entry_ptr,
                                            create_entry_callback);
@@ -843,14 +751,15 @@ void CacheStorageCache::PutDidDelete(scoped_ptr<PutContext> put_context,
     create_entry_callback.Run(create_rv);
 }
 
-void CacheStorageCache::PutDidCreateEntry(scoped_ptr<PutContext> put_context,
-                                          int rv) {
+void CacheStorageCache::PutDidCreateEntry(
+    scoped_ptr<disk_cache::Entry*> entry_ptr,
+    scoped_ptr<PutContext> put_context,
+    int rv) {
   if (rv != net::OK) {
     put_context->callback.Run(CACHE_STORAGE_ERROR_EXISTS);
     return;
   }
-
-  DCHECK(put_context->cache_entry);
+  put_context->cache_entry.reset(*entry_ptr);
 
   CacheMetadata metadata;
   CacheRequest* request_metadata = metadata.mutable_request();
@@ -891,7 +800,7 @@ void CacheStorageCache::PutDidCreateEntry(scoped_ptr<PutContext> put_context,
       new net::StringIOBuffer(serialized.Pass()));
 
   // Get a temporary copy of the entry pointer before passing it in base::Bind.
-  disk_cache::Entry* tmp_entry_ptr = put_context->cache_entry;
+  disk_cache::Entry* tmp_entry_ptr = put_context->cache_entry.get();
 
   net::CompletionCallback write_headers_callback = base::Bind(
       &CacheStorageCache::PutDidWriteHeaders, weak_ptr_factory_.GetWeakPtr(),
@@ -931,8 +840,7 @@ void CacheStorageCache::PutDidWriteHeaders(scoped_ptr<PutContext> put_context,
 
   DCHECK(put_context->blob_data_handle);
 
-  disk_cache::ScopedEntryPtr entry(put_context->cache_entry);
-  put_context->cache_entry = NULL;
+  disk_cache::ScopedEntryPtr entry(put_context->cache_entry.Pass());
   scoped_ptr<BlobReader> reader(new BlobReader());
   BlobReader* reader_ptr = reader.get();
 
@@ -956,7 +864,7 @@ void CacheStorageCache::PutDidWriteBlobToCache(
     disk_cache::ScopedEntryPtr entry,
     bool success) {
   DCHECK(entry);
-  put_context->cache_entry = entry.release();
+  put_context->cache_entry = entry.Pass();
 
   if (!success) {
     put_context->cache_entry->Doom();