CacheStorage: Introduce QueryCache algorithm.

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 5a9bb3dbc860c25d9bd8807a972dfeda63b63b44..c400f7dc1d2c30f15974af1b85e4432f040f4dbe 100644
--- a/content/browser/cache_storage/cache_storage_cache.cc
+++ b/content/browser/cache_storage/cache_storage_cache.cc
@@ -210,55 +210,6 @@ struct CacheStorageCache::OpenAllEntriesContext {
   DISALLOW_COPY_AND_ASSIGN(OpenAllEntriesContext);
 };
 
-// The state needed to pass between CacheStorageCache::MatchAll callbacks.
-struct CacheStorageCache::MatchAllContext {
-  MatchAllContext(std::unique_ptr<ServiceWorkerFetchRequest> request,
-                  const CacheStorageCacheQueryParams& match_params,
-                  const ResponsesCallback& callback)
-      : request(std::move(request)),
-        options(match_params),
-        original_callback(callback),
-        out_responses(new Responses),
-        out_blob_data_handles(new BlobDataHandles) {}
-  ~MatchAllContext() {}
-
-  std::unique_ptr<ServiceWorkerFetchRequest> request;
-
-  CacheStorageCacheQueryParams options;
-
-  // The callback passed to the MatchAll() function.
-  ResponsesCallback original_callback;
-
-  // The outputs of the MatchAll function.
-  std::unique_ptr<Responses> out_responses;
-  std::unique_ptr<BlobDataHandles> out_blob_data_handles;
-
-  // The context holding open entries.
-  std::unique_ptr<OpenAllEntriesContext> entries_context;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MatchAllContext);
-};
-
-// The state needed to pass between CacheStorageCache::Keys callbacks.
-struct CacheStorageCache::KeysContext {
-  explicit KeysContext(const CacheStorageCache::RequestsCallback& callback)
-      : original_callback(callback), out_keys(new Requests()) {}
-  ~KeysContext() {}
-
-  // The callback passed to the Keys() function.
-  RequestsCallback original_callback;
-
-  // The output of the Keys function.
-  std::unique_ptr<Requests> out_keys;
-
-  // The context holding open entries.
-  std::unique_ptr<OpenAllEntriesContext> entries_context;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(KeysContext);
-};
-
 // The state needed to pass between CacheStorageCache::Put callbacks.
 struct CacheStorageCache::PutContext {
   PutContext(std::unique_ptr<ServiceWorkerFetchRequest> request,
@@ -281,6 +232,31 @@ struct CacheStorageCache::PutContext {
   DISALLOW_COPY_AND_ASSIGN(PutContext);
 };
 
+struct CacheStorageCache::QueryCacheResults {
+  QueryCacheResults(std::unique_ptr<ServiceWorkerFetchRequest> request,
+                    const CacheStorageCacheQueryParams& options,
+                    const QueryCacheResultsCallback& callback)
+      : request(std::move(request)),
+        options(options),
+        callback(callback),
+        out_requests(new Requests),
+        out_responses(new Responses),
+        out_blob_data_handles(new BlobDataHandles) {}
+
+  std::unique_ptr<ServiceWorkerFetchRequest> request;
+  CacheStorageCacheQueryParams options;
+  QueryCacheResultsCallback callback;
+
+  std::unique_ptr<Requests> out_requests;
+  std::unique_ptr<Responses> out_responses;
+  std::unique_ptr<BlobDataHandles> out_blob_data_handles;
+
+  std::unique_ptr<OpenAllEntriesContext> entries_context;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(QueryCacheResults);
+};
+
 // static
 std::unique_ptr<CacheStorageCache> CacheStorageCache::CreateMemoryCache(
     const GURL& origin,
@@ -346,11 +322,9 @@ void CacheStorageCache::MatchAll(
       base::Bind(&CacheStorageCache::PendingResponsesCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback);
 
-  std::unique_ptr<MatchAllContext> context(
-      new MatchAllContext(std::move(request), match_params, pending_callback));
   scheduler_->ScheduleOperation(base::Bind(&CacheStorageCache::MatchAllImpl,
                                            weak_ptr_factory_.GetWeakPtr(),
-                                           base::Passed(std::move(context))));
+                                           base::Passed(std::move(request)), match_params, pending_callback));

[nhiroki, 2016/07/20 15:44:16]

nit: please wrap at 80 columns

[zino, 2016/07/24 10:14:04]

Done.

 }
 
 void CacheStorageCache::WriteSideData(const ErrorCallback& callback,
@@ -635,6 +609,105 @@ void CacheStorageCache::DidOpenNextEntry(
     open_entry_callback.Run(rv);
 }
 
+void CacheStorageCache::QueryCache(
+    std::unique_ptr<ServiceWorkerFetchRequest> request,
+    const CacheStorageCacheQueryParams& options,
+    const QueryCacheResultsCallback& callback) {
+  DCHECK_NE(BACKEND_UNINITIALIZED, backend_state_);
+  if (backend_state_ != BACKEND_OPEN) {
+    callback.Run(CACHE_STORAGE_ERROR_STORAGE,
+                 std::unique_ptr<QueryCacheResults>());
+    return;
+  }
+
+  std::unique_ptr<QueryCacheResults> query_cache_results(
+      new QueryCacheResults(std::move(request), options, callback));
+  OpenAllEntries(base::Bind(&CacheStorageCache::QueryCacheDidOpenAllEntries,
+                            weak_ptr_factory_.GetWeakPtr(),
+                            base::Passed(std::move(query_cache_results))));
+}
+
+void CacheStorageCache::QueryCacheDidOpenAllEntries(
+    std::unique_ptr<QueryCacheResults> query_cache_results,
+    std::unique_ptr<OpenAllEntriesContext> entries_context,
+    CacheStorageError error) {
+  if (error != CACHE_STORAGE_OK) {
+    query_cache_results->callback.Run(error,
+                                      std::unique_ptr<QueryCacheResults>());
+    return;
+  }
+
+  query_cache_results->entries_context.swap(entries_context);
+  Entries::iterator iter =
+      query_cache_results->entries_context->entries.begin();
+  QueryCacheProcessNextEntry(std::move(query_cache_results), iter);
+}
+
+void CacheStorageCache::QueryCacheProcessNextEntry(
+    std::unique_ptr<QueryCacheResults> query_cache_results,
+    const Entries::iterator& iter) {
+  if (iter == query_cache_results->entries_context->entries.end()) {
+    query_cache_results->callback.Run(CACHE_STORAGE_OK,
+                                      std::move(query_cache_results));
+    return;
+  }
+
+  if (query_cache_results->options.ignore_search) {
+    DCHECK(query_cache_results->request);
+    disk_cache::Entry* entry(*iter);
+    if (RemoveQueryParam(query_cache_results->request->url) !=
+        RemoveQueryParam(GURL(entry->GetKey()))) {
+      QueryCacheProcessNextEntry(std::move(query_cache_results), iter + 1);
+      return;
+    }
+  }
+
+  ReadMetadata(*iter,
+               base::Bind(&CacheStorageCache::QueryCacheDidReadMetadata,
+                          weak_ptr_factory_.GetWeakPtr(),
+                          base::Passed(std::move(query_cache_results)), iter));
+}
+
+void CacheStorageCache::QueryCacheDidReadMetadata(
+    std::unique_ptr<QueryCacheResults> query_cache_results,
+    const Entries::iterator& iter,
+    std::unique_ptr<CacheMetadata> metadata) {
+  disk_cache::ScopedEntryPtr entry(*iter);
+  *iter = nullptr;
+
+  if (!metadata) {
+    entry->Doom();
+    QueryCacheProcessNextEntry(std::move(query_cache_results), iter + 1);
+    return;
+  }
+
+  ServiceWorkerFetchRequest request;
+  PopulateRequestFromMetadata(*metadata, & request);

[nhiroki, 2016/07/20 15:44:17]

There is an extra space between "&" and "request".

[zino, 2016/07/24 10:14:04]

Done.

+  query_cache_results->out_requests->push_back(request);
+
+  ServiceWorkerResponse response;
+  PopulateResponseMetadata(*metadata, &response);
+
+  if (entry->GetDataSize(INDEX_RESPONSE_BODY) == 0) {
+    query_cache_results->out_responses->push_back(response);
+    QueryCacheProcessNextEntry(std::move(query_cache_results), iter + 1);
+    return;
+  }
+
+  if (!blob_storage_context_) {
+    query_cache_results->callback.Run(CACHE_STORAGE_ERROR_STORAGE,
+                                      std::unique_ptr<QueryCacheResults>());
+    return;
+  }
+
+  std::unique_ptr<storage::BlobDataHandle> blob_data_handle =
+      PopulateResponseBody(std::move(entry), &response);
+
+  query_cache_results->out_responses->push_back(response);
+  query_cache_results->out_blob_data_handles->push_back(*blob_data_handle);
+  QueryCacheProcessNextEntry(std::move(query_cache_results), iter + 1);
+}
+
 void CacheStorageCache::MatchImpl(
     std::unique_ptr<ServiceWorkerFetchRequest> request,
     const ResponseCallback& callback) {
@@ -733,98 +806,35 @@ void CacheStorageCache::MatchDidReadMetadata(
                std::move(blob_data_handle));
 }
 
-void CacheStorageCache::MatchAllImpl(std::unique_ptr<MatchAllContext> context) {
+void CacheStorageCache::MatchAllImpl(std::unique_ptr<ServiceWorkerFetchRequest> request,
+                    const CacheStorageCacheQueryParams& options,
+                    const ResponsesCallback& callback) {

[nhiroki, 2016/07/20 15:44:17]

Indents look broken.

[zino, 2016/07/24 10:14:04]

Done.

   DCHECK_NE(BACKEND_UNINITIALIZED, backend_state_);
   if (backend_state_ != BACKEND_OPEN) {
-    context->original_callback.Run(CACHE_STORAGE_ERROR_STORAGE,
+    callback.Run(CACHE_STORAGE_ERROR_STORAGE,
                                    std::unique_ptr<Responses>(),
                                    std::unique_ptr<BlobDataHandles>());
     return;
   }
 
-  OpenAllEntries(base::Bind(&CacheStorageCache::MatchAllDidOpenAllEntries,
-                            weak_ptr_factory_.GetWeakPtr(),
-                            base::Passed(std::move(context))));
-}
-
-void CacheStorageCache::MatchAllDidOpenAllEntries(
-    std::unique_ptr<MatchAllContext> context,
-    std::unique_ptr<OpenAllEntriesContext> entries_context,
-    CacheStorageError error) {
-  if (error != CACHE_STORAGE_OK) {
-    context->original_callback.Run(error, std::unique_ptr<Responses>(),
-                                   std::unique_ptr<BlobDataHandles>());
-    return;
-  }
-
-  context->entries_context.swap(entries_context);
-  Entries::iterator iter = context->entries_context->entries.begin();
-  MatchAllProcessNextEntry(std::move(context), iter);
-}
-
-void CacheStorageCache::MatchAllProcessNextEntry(
-    std::unique_ptr<MatchAllContext> context,
-    const Entries::iterator& iter) {
-  if (iter == context->entries_context->entries.end()) {
-    // All done. Return all of the responses.
-    context->original_callback.Run(CACHE_STORAGE_OK,
-                                   std::move(context->out_responses),
-                                   std::move(context->out_blob_data_handles));
-    return;
-  }
-
-  if (context->options.ignore_search) {
-    DCHECK(context->request);
-    disk_cache::Entry* entry(*iter);
-    if (RemoveQueryParam(context->request->url) !=
-        RemoveQueryParam(GURL(entry->GetKey()))) {
-      // In this case, we don't need to read data.
-      MatchAllProcessNextEntry(std::move(context), iter + 1);
-      return;
-    }
-  }
-
-  ReadMetadata(*iter, base::Bind(&CacheStorageCache::MatchAllDidReadMetadata,
-                                 weak_ptr_factory_.GetWeakPtr(),
-                                 base::Passed(std::move(context)), iter));
+  QueryCache(
+      std::move(request), options,
+      base::Bind(&CacheStorageCache::MatchAllDidQueryCache,
+                 weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
-void CacheStorageCache::MatchAllDidReadMetadata(
-    std::unique_ptr<MatchAllContext> context,
-    const Entries::iterator& iter,
-    std::unique_ptr<CacheMetadata> metadata) {
-  // Move ownership of the entry from the context.
-  disk_cache::ScopedEntryPtr entry(*iter);
-  *iter = nullptr;
-
-  if (!metadata) {
-    entry->Doom();
-    MatchAllProcessNextEntry(std::move(context), iter + 1);
-    return;
-  }
-
-  ServiceWorkerResponse response;
-  PopulateResponseMetadata(*metadata, &response);
-
-  if (entry->GetDataSize(INDEX_RESPONSE_BODY) == 0) {
-    context->out_responses->push_back(response);
-    MatchAllProcessNextEntry(std::move(context), iter + 1);
-    return;
-  }
-
-  if (!blob_storage_context_) {
-    context->original_callback.Run(CACHE_STORAGE_ERROR_STORAGE,
-                                   std::unique_ptr<Responses>(),
-                                   std::unique_ptr<BlobDataHandles>());
+void CacheStorageCache::MatchAllDidQueryCache(
+    const ResponsesCallback& callback,
+    CacheStorageError error,
+    std::unique_ptr<QueryCacheResults> query_cache_results) {
+  if (error != CACHE_STORAGE_OK || !query_cache_results) {

[nhiroki, 2016/07/20 15:44:17]

I suspect the latter condition |!query_cache_results| is never satisfied because
it should be non-null whenever |error| is OK.

[zino, 2016/07/24 10:14:04]

Done.

+    callback.Run(CACHE_STORAGE_ERROR_STORAGE, std::unique_ptr<Responses>(),

[nhiroki, 2016/07/20 15:44:16]

We should pass |error| to the callback in order not to lose information. For
example, ERROR_NOT_FOUND is a meaningful error code for MatchAll (see
CacheStorageDispatcherHost::OnCacheMatchAllCallback)

[zino, 2016/07/24 10:14:04]

Done.

+                 std::unique_ptr<BlobDataHandles>());
     return;
   }
 
-  std::unique_ptr<storage::BlobDataHandle> blob_data_handle =
-      PopulateResponseBody(std::move(entry), &response);
-
-  context->out_responses->push_back(response);
-  context->out_blob_data_handles->push_back(*blob_data_handle);
-  MatchAllProcessNextEntry(std::move(context), iter + 1);
+  callback.Run(CACHE_STORAGE_OK, std::move(query_cache_results->out_responses),
+               std::move(query_cache_results->out_blob_data_handles));
 }
 
 void CacheStorageCache::WriteSideDataDidGetQuota(
@@ -1301,75 +1311,23 @@ void CacheStorageCache::KeysImpl(const RequestsCallback& callback) {
     return;
   }
 
-  // 1. Iterate through all of the entries, open them, and add them to a vector.
-  // 2. For each open entry:
-  //  2.1. Read the headers into a protobuf.
-  //  2.2. Copy the protobuf into a ServiceWorkerFetchRequest (a "key").
-  //  2.3. Push the response into a vector of requests to be returned.
-  // 3. Return the vector of requests (keys).
-
-  // The entries have to be loaded into a vector first because enumeration loops
-  // forever if you read data from a cache entry while enumerating.
-
-  OpenAllEntries(base::Bind(&CacheStorageCache::KeysDidOpenAllEntries,
-                            weak_ptr_factory_.GetWeakPtr(), callback));
+  std::unique_ptr<ServiceWorkerFetchRequest> request(new ServiceWorkerFetchRequest);

[nhiroki, 2016/07/20 15:44:17]

nit (80 columns)

[zino, 2016/07/24 10:14:04]

Done.

+  QueryCache(
+      std::move(request), CacheStorageCacheQueryParams(),
+      base::Bind(&CacheStorageCache::KeysDidQueryCache,
+                 weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
-void CacheStorageCache::KeysDidOpenAllEntries(
+void CacheStorageCache::KeysDidQueryCache(
     const RequestsCallback& callback,
-    std::unique_ptr<OpenAllEntriesContext> entries_context,
-    CacheStorageError error) {
-  if (error != CACHE_STORAGE_OK) {
-    callback.Run(error, std::unique_ptr<Requests>());
-    return;
-  }
-
-  std::unique_ptr<KeysContext> keys_context(new KeysContext(callback));
-  keys_context->entries_context.swap(entries_context);
-  Entries::iterator iter = keys_context->entries_context->entries.begin();
-  KeysProcessNextEntry(std::move(keys_context), iter);
-}
-
-void CacheStorageCache::KeysProcessNextEntry(
-    std::unique_ptr<KeysContext> keys_context,
-    const Entries::iterator& iter) {
-  if (iter == keys_context->entries_context->entries.end()) {
-    // All done. Return all of the keys.
-    keys_context->original_callback.Run(CACHE_STORAGE_OK,
-                                        std::move(keys_context->out_keys));
+    CacheStorageError error,
+    std::unique_ptr<QueryCacheResults> query_cache_results) {
+  if (error != CACHE_STORAGE_OK || !query_cache_results) {
+    callback.Run(CACHE_STORAGE_ERROR_STORAGE, std::unique_ptr<Requests>());

[nhiroki, 2016/07/20 15:44:17]

ditto (|!query_cache_results| and an error code)

[zino, 2016/07/24 10:14:04]

Done.

     return;
   }
 
-  ReadMetadata(*iter, base::Bind(&CacheStorageCache::KeysDidReadMetadata,
-                                 weak_ptr_factory_.GetWeakPtr(),
-                                 base::Passed(std::move(keys_context)), iter));
-}
-
-void CacheStorageCache::KeysDidReadMetadata(
-    std::unique_ptr<KeysContext> keys_context,
-    const Entries::iterator& iter,
-    std::unique_ptr<CacheMetadata> metadata) {
-  disk_cache::Entry* entry = *iter;
-
-  if (metadata) {
-    keys_context->out_keys->push_back(ServiceWorkerFetchRequest(
-        GURL(entry->GetKey()), metadata->request().method(),
-        ServiceWorkerHeaderMap(), Referrer(), false));
-
-    ServiceWorkerHeaderMap& req_headers =
-        keys_context->out_keys->back().headers;
-
-    for (int i = 0; i < metadata->request().headers_size(); ++i) {
-      const CacheHeaderMap header = metadata->request().headers(i);
-      DCHECK_EQ(std::string::npos, header.name().find('\0'));
-      DCHECK_EQ(std::string::npos, header.value().find('\0'));
-      req_headers.insert(std::make_pair(header.name(), header.value()));
-    }
-  } else {
-    entry->Doom();
-  }
-
-  KeysProcessNextEntry(std::move(keys_context), iter + 1);
+  callback.Run(CACHE_STORAGE_OK, std::move(query_cache_results->out_requests));
 }
 
 void CacheStorageCache::CloseImpl(const base::Closure& callback) {
@@ -1539,6 +1497,32 @@ void CacheStorageCache::PendingSizeCallback(const SizeCallback& callback,
     scheduler_->CompleteOperationAndRunNext();
 }
 
+void CacheStorageCache::PendingQueryCacheResultsCallback(
+    const QueryCacheResultsCallback& callback,
+    CacheStorageError error,
+    std::unique_ptr<QueryCacheResults> results) {
+  base::WeakPtr<CacheStorageCache> cache = weak_ptr_factory_.GetWeakPtr();
+
+  callback.Run(error, std::move(results));
+  if (cache)
+    scheduler_->CompleteOperationAndRunNext();
+}
+
+void CacheStorageCache::PopulateRequestFromMetadata(
+    const CacheMetadata& metadata,
+    ServiceWorkerFetchRequest* request) {
+  *request = ServiceWorkerFetchRequest(
+      GURL(metadata.response().url()), metadata.request().method(),

[nhiroki, 2016/07/20 15:44:17]

Regarding the first argument of ServiceWorkerFetchRequest(), the original code
uses |entry->GetKey()| that is a URL of a request, but this code uses response's
URL. I guess these can be different, for example, when a responses is stored by
Cache.put().

[zino, 2016/07/24 10:14:04]

Done.

+      ServiceWorkerHeaderMap(), Referrer(), false);
+
+  for (int i = 0; i < metadata.request().headers_size(); ++i) {
+    const CacheHeaderMap header = metadata.request().headers(i);
+    DCHECK_EQ(std::string::npos, header.name().find('\0'));
+    DCHECK_EQ(std::string::npos, header.value().find('\0'));
+    request->headers.insert(std::make_pair(header.name(), header.value()));
+  }
+}
+
 void CacheStorageCache::PopulateResponseMetadata(
     const CacheMetadata& metadata,
     ServiceWorkerResponse* response) {