Fetch: opaque redirect responses have a URL. (issue 1857723002 by mkwst@chromium.org)

[mk...@chromium.org]

Reviewers: horo
CL: https://codereview.chromium.org/1857723002/

Message:
Extracting this from the other CL to reduce the number of moving parts. :) WDYT?

Description:
Fetch: opaque redirect responses have a URL.

Tiny bug fix, extracted from https://codereview.chromium.org/1844053003.
Opaque redirect responses maintain their url list, which is safe to do
because the redirect never actually happens.

R=ho...@chromium.org

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+21, -7 lines):
M third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/fetch.js
M third_party/WebKit/Source/modules/fetch/FetchResponseData.cpp
M third_party/WebKit/Source/modules/fetch/FetchResponseDataTest.cpp


Index: third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/fetch.js
diff --git a/third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/fetch.js b/third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/fetch.js
index d2d188c6acc72789ad215df24bf776f2f9ecc66a..1e9a77232b7510dcd0d513c4bb36f973f8ddfbae 100644
--- a/third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/fetch.js
+++ b/third_party/WebKit/LayoutTests/http/tests/fetch/script-tests/fetch.js
@@ -171,7 +171,7 @@ promise_test(function(t) {
.then(function(response) {
assert_equals(response.status, 0);
assert_equals(response.type, 'opaqueredirect');
- assert_equals(response.url, '');
+ assert_equals(response.url, request.url);
});
}, 'Manual redirect fetch returns opaque redirect response');

Index: third_party/WebKit/Source/modules/fetch/FetchResponseData.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/FetchResponseData.cpp b/third_party/WebKit/Source/modules/fetch/FetchResponseData.cpp
index afeca9ba07615f1f0baca2ead360b542d560e66c..2509844b9e9bf31b22a25092c77052037380b244 100644
--- a/third_party/WebKit/Source/modules/fetch/FetchResponseData.cpp
+++ b/third_party/WebKit/Source/modules/fetch/FetchResponseData.cpp
@@ -116,8 +116,11 @@ FetchResponseData* FetchResponseData::createCORSFilteredResponse()
FetchResponseData* FetchResponseData::createOpaqueFilteredResponse()
{
// "An opaque filtered response is a filtered response whose type is
- // |opaque|, status is 0, status message is the empty byte sequence, header
- // list is an empty list, and body is null."
+ // 'opaque', url list is the empty list, status is 0, status message is the
+ // empty byte sequence, header list is the empty list, body is null, and
+ // cache state is 'none'."
+ //
+ // https://fetch.spec.whatwg.org/#concept-filtered-response-opaque
FetchResponseData* response = new FetchResponseData(OpaqueType, 0, "");
response->m_internalResponse = this;
return response;
@@ -125,11 +128,13 @@ FetchResponseData* FetchResponseData::createOpaqueFilteredResponse()

FetchResponseData* FetchResponseData::createOpaqueRedirectFilteredResponse()
{
- // "An opaque-redirect filtered response is a filtered response whose type
- // is |opaqueredirect|, status is 0, status message is the empty byte
- // sequence, header list is the empty list, body is null, and cache state is
- // |none|.
+ // "An opaque filtered response is a filtered response whose type is
+ // 'opaqueredirect', status is 0, status message is the empty byte sequence,
+ // header list is the empty list, body is null, and cache state is 'none'."
+ //
+ // https://fetch.spec.whatwg.org/#concept-filtered-response-opaque-redirect
FetchResponseData* response = new FetchResponseData(OpaqueRedirectType, 0, "");
+ response->m_url = this->m_url;
response->m_internalResponse = this;
return response;
}
Index: third_party/WebKit/Source/modules/fetch/FetchResponseDataTest.cpp
diff --git a/third_party/WebKit/Source/modules/fetch/FetchResponseDataTest.cpp b/third_party/WebKit/Source/modules/fetch/FetchResponseDataTest.cpp
index b2d5667355a908c81eb3fa572763e7dc2d6a6d27..9bb05cb9a44dba8806114cf43272deb3f89a2f34 100644
--- a/third_party/WebKit/Source/modules/fetch/FetchResponseDataTest.cpp
+++ b/third_party/WebKit/Source/modules/fetch/FetchResponseDataTest.cpp
@@ -143,6 +143,15 @@ TEST_F(FetchResponseDataTest, OpaqueFilter)
EXPECT_FALSE(opaqueResponseData->headerList()->has("cache-control"));
}

+TEST_F(FetchResponseDataTest, OpaqueRedirectFilter)
+{
+ FetchResponseData* internalResponse = createInternalResponse();
+ FetchResponseData* opaqueResponseData = internalResponse->createOpaqueRedirectFilteredResponse();
+
+ EXPECT_EQ(opaqueResponseData->headerList()->size(), 0u);
+ EXPECT_EQ(opaqueResponseData->url(), internalResponse->url());
+}
+
TEST_F(FetchResponseDataTest, OpaqueFilterOnResponseWithAccessControlExposeHeaders)
{
FetchResponseData* internalResponse = createInternalResponse();

[mk...@chromium.org]

Ping?

https://codereview.chromium.org/1857723002/

[ho...@chromium.org]

lgtm



https://codereview.chromium.org/1857723002/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1857723002/20001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1857723002/20001


https://codereview.chromium.org/1857723002/

[commit-bot@chromium.org via codereview.chromium.org]

Committed patchset #2 (id:20001)

https://codereview.chromium.org/1857723002/

[commit-bot@chromium.org via codereview.chromium.org]

Patchset 2 (id:??) landed as
https://crrev.com/88f080c222754f5dd5ad5759efb25ea005ee30ca
Cr-Commit-Position: refs/heads/master@{#385109}

https://codereview.chromium.org/1857723002/