CacheStorage: Add ignoreSearch option to cache.matchAll().

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 129 matching lines @@
     // If the header exists in one, it exists in both. Verify that the values
     // are equal.
     if (request_iter != request.end() &&
         request_iter->second != cached_request_iter->second)
       return false;
   }
 
   return true;
 }
 
+bool URLMatchWithoutQuery(const GURL& request_url, const GURL& key) {

[nhiroki, 2016/01/20 08:56:02]

This function name sounds a bit strange to me.

This does 2 things: (1) removing query parts from given URLs and (2) comparing
them. Such mixed functions seem to make it difficult to name this. Instead, how
about making a simple "GURL RemoveQueryParam(const GURL& url)" just to remove a
query part and calling it in MatchAllProcessNextEntry() like this?:

if (RemoveQueryParam(context->request->url) !=
RemoveQueryParam(GURL(entry->GetKey())) { ... }

[zino, 2016/02/03 12:04:47]

Done.

+  url::Replacements<char> replacements;
+  replacements.ClearQuery();
+  return request_url.ReplaceComponents(replacements) ==
+         key.ReplaceComponents(replacements);
+}
+
 void ReadMetadata(disk_cache::Entry* entry, const MetadataCallback& callback) {
   DCHECK(entry);
 
   scoped_refptr<net::IOBufferWithSize> buffer(
       new net::IOBufferWithSize(entry->GetDataSize(INDEX_HEADERS)));
 
   net::CompletionCallback read_header_callback =
       base::Bind(ReadMetadataDidReadMetadata, entry, callback, buffer);
 
   int read_rv = entry->ReadData(INDEX_HEADERS, 0, buffer.get(), buffer->size(),
@@ skipping 43 matching lines @@
   // Used for enumerating cache entries.
   scoped_ptr<disk_cache::Backend::Iterator> backend_iterator;
   disk_cache::Entry* enumerated_entry;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(OpenAllEntriesContext);
 };
 
 // The state needed to pass between CacheStorageCache::MatchAll callbacks.
 struct CacheStorageCache::MatchAllContext {
-  explicit MatchAllContext(const CacheStorageCache::ResponsesCallback& callback)
-      : original_callback(callback),
+  MatchAllContext(scoped_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() {}
 
+  scoped_ptr<ServiceWorkerFetchRequest> request;
+
+  CacheStorageCacheQueryParams options;
+
   // The callback passed to the MatchAll() function.
   ResponsesCallback original_callback;
 
   // The outputs of the MatchAll function.
   scoped_ptr<Responses> out_responses;
   scoped_ptr<BlobDataHandles> out_blob_data_handles;
 
   // The context holding open entries.
   scoped_ptr<OpenAllEntriesContext> entries_context;
 
@@ skipping 91 matching lines @@
   }
 
   ResponseCallback pending_callback =
       base::Bind(&CacheStorageCache::PendingResponseCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback);
   scheduler_->ScheduleOperation(
       base::Bind(&CacheStorageCache::MatchImpl, weak_ptr_factory_.GetWeakPtr(),
                  base::Passed(std::move(request)), pending_callback));
 }
 
-void CacheStorageCache::MatchAll(const ResponsesCallback& callback) {
+void CacheStorageCache::MatchAll(
+    scoped_ptr<ServiceWorkerFetchRequest> request,
+    const CacheStorageCacheQueryParams& match_params,
+    const ResponsesCallback& callback) {
   if (!LazyInitialize()) {
     callback.Run(CACHE_STORAGE_ERROR_STORAGE, scoped_ptr<Responses>(),
                  scoped_ptr<BlobDataHandles>());
     return;
   }
 
   ResponsesCallback pending_callback =
       base::Bind(&CacheStorageCache::PendingResponsesCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback);
+
+  scoped_ptr<MatchAllContext> context(
+      new MatchAllContext(std::move(request), match_params, pending_callback));
   scheduler_->ScheduleOperation(base::Bind(&CacheStorageCache::MatchAllImpl,
                                            weak_ptr_factory_.GetWeakPtr(),
-                                           pending_callback));
+                                           base::Passed(std::move(context))));
 }
 
 void CacheStorageCache::BatchOperation(
     const std::vector<CacheStorageBatchOperation>& operations,
     const ErrorCallback& callback) {
   if (!LazyInitialize()) {
     callback.Run(CACHE_STORAGE_ERROR_STORAGE);
     return;
   }
 
@@ skipping 278 matching lines @@
                  scoped_ptr<storage::BlobDataHandle>());
     return;
   }
 
   scoped_ptr<storage::BlobDataHandle> blob_data_handle =
       PopulateResponseBody(std::move(entry), response.get());
   callback.Run(CACHE_STORAGE_OK, std::move(response),
                std::move(blob_data_handle));
 }
 
