Skip to content

Commit

Permalink
Split cache per request's includeCredentials
Browse files Browse the repository at this point in the history
whatwg/fetch issue:
whatwg/fetch#1253

Design doc:
https://docs.google.com/document/d/1lvbiy4n-GM5I56Ncw304sgvY5Td32R6KHitjRXvkZ6U/edit#

Add the feature "SplitCacheByIncludeCredentials".
When enabled, it makes the HTTP cache to differentiate the requests sent
anonymously from the ones sent with credentials.

This avoids anonymous requests from getting responses requested with
credentials.

This aligns Chrome toward Firefox. See Firefox implementations:
https://github.com/mozilla/gecko-dev/blob/b31b78eea683b0eb341c676adb422cd129909fe9/netwerk/protocol/http/nsHttpChannel.cpp#L4117

Goal after this patch is to start a finch experiment to check for
potential performance regressions.

VIRTUAL_OWNERS: This adds 13 tests from http-cache WPT directory to be
run with the feature enabled. Tests are often "any.js" tests, causing
46 run.

Bug: 1221529
Change-Id: I0f1c969eb2c3401e9548c5d1e6ec63570166d7e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3010379
Auto-Submit: Arthur Sonzogni <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Reviewed-by: Mike West <[email protected]>
Reviewed-by: Camille Lamy <[email protected]>
Reviewed-by: Maksim Orlovich <[email protected]>
Commit-Queue: Arthur Sonzogni <[email protected]>
Cr-Commit-Position: refs/heads/main@{#918101}
NOKEYCHECK=True
GitOrigin-RevId: 54013f2a5d6c06a909cbc7318895c2ccf4039921
  • Loading branch information
ArthurSonzogni authored and copybara-github committed Sep 3, 2021
1 parent c8840b9 commit 5652727
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 4 deletions.
3 changes: 2 additions & 1 deletion blink/public/mojom/frame/frame.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,8 @@ interface LocalFrameHost {
// Note: May only be sent once per URL per frame per committed load.
DidLoadResourceFromMemoryCache(
url.mojom.Url url, string http_method,
string mime_type, network.mojom.RequestDestination request_destination);
string mime_type, network.mojom.RequestDestination request_destination,
bool include_credentials);

// Notifies the browser that frame owner properties have changed.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,8 @@ void ResourceLoadObserverForFrame::DidReceiveResponse(
resource_request.Url(),
String::FromUTF8(resource_request.HttpMethod().Utf8()),
String::FromUTF8(response.MimeType().Utf8()),
resource_request.GetRequestDestination());
resource_request.GetRequestDestination(),
response.RequestIncludeCredentials());
}

// Note: probe::WillSendRequest needs to precede before this probe method.
Expand Down
3 changes: 2 additions & 1 deletion blink/renderer/core/testing/fake_local_frame_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ void FakeLocalFrameHost::DidLoadResourceFromMemoryCache(
const KURL& url,
const WTF::String& http_method,
const WTF::String& mime_type,
network::mojom::blink::RequestDestination request_destination) {}
network::mojom::blink::RequestDestination request_destination,
bool include_credentials) {}

void FakeLocalFrameHost::DidChangeFrameOwnerProperties(
const blink::FrameToken& child_frame_token,
Expand Down
3 changes: 2 additions & 1 deletion blink/renderer/core/testing/fake_local_frame_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ class FakeLocalFrameHost : public mojom::blink::LocalFrameHost {
const KURL& url,
const WTF::String& http_method,
const WTF::String& mime_type,
network::mojom::blink::RequestDestination request_destination) override;
network::mojom::blink::RequestDestination request_destination,
bool include_credentials) override;
void DidChangeFrameOwnerProperties(
const blink::FrameToken& child_frame_token,
mojom::blink::FrameOwnerPropertiesPtr frame_owner_properties) override;
Expand Down
5 changes: 5 additions & 0 deletions blink/web_tests/VirtualTestSuites
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,11 @@
"bases": [],
"args": ["--force-color-profile=scrgb-linear"]
},
{
"prefix": "split-cache-by-include-credentials",
"bases": ["external/wpt/fetch/http-cache"],
"args": ["--enable-features=SplitCacheByIncludeCredentials"]
},
{
"prefix": "split-http-cache",
"bases": ["external/wpt/fetch/http-cache",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# virtual/split-cache-by-include-credentials/

This directory is for tests when the HTTP cache is partitioned by the
"includeCredentials" attribute of the fetch specification. Test are run with:
`--enable-features=SplitCacheByIncludeCredentials`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This is a testharness.js-based test.
PASS same-origin: 2xAnonymous, 2xCredentialled, 1xAnonymous
PASS same-origin: 2xCredentialled, 2xAnonymous, 1xCredentialled
Harness: the test ran to completion.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This is a testharness.js-based test.
PASS same-origin: 2xAnonymous, 2xCredentialled, 1xAnonymous
PASS same-origin: 2xCredentialled, 2xAnonymous, 1xCredentialled
Harness: the test ran to completion.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This is a testharness.js-based test.
PASS same-origin: 2xAnonymous, 2xCredentialled, 1xAnonymous
PASS same-origin: 2xCredentialled, 2xAnonymous, 1xCredentialled
Harness: the test ran to completion.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This is a testharness.js-based test.
PASS same-origin: 2xAnonymous, 2xCredentialled, 1xAnonymous
PASS same-origin: 2xCredentialled, 2xAnonymous, 1xCredentialled
Harness: the test ran to completion.

0 comments on commit 5652727

Please sign in to comment.