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) {
+  return request_url.scheme() == key.scheme() &&
+         request_url.host() == key.host() && request_url.path() == key.path();

[nhiroki, 2016/01/18 04:29:58]

This match algorithm could be insufficient, for example, when the url contains
username/password parts. According to the "Query Cache" algorithm in the sw
spec, request/cached URLs should be serialized by the "URL serializing"
algorithm (https://url.spec.whatwg.org/#url-serializing) and then be compared.

Maybe we could achieve that by creating new KURL objects without search
parameters and compares them by KURL::operator==()?

[zino, 2016/01/19 13:46:38]

I modified it but I couldn't write a test for it.
I'm not sure but Request object seems to contains no username/password parts.
The spec[1] and our implementation[2] says "if parsedURL includes credentials,
throw a TypeError".

[1] https://fetch.spec.whatwg.org/#dom-request (12-3)
[2] http://code.google.com/p/chromium/issues/detail?id=474439

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

Yes, the constructor rejects such a URL.

A Request object made by a regular network request can contain
username/password, for example, clicking "<a
href='http://username:password@example.com/'>".

"LayoutTests/http/tests/cachestorage/serviceworker/credentials.html" is an
example of credential tests.

+}
+
 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),
+  explicit MatchAllContext(scoped_ptr<ServiceWorkerFetchRequest> request,

[nhiroki, 2016/01/18 04:29:58]

Remove "explicit"

[zino, 2016/01/19 13:46:38]

Done.

+                           const CacheStorageCache::ResponsesCallback& callback)

[nhiroki, 2016/01/18 04:29:58]

"CacheStorageCache::" prefix is not necessary.

[zino, 2016/01/19 13:46:38]

Done.

+      : request(std::move(request)),
+        original_callback(callback),
         out_responses(new Responses),
         out_blob_data_handles(new BlobDataHandles) {}
   ~MatchAllContext() {}
 
+  scoped_ptr<ServiceWorkerFetchRequest> request;
+
   // 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) {

[nhiroki, 2016/01/18 04:29:59]

Could you remove this indirection and directly call MatchAll(null_req,
callback)?

[zino, 2016/01/19 13:46:38]

Done.

+  MatchAll(nullptr, callback);
+}
+
+void CacheStorageCache::MatchAll(scoped_ptr<ServiceWorkerFetchRequest> request,
+                                 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);
-  scheduler_->ScheduleOperation(base::Bind(&CacheStorageCache::MatchAllImpl,
-                                           weak_ptr_factory_.GetWeakPtr(),
-                                           pending_callback));
+  scheduler_->ScheduleOperation(base::Bind(
+      &CacheStorageCache::MatchAllImpl, weak_ptr_factory_.GetWeakPtr(),
+      base::Passed(std::move(request)), pending_callback));
 }
 
 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<ServiceWorkerFetchRequest> request,
+    const ResponsesCallback& callback) {
   DCHECK_NE(BACKEND_UNINITIALIZED, backend_state_);
   if (backend_state_ != BACKEND_OPEN) {
     callback.Run(CACHE_STORAGE_ERROR_STORAGE, scoped_ptr<Responses>(),
                  scoped_ptr<BlobDataHandles>());
     return;
   }
 
   OpenAllEntries(base::Bind(&CacheStorageCache::MatchAllDidOpenAllEntries,
-                            weak_ptr_factory_.GetWeakPtr(), callback));
+                            weak_ptr_factory_.GetWeakPtr(),
+                            base::Passed(std::move(request)), callback));
 }
 
 void CacheStorageCache::MatchAllDidOpenAllEntries(
+    scoped_ptr<ServiceWorkerFetchRequest> request,
     const ResponsesCallback& callback,
     scoped_ptr<OpenAllEntriesContext> entries_context,
     CacheStorageError error) {
   if (error != CACHE_STORAGE_OK) {
     callback.Run(error, scoped_ptr<Responses>(), scoped_ptr<BlobDataHandles>());
     return;
   }
 
-  scoped_ptr<MatchAllContext> context(new MatchAllContext(callback));
+  scoped_ptr<MatchAllContext> context(
+      new MatchAllContext(std::move(request), 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;
   }
 
-  ReadMetadata(*iter, base::Bind(&CacheStorageCache::MatchAllDidReadMetadata,
-                                 weak_ptr_factory_.GetWeakPtr(),
-                                 base::Passed(std::move(context)), iter));
+  disk_cache::Entry* entry(*iter);
+  if (!context->request ||
+      URLMatchWithoutQuery(context->request->url, GURL(entry->GetKey()))) {

[nhiroki, 2016/01/18 04:29:59]

This combined condition is a bit confusing to me. How about checking the
|ignore_search| option to clarify that we're handling the case here.

// context->options() is CacheStorageCacheQueryParams passed from
CacheStorageDispatcherHost.
if (context->options().ignore_search) {
  DCHECK(context->request);
  if (!URLMatchWithoutQuery(...))
    ReadMetadata(...);
  else
    MatchAllProcessNextEntry(...);
  return;
}
DCHECK(!context->request);
ReadMetadata(...);

[zino, 2016/01/19 13:46:38]

Done.

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