-void CacheStorageCache::MatchAllImpl(const ResponsesCallback& callback) {
+void CacheStorageCache::MatchAllImpl(scoped_ptr<MatchAllContext> context) {
   DCHECK_NE(BACKEND_UNINITIALIZED, backend_state_);
   if (backend_state_ != BACKEND_OPEN) {
-    callback.Run(CACHE_STORAGE_ERROR_STORAGE, scoped_ptr<Responses>(),
-                 scoped_ptr<BlobDataHandles>());
+    context->original_callback.Run(CACHE_STORAGE_ERROR_STORAGE,
+                                   std::move(context->out_responses),
+                                   std::move(context->out_blob_data_handles));

[nhiroki, 2016/01/20 08:56:02]

Do you have any reason to change these parameters? I think it'd be common
pattern to pass explicit empty results to an error callback as the original code
does.

[zino, 2016/02/03 12:04:47]

Done.

I thought that we need to avoid creating object duplicately (even if it's a
empty pointer object) if possible.
The out_responses and blob_data_handles were already initialized as empty
object.

On second thoughts, I was wrong and readability seems more important.

     return;
   }
 
   OpenAllEntries(base::Bind(&CacheStorageCache::MatchAllDidOpenAllEntries,
-                            weak_ptr_factory_.GetWeakPtr(), callback));
+                            weak_ptr_factory_.GetWeakPtr(),
+                            base::Passed(std::move(context))));
 }
 
 void CacheStorageCache::MatchAllDidOpenAllEntries(
-    const ResponsesCallback& callback,
+    scoped_ptr<MatchAllContext> context,
     scoped_ptr<OpenAllEntriesContext> entries_context,
     CacheStorageError error) {
   if (error != CACHE_STORAGE_OK) {
-    callback.Run(error, scoped_ptr<Responses>(), scoped_ptr<BlobDataHandles>());
+    context->original_callback.Run(error, std::move(context->out_responses),
+                                   std::move(context->out_blob_data_handles));

[nhiroki, 2016/01/20 08:56:02]

ditto.

[zino, 2016/02/03 12:04:47]

Done.

     return;
   }
 
-  scoped_ptr<MatchAllContext> context(new MatchAllContext(callback));
   context->entries_context.swap(entries_context);
   Entries::iterator iter = context->entries_context->entries.begin();
   MatchAllProcessNextEntry(std::move(context), iter);
 }
 
 void CacheStorageCache::MatchAllProcessNextEntry(
     scoped_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;
   }
 
+  disk_cache::Entry* entry(*iter);

[nhiroki, 2016/01/20 08:56:02]

Can you move this to line 705?

[zino, 2016/02/03 12:04:47]

Done.

+  if (context->options.ignore_search) {
+    DCHECK(context->request);
+    if (!URLMatchWithoutQuery(context->request->url, GURL(entry->GetKey()))) {
+      // In this caase, we don't need to read data.

[nhiroki, 2016/01/20 08:56:02]

s/casse/case

[zino, 2016/02/03 12:04:47]

Done.

+      MatchAllProcessNextEntry(std::move(context), iter + 1);
+      return;
+    }
+  }
+
   ReadMetadata(*iter, base::Bind(&CacheStorageCache::MatchAllDidReadMetadata,
                                  weak_ptr_factory_.GetWeakPtr(),
                                  base::Passed(std::move(context)), iter));
 }
 
 void CacheStorageCache::MatchAllDidReadMetadata(
     scoped_ptr<MatchAllContext> context,
     const Entries::iterator& iter,
     scoped_ptr<CacheMetadata> metadata) {
   // Move ownership of the entry from the context.
@@ skipping 567 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