Pass arguments separately to HttpCache::GenerateCacheKey()

This is a prerequisite for network service memory cache. Memory cache
in the network service wants to use the same cache key as HttpCache.
Expose GenerateCacheKey() and add a variant of GenerateCacheKey() so
that the network service can call it without creating HttpRequestInfo.

Bug: 1339708
Change-Id: Ib5362f6693e1cde7c6f4f2969be4341d0c11ecd0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3721441
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1018518}

net/http/http_cache.cc

@@ -363,8 +363,8 @@ void HttpCache::OnExternalCacheHit(
       request_info.load_flags |= ~LOAD_DO_NOT_SAVE_COOKIES;
   }
 
-  std::string key =
-      GenerateCacheKey(&request_info, /*use_single_keyed_cache=*/false);
+  std::string key = GenerateCacheKeyForRequest(
+      &request_info, /*use_single_keyed_cache=*/false);
   disk_cache_->OnExternalCacheHit(key);
 }
 
@@ -462,8 +462,8 @@ Error HttpCache::CheckResourceExistence(
 
   // TODO(https://crbug.com/1325315): Support looking in the single-keyed cache
   // for the resource.
-  std::string key =
-      GenerateCacheKey(&request_info, /*use_single_keyed_cache=*/false);
+  std::string key = GenerateCacheKeyForRequest(
+      &request_info, /*use_single_keyed_cache=*/false);
   disk_cache::EntryResult entry_result = disk_cache_->OpenEntry(
       key, net::IDLE,
       base::BindOnce(&HttpCache::ResourceExistenceCheckCallback, GetWeakPtr(),
@@ -476,8 +476,73 @@ Error HttpCache::CheckResourceExistence(
 }
 
 // static
-std::string HttpCache::GenerateCacheKeyForTest(const HttpRequestInfo* request) {
-  return GenerateCacheKey(request, /*use_single_keyed_cache=*/false);
+// Generate a key that can be used inside the cache.
+std::string HttpCache::GenerateCacheKey(
+    const GURL& url,
+    int load_flags,
+    const NetworkIsolationKey& network_isolation_key,
+    int64_t upload_data_identifier,
+    bool is_subframe_document_resource,
+    bool use_single_keyed_cache,
+    const std::string& single_key_checksum) {
+  // The first character of the key may vary depending on whether or not sending
+  // credentials is permitted for this request. This only happens if the
+  // SplitCacheByIncludeCredentials feature is enabled, or if the single-keyed
+  // cache is enabled. The single-keyed cache must always be split by
+  // credentials in order to make coep:credentialless work safely.
+  const char credential_key =
+      ((base::FeatureList::IsEnabled(
+            features::kSplitCacheByIncludeCredentials) ||
+        use_single_keyed_cache) &&
+       (load_flags & LOAD_DO_NOT_SAVE_COOKIES))
+          ? '0'
+          : '1';
+
+  std::string isolation_key;
+  if (use_single_keyed_cache) {
+    DCHECK(IsSplitCacheEnabled());
+    DCHECK(!(load_flags &
+             (net::LOAD_VALIDATE_CACHE | net::LOAD_BYPASS_CACHE |
+              net::LOAD_SKIP_CACHE_VALIDATION | net::LOAD_ONLY_FROM_CACHE |
+              net::LOAD_DISABLE_CACHE | net::LOAD_SKIP_VARY_CHECK)));
+    isolation_key = base::StrCat(
+        {kSingleKeyPrefix, single_key_checksum, kSingleKeySeparator});
+  } else if (IsSplitCacheEnabled()) {
+    // Prepend the key with |kDoubleKeyPrefix| = "_dk_" to mark it as
+    // double-keyed (and makes it an invalid url so that it doesn't get
+    // confused with a single-keyed entry). Separate the origin and url
+    // with invalid whitespace character |kDoubleKeySeparator|.
+    DCHECK(network_isolation_key.IsFullyPopulated());
+    std::string subframe_document_resource_prefix =
+        is_subframe_document_resource ? kSubframeDocumentResourcePrefix : "";
+    isolation_key =
+        base::StrCat({kDoubleKeyPrefix, subframe_document_resource_prefix,
+                      network_isolation_key.ToString(), kDoubleKeySeparator});
+  }
+
+  // The key format is:
+  // credential_key/upload_data_identifier/[isolation_key]url
+
+  // Strip out the reference, username, and password sections of the URL and
+  // concatenate with the credential_key, the post_key, and the network
+  // isolation key if we are splitting the cache.
+  return base::StringPrintf("%c/%" PRId64 "/%s%s", credential_key,
+                            upload_data_identifier, isolation_key.c_str(),
+                            HttpUtil::SpecForRequest(url).c_str());
+}
+
+// static
+std::string HttpCache::GenerateCacheKeyForRequest(
+    const HttpRequestInfo* request,
+    bool use_single_keyed_cache) {
+  DCHECK(request);
+  const int64_t upload_data_identifier =
+      request->upload_data_stream ? request->upload_data_stream->identifier()
+                                  : int64_t(0);
+  return GenerateCacheKey(
+      request->url, request->load_flags, request->network_isolation_key,
+      upload_data_identifier, request->is_subframe_document_resource,
+      use_single_keyed_cache, request->checksum);
 }
 
 // static
@@ -577,61 +642,6 @@ int HttpCache::GetBackendForTransaction(Transaction* transaction) {
   return ERR_IO_PENDING;
 }
 
-// static
-// Generate a key that can be used inside the cache.
-std::string HttpCache::GenerateCacheKey(const HttpRequestInfo* request,
-                                        bool use_single_keyed_cache) {
-  // The first character of the key may vary depending on whether or not sending
-  // credentials is permitted for this request. This only happens if the
-  // SplitCacheByIncludeCredentials feature is enabled, or if the single-keyed
-  // cache is enabled. The single-keyed cache must always be split by
-  // credentials in order to make coep:credentialless work safely.
-  const char credential_key =
-      ((base::FeatureList::IsEnabled(
-            features::kSplitCacheByIncludeCredentials) ||
-        use_single_keyed_cache) &&
-       (request->load_flags & LOAD_DO_NOT_SAVE_COOKIES))
-          ? '0'
-          : '1';
-
-  const int64_t post_key = request->upload_data_stream
-                               ? request->upload_data_stream->identifier()
-                               : int64_t(0);
-  std::string isolation_key;
-  if (use_single_keyed_cache) {
-    DCHECK(IsSplitCacheEnabled());
-    DCHECK(!request->checksum.empty());
-    DCHECK(!(request->load_flags &
-             (net::LOAD_VALIDATE_CACHE | net::LOAD_BYPASS_CACHE |
-              net::LOAD_SKIP_CACHE_VALIDATION | net::LOAD_ONLY_FROM_CACHE |
-              net::LOAD_DISABLE_CACHE | net::LOAD_SKIP_VARY_CHECK)));
-    isolation_key = base::StrCat(
-        {kSingleKeyPrefix, request->checksum, kSingleKeySeparator});
-  } else if (IsSplitCacheEnabled()) {
-    // Prepend the key with |kDoubleKeyPrefix| = "_dk_" to mark it as
-    // double-keyed (and makes it an invalid url so that it doesn't get
-    // confused with a single-keyed entry). Separate the origin and url
-    // with invalid whitespace character |kDoubleKeySeparator|.
-    DCHECK(request->network_isolation_key.IsFullyPopulated());
-    std::string subframe_document_resource_prefix =
-        request->is_subframe_document_resource ? kSubframeDocumentResourcePrefix
-                                               : "";
-    isolation_key = base::StrCat(
-        {kDoubleKeyPrefix, subframe_document_resource_prefix,
-         request->network_isolation_key.ToString(), kDoubleKeySeparator});
-  }
-
-  // The key format is:
-  // credential_key/post_key/[isolation_key]url
-
-  // Strip out the reference, username, and password sections of the URL and
-  // concatenate with the credential_key, the post_key, and the network
-  // isolation key if we are splitting the cache.
-  return base::StringPrintf("%c/%" PRId64 "/%s%s", credential_key, post_key,
-                            isolation_key.c_str(),
-                            HttpUtil::SpecForRequest(request->url).c_str());
-}
-
 void HttpCache::DoomActiveEntry(const std::string& key) {
   auto it = active_entries_.find(key);
   if (it == active_entries_.end())
@@ -708,7 +718,7 @@ void HttpCache::DoomMainEntryForUrl(const GURL& url,
   // single-keyed cache, so therefore it is correct that use_single_keyed_cache
   // be false.
   std::string key =
-      GenerateCacheKey(&temp_info, /*use_single_keyed_cache=*/false);
+      GenerateCacheKeyForRequest(&temp_info, /*use_single_keyed_cache=*/false);
 
   // Defer to DoomEntry if there is an active entry, otherwise call
   // AsyncDoomEntry without triggering a callback.

net/http/http_cache.h

@@ -258,8 +258,18 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory {
   // Get the URL from the entry's cache key.
   static std::string GetResourceURLFromHttpCacheKey(const std::string& key);
 
-  // Function to generate cache key for testing.
-  static std::string GenerateCacheKeyForTest(const HttpRequestInfo* request);
+  // Generates the cache key for a request.
+  static std::string GenerateCacheKey(
+      const GURL& url,
+      int load_flags,
+      const NetworkIsolationKey& network_isolation_key,
+      int64_t upload_data_identifier,
+      bool is_subframe_document_resource,
+      bool use_single_keyed_cache,
+      const std::string& single_key_checksum);
+  static std::string GenerateCacheKeyForRequest(
+      const HttpRequestInfo* request,
+      bool use_single_keyed_cache = false);
 
   // Enable split cache feature if not already overridden in the feature list.
   // Should only be invoked during process initialization before the HTTP
@@ -421,10 +431,6 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory {
   // time after receiving the notification.
   int GetBackendForTransaction(Transaction* transaction);
 
-  // Generates the cache key for this request.
-  static std::string GenerateCacheKey(const HttpRequestInfo*,
-                                      bool use_single_keyed_cache);
-
   // Dooms the entry selected by |key|, if it is currently in the list of active
   // entries.
   void DoomActiveEntry(const std::string& key);

net/http/http_cache_transaction.cc

@@ -1042,7 +1042,7 @@ int HttpCache::Transaction::DoGetBackendComplete(int result) {
     // the entry was marked unusable and the transaction was restarted in
     //  DoCacheReadResponseComplete(), so it will no longer match the value in
     //  `request_`. So we pass it through explicitly.
-    cache_key_ = cache_->GenerateCacheKey(
+    cache_key_ = cache_->GenerateCacheKeyForRequest(
         request_, effective_load_flags_ & LOAD_USE_SINGLE_KEYED_CACHE);
 
     // Requested cache access mode.

net/http/http_cache_unittest.cc

@@ -825,7 +825,7 @@ class HttpSplitCacheKeyTest : public HttpCacheTest {
     request_info.method = "GET";
     request_info.network_isolation_key = net::NetworkIsolationKey(site, site);
     MockHttpCache cache;
-    return cache.http_cache()->GenerateCacheKeyForTest(&request_info);
+    return cache.http_cache()->GenerateCacheKeyForRequest(&request_info);
   }
 };
 
@@ -5336,7 +5336,8 @@ TEST_F(HttpCacheTest, SimpleGET_ManyWriters_CancelFirst) {
   // Allow all requests to move from the Create queue to the active entry.
   // All would have been added to writers.
   base::RunLoop().RunUntilIdle();
-  std::string cache_key = cache.http_cache()->GenerateCacheKeyForTest(&request);
+  std::string cache_key =
+      cache.http_cache()->GenerateCacheKeyForRequest(&request);
   EXPECT_EQ(kNumTransactions, cache.GetCountWriterTransactions(cache_key));
 
   // The second transaction skipped validation, thus only one network
@@ -8305,7 +8306,8 @@ TEST_F(HttpCacheTest, Sparse_WaitForEntry) {
   // Simulate a previous transaction being cancelled.
   disk_cache::Entry* entry;
   MockHttpRequest request(transaction);
-  std::string cache_key = cache.http_cache()->GenerateCacheKeyForTest(&request);
+  std::string cache_key =
+      cache.http_cache()->GenerateCacheKeyForRequest(&request);
   ASSERT_TRUE(cache.OpenBackendEntry(cache_key, &entry));
   entry->CancelSparseIO();
 

net/http/http_transaction_test_util.cc

@@ -204,7 +204,7 @@ MockHttpRequest::MockHttpRequest(const MockTransaction& t) {
 }
 
 std::string MockHttpRequest::CacheKey() {
-  return HttpCache::GenerateCacheKeyForTest(this);
+  return HttpCache::GenerateCacheKeyForRequest(this);
 }
 
 //-----------------------------------------------------------------------------

services/network/http_cache_data_remover_unittest.cc

@@ -123,7 +123,7 @@ class HttpCacheDataRemoverTest : public testing::Test {
     request_info.method = "GET";
     request_info.network_isolation_key =
         net::NetworkIsolationKey(kOrigin, kOrigin);
-    return cache_->GenerateCacheKeyForTest(&request_info);
+    return cache_->GenerateCacheKeyForRequest(&request_info);
   }
 
   void RemoveData(mojom::ClearDataFilterPtr filter,

services/network/network_context_unittest.cc

@@ -1775,8 +1775,8 @@ TEST_F(NetworkContextTest, NotifyExternalCacheHit) {
             is_subframe_document_resource;
         request_info.network_isolation_key = isolation_key;
         disk_cache::EntryResult result = backend->OpenOrCreateEntry(
-            net::HttpCache::GenerateCacheKeyForTest(&request_info), net::LOWEST,
-            base::BindOnce([](disk_cache::EntryResult) {}));
+            net::HttpCache::GenerateCacheKeyForRequest(&request_info),
+            net::LOWEST, base::BindOnce([](disk_cache::EntryResult) {}));
         ASSERT_EQ(result.net_error(), net::OK);
 
         disk_cache::ScopedEntryPtr entry(result.ReleaseEntry());

services/network/resource_scheduler/resource_scheduler_unittest.cc

@@ -522,7 +522,7 @@ class ResourceSchedulerTest : public testing::Test {
     request_info.network_isolation_key =
         net::IsolationInfo().network_isolation_key();
     request_info.is_subframe_document_resource = false;
-    std::string key = net::HttpCache::GenerateCacheKeyForTest(&request_info);
+    std::string key = net::HttpCache::GenerateCacheKeyForRequest(&request_info);
 
     TestEntryResultCompletionCallback create_entry_callback;
     disk_cache::EntryResult result = backend->OpenOrCreateEntry(