Title: [206206] trunk
Revision
206206
Author
commit-qu...@webkit.org
Date
2016-09-21 02:56:16 -0700 (Wed, 21 Sep 2016)

Log Message

[Fetch] Align Accept header default values with fetch spec
https://bugs.webkit.org/show_bug.cgi?id=162260

Patch by Youenn Fablet <you...@apple.com> on 2016-09-21
Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

Ensuring Accept and Accept-Language user-specific values are going up to the server.

* web-platform-tests/fetch/api/basic/accept-header-expected.txt:
* web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt:
* web-platform-tests/fetch/api/basic/accept-header.js:
(promise_test):

Source/WebCore:

Covered by existing and updated tests.

To start implementing step 1 to 7 of fetch algorithm, this patch updates Accept header handling.

Default values are set according the spec based on resource type.
Some resource types are not defined in the spec and we keep using existing values.

We check if Accept header is already present in the request. If that is the case, no change is done to that header.

If the Accept header is not set, the default value '*/*' is used.
An Accept header is therefore always set at CachedResourceLoader level.

* loader/cache/CachedCSSStyleSheet.cpp:
(WebCore::CachedCSSStyleSheet::CachedCSSStyleSheet): Removing accept initialization.
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::load): Removing accept header setting.
* loader/cache/CachedResource.h:
(WebCore::CachedResource::accept): Deleted.
(WebCore::CachedResource::setAccept): Deleted.
* loader/cache/CachedResourceLoader.cpp:
(WebCore::acceptHeaderValueFromType): helper routine merging fetch spec and existing WebKit accept values.
(WebCore::CachedResourceLoader::prepareFetch): Should implement step 1 to 7 of https://fetch.spec.whatwg.org/#fetching.
(WebCore::CachedResourceLoader::requestResource): Making use of prepareFetch.
* loader/cache/CachedResourceLoader.h:
* loader/cache/CachedSVGDocument.cpp:
(WebCore::CachedSVGDocument::CachedSVGDocument): Removing accept initialization.
* loader/cache/CachedScript.cpp:
(WebCore::CachedScript::CachedScript): Removing accept initialization.
* loader/cache/CachedXSLStyleSheet.cpp:
(WebCore::CachedXSLStyleSheet::CachedXSLStyleSheet): Removing accept initialization.
* platform/network/ResourceRequestBase.cpp:
(WebCore::ResourceRequestBase::hasHTTPHeader): Introduced to check for header presence.
* platform/network/ResourceRequestBase.h:

LayoutTests:

* http/tests/misc/resources/image-checks-for-accept.php: Updated according new image Accept header value.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (206205 => 206206)


--- trunk/LayoutTests/ChangeLog	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/LayoutTests/ChangeLog	2016-09-21 09:56:16 UTC (rev 206206)
@@ -1,3 +1,12 @@
+2016-09-21  Youenn Fablet  <you...@apple.com>
+
+        [Fetch] Align Accept header default values with fetch spec
+        https://bugs.webkit.org/show_bug.cgi?id=162260
+
+        Reviewed by Sam Weinig.
+
+        * http/tests/misc/resources/image-checks-for-accept.php: Updated according new image Accept header value.
+
 2016-09-21  Chris Dumez  <cdu...@apple.com>
 
         Import html/syntax web platform tests

Modified: trunk/LayoutTests/http/tests/misc/resources/image-checks-for-accept.php (206205 => 206206)


--- trunk/LayoutTests/http/tests/misc/resources/image-checks-for-accept.php	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/LayoutTests/http/tests/misc/resources/image-checks-for-accept.php	2016-09-21 09:56:16 UTC (rev 206206)
@@ -1,5 +1,5 @@
 <?php
-    if($_SERVER["HTTP_ACCEPT"] == "*/*" || $_SERVER["HTTP_ACCEPT"] == "image/*" || $_SERVER["HTTP_ACCEPT"] == "image/jpg")
+    if($_SERVER["HTTP_ACCEPT"] == "image/png,image/svg+xml,image/*;q=0.8,*/*;q=0.5")
     {
         header("Content-Type: image/jpg");
         header("Cache-Control: no-store");

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (206205 => 206206)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2016-09-21 09:56:16 UTC (rev 206206)
@@ -1,3 +1,17 @@
+2016-09-21  Youenn Fablet  <you...@apple.com>
+
+        [Fetch] Align Accept header default values with fetch spec
+        https://bugs.webkit.org/show_bug.cgi?id=162260
+
+        Reviewed by Sam Weinig.
+
+        Ensuring Accept and Accept-Language user-specific values are going up to the server.
+
+        * web-platform-tests/fetch/api/basic/accept-header-expected.txt:
+        * web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt:
+        * web-platform-tests/fetch/api/basic/accept-header.js:
+        (promise_test):
+
 2016-09-21  Chris Dumez  <cdu...@apple.com>
 
         Import html/syntax web platform tests

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-expected.txt (206205 => 206206)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-expected.txt	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-expected.txt	2016-09-21 09:56:16 UTC (rev 206206)
@@ -1,3 +1,6 @@
 
 PASS Request through fetch should have 'accept' header with value '*/*' 
+PASS Request through fetch should have 'accept' header with value 'custom/*' 
+PASS Request through fetch should have a 'accept-language' header 
+PASS Request through fetch should have 'accept-language' header with value 'bzh' 
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt (206205 => 206206)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header-worker-expected.txt	2016-09-21 09:56:16 UTC (rev 206206)
@@ -1,3 +1,6 @@
 
 PASS Request through fetch should have 'accept' header with value '*/*' 
+PASS Request through fetch should have 'accept' header with value 'custom/*' 
+PASS Request through fetch should have a 'accept-language' header 
+PASS Request through fetch should have 'accept-language' header with value 'bzh' 
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header.js (206205 => 206206)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header.js	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/basic/accept-header.js	2016-09-21 09:56:16 UTC (rev 206206)
@@ -11,4 +11,28 @@
   });
 }, "Request through fetch should have 'accept' header with value '*/*'");
 
+promise_test(function() {
+  return fetch(RESOURCES_DIR + "inspect-headers.py?headers=Accept", {"headers": [["Accept", "custom/*"]]}).then(function(response) {
+    assert_equals(response.status, 200, "HTTP status is 200");
+    assert_equals(response.type , "basic", "Response's type is basic");
+    assert_equals(response.headers.get("x-request-accept"), "custom/*", "Request has accept header with value 'custom/*'");
+  });
+}, "Request through fetch should have 'accept' header with value 'custom/*'");
+
+promise_test(function() {
+  return fetch(RESOURCES_DIR + "inspect-headers.py?headers=Accept-Language").then(function(response) {
+    assert_equals(response.status, 200, "HTTP status is 200");
+    assert_equals(response.type , "basic", "Response's type is basic");
+    assert_true(response.headers.has("x-request-accept-language"));
+  });
+}, "Request through fetch should have a 'accept-language' header");
+
+promise_test(function() {
+  return fetch(RESOURCES_DIR + "inspect-headers.py?headers=Accept-Language", {"headers": [["Accept-Language", "bzh"]]}).then(function(response) {
+    assert_equals(response.status, 200, "HTTP status is 200");
+    assert_equals(response.type , "basic", "Response's type is basic");
+    assert_equals(response.headers.get("x-request-accept-language"), "bzh", "Request has accept header with value 'bzh'");
+  });
+}, "Request through fetch should have 'accept-language' header with value 'bzh'");
+
 done();

Modified: trunk/Source/WebCore/ChangeLog (206205 => 206206)


--- trunk/Source/WebCore/ChangeLog	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/ChangeLog	2016-09-21 09:56:16 UTC (rev 206206)
@@ -1,3 +1,44 @@
+2016-09-21  Youenn Fablet  <you...@apple.com>
+
+        [Fetch] Align Accept header default values with fetch spec
+        https://bugs.webkit.org/show_bug.cgi?id=162260
+
+        Reviewed by Sam Weinig.
+
+        Covered by existing and updated tests.
+
+        To start implementing step 1 to 7 of fetch algorithm, this patch updates Accept header handling.
+
+        Default values are set according the spec based on resource type.
+        Some resource types are not defined in the spec and we keep using existing values.
+
+        We check if Accept header is already present in the request. If that is the case, no change is done to that header.
+
+        If the Accept header is not set, the default value '*/*' is used.
+        An Accept header is therefore always set at CachedResourceLoader level.
+
+        * loader/cache/CachedCSSStyleSheet.cpp:
+        (WebCore::CachedCSSStyleSheet::CachedCSSStyleSheet): Removing accept initialization.
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::load): Removing accept header setting.
+        * loader/cache/CachedResource.h:
+        (WebCore::CachedResource::accept): Deleted.
+        (WebCore::CachedResource::setAccept): Deleted.
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::acceptHeaderValueFromType): helper routine merging fetch spec and existing WebKit accept values.
+        (WebCore::CachedResourceLoader::prepareFetch): Should implement step 1 to 7 of https://fetch.spec.whatwg.org/#fetching.
+        (WebCore::CachedResourceLoader::requestResource): Making use of prepareFetch.
+        * loader/cache/CachedResourceLoader.h:
+        * loader/cache/CachedSVGDocument.cpp:
+        (WebCore::CachedSVGDocument::CachedSVGDocument): Removing accept initialization.
+        * loader/cache/CachedScript.cpp:
+        (WebCore::CachedScript::CachedScript): Removing accept initialization.
+        * loader/cache/CachedXSLStyleSheet.cpp:
+        (WebCore::CachedXSLStyleSheet::CachedXSLStyleSheet): Removing accept initialization.
+        * platform/network/ResourceRequestBase.cpp:
+        (WebCore::ResourceRequestBase::hasHTTPHeader): Introduced to check for header presence.
+        * platform/network/ResourceRequestBase.h:
+
 2016-09-21  Jeremy Huddleston Sequoia  <jerem...@apple.com>
 
         [GTK] Fix build failure of ScrollbarThemeGtk with libc++

Modified: trunk/Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp (206205 => 206206)


--- trunk/Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp	2016-09-21 09:56:16 UTC (rev 206206)
@@ -45,9 +45,6 @@
     : CachedResource(WTFMove(request), CSSStyleSheet, sessionID)
     , m_decoder(TextResourceDecoder::create("text/css", request.charset()))
 {
-    // Prefer text/css but accept any type (dell.com serves a stylesheet
-    // as text/html; see <http://bugs.webkit.org/show_bug.cgi?id=11451>).
-    setAccept("text/css,*/*;q=0.1");
 }
 
 CachedCSSStyleSheet::~CachedCSSStyleSheet()

Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (206205 => 206206)


--- trunk/Source/WebCore/loader/cache/CachedResource.cpp	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp	2016-09-21 09:56:16 UTC (rev 206206)
@@ -307,9 +307,6 @@
     }
 #endif
 
-    if (!accept().isEmpty())
-        m_resourceRequest.setHTTPAccept(accept());
-
     if (isCacheValidator()) {
         CachedResource* resourceToRevalidate = m_resourceToRevalidate;
         ASSERT(resourceToRevalidate->canUseCacheValidator());

Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (206205 => 206206)


--- trunk/Source/WebCore/loader/cache/CachedResource.h	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h	2016-09-21 09:56:16 UTC (rev 206206)
@@ -220,11 +220,6 @@
 
     bool isExpired() const;
 
-    // List of acceptable MIME types separated by ",".
-    // A MIME type may contain a wildcard, e.g. "text/*".
-    String accept() const { return m_accept; }
-    void setAccept(const String& accept) { m_accept = accept; }
-
     void cancelLoad();
     bool wasCanceled() const { return m_error.isCancellation(); }
     bool errorOccurred() const { return m_status == LoadError || m_status == DecodeError; }
@@ -319,7 +314,6 @@
 
     HashMap<CachedResourceClient*, std::unique_ptr<Callback>> m_clientsAwaitingCallback;
     SessionID m_sessionID;
-    String m_accept;
     ResourceLoadPriority m_loadPriority;
     std::chrono::system_clock::time_point m_responseTimestamp;
 

Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (206205 => 206206)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2016-09-21 09:56:16 UTC (rev 206206)
@@ -570,6 +570,38 @@
         frame->page()->diagnosticLoggingClient().logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceRequestKey(), description, value, ShouldSample::Yes);
 }
 
+static inline String acceptHeaderValueFromType(CachedResource::Type type)
+{
+    switch (type) {
+    case CachedResource::Type::MainResource:
+        return ASCIILiteral("text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8");
+    case CachedResource::Type::ImageResource:
+        return ASCIILiteral("image/png,image/svg+xml,image/*;q=0.8,*/*;q=0.5");
+    case CachedResource::Type::CSSStyleSheet:
+        return ASCIILiteral("text/css,*/*;q=0.1");
+    case CachedResource::Type::SVGDocumentResource:
+        return ASCIILiteral("image/svg+xml");
+#if ENABLE(XSLT)
+    case CachedResource::Type::XSLStyleSheet:
+        // FIXME: This should accept more general xml formats */*+xml, image/svg+xml for example.
+        return ASCIILiteral("text/xml,application/xml,application/xhtml+xml,text/xsl,application/rss+xml,application/atom+xml");
+#endif
+    default:
+        return ASCIILiteral("*/*");
+    }
+}
+
+void CachedResourceLoader::prepareFetch(CachedResource::Type type, CachedResourceRequest& request)
+{
+    // Implementing step 1 to 7 of https://fetch.spec.whatwg.org/#fetching
+
+    if (!request.resourceRequest().hasHTTPHeader(HTTPHeaderName::Accept))
+        request.mutableResourceRequest().setHTTPHeaderField(HTTPHeaderName::Accept, acceptHeaderValueFromType(type));
+
+    // Accept-Language value is handled in underlying port-specific code.
+    // FIXME: Decide whether to support client hints
+}
+
 CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(CachedResource::Type type, CachedResourceRequest&& request)
 {
     if (Document* document = this->document())
@@ -587,6 +619,8 @@
         return nullptr;
     }
 
+    prepareFetch(type, request);
+
     if (!canRequest(type, url, request.options(), request.forPreload())) {
         RELEASE_LOG_IF_ALLOWED("requestResource: Not allowed to request resource (frame = %p)", frame());
         return nullptr;

Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.h (206205 => 206206)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.h	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.h	2016-09-21 09:56:16 UTC (rev 206206)
@@ -151,6 +151,7 @@
     explicit CachedResourceLoader(DocumentLoader*);
 
     CachedResourceHandle<CachedResource> requestResource(CachedResource::Type, CachedResourceRequest&&);
+    void prepareFetch(CachedResource::Type, CachedResourceRequest&);
     CachedResourceHandle<CachedResource> revalidateResource(CachedResourceRequest&&, CachedResource&);
     CachedResourceHandle<CachedResource> loadResource(CachedResource::Type, CachedResourceRequest&&);
     CachedResourceHandle<CachedResource> requestPreload(CachedResource::Type, CachedResourceRequest&&);

Modified: trunk/Source/WebCore/loader/cache/CachedSVGDocument.cpp (206205 => 206206)


--- trunk/Source/WebCore/loader/cache/CachedSVGDocument.cpp	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/loader/cache/CachedSVGDocument.cpp	2016-09-21 09:56:16 UTC (rev 206206)
@@ -31,7 +31,6 @@
     : CachedResource(WTFMove(request), SVGDocumentResource, sessionID)
     , m_decoder(TextResourceDecoder::create("application/xml"))
 {
-    setAccept("image/svg+xml");
 }
 
 CachedSVGDocument::~CachedSVGDocument()

Modified: trunk/Source/WebCore/loader/cache/CachedScript.cpp (206205 => 206206)


--- trunk/Source/WebCore/loader/cache/CachedScript.cpp	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/loader/cache/CachedScript.cpp	2016-09-21 09:56:16 UTC (rev 206206)
@@ -44,10 +44,6 @@
     : CachedResource(WTFMove(request), Script, sessionID)
     , m_decoder(TextResourceDecoder::create(ASCIILiteral("application/_javascript_"), request.charset()))
 {
-    // It's _javascript_ we want.
-    // But some websites think their scripts are <some wrong mimetype here>
-    // and refuse to serve them if we only accept application/x-_javascript_.
-    setAccept("*/*");
 }
 
 CachedScript::~CachedScript()

Modified: trunk/Source/WebCore/loader/cache/CachedXSLStyleSheet.cpp (206205 => 206206)


--- trunk/Source/WebCore/loader/cache/CachedXSLStyleSheet.cpp	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/loader/cache/CachedXSLStyleSheet.cpp	2016-09-21 09:56:16 UTC (rev 206206)
@@ -40,9 +40,6 @@
     : CachedResource(WTFMove(request), XSLStyleSheet, sessionID)
     , m_decoder(TextResourceDecoder::create("text/xsl"))
 {
-    // It's XML we want.
-    // FIXME: This should accept more general xml formats */*+xml, image/svg+xml for example.
-    setAccept("text/xml, application/xml, application/xhtml+xml, text/xsl, application/rss+xml, application/atom+xml");
 }
 
 CachedXSLStyleSheet::~CachedXSLStyleSheet()

Modified: trunk/Source/WebCore/platform/network/ResourceRequestBase.cpp (206205 => 206206)


--- trunk/Source/WebCore/platform/network/ResourceRequestBase.cpp	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/platform/network/ResourceRequestBase.cpp	2016-09-21 09:56:16 UTC (rev 206206)
@@ -333,6 +333,11 @@
         m_platformRequestUpdated = false;
 }
 
+bool ResourceRequestBase::hasHTTPHeader(HTTPHeaderName name) const
+{
+    return m_httpHeaderFields.contains(name);
+}
+
 String ResourceRequestBase::httpUserAgent() const
 {
     return httpHeaderField(HTTPHeaderName::UserAgent);

Modified: trunk/Source/WebCore/platform/network/ResourceRequestBase.h (206205 => 206206)


--- trunk/Source/WebCore/platform/network/ResourceRequestBase.h	2016-09-21 09:48:51 UTC (rev 206205)
+++ trunk/Source/WebCore/platform/network/ResourceRequestBase.h	2016-09-21 09:56:16 UTC (rev 206206)
@@ -97,6 +97,8 @@
     WEBCORE_EXPORT void setHTTPContentType(const String&);
     void clearHTTPContentType();
 
+    bool hasHTTPHeader(HTTPHeaderName) const;
+
     WEBCORE_EXPORT String httpReferrer() const;
     bool hasHTTPReferrer() const;
     WEBCORE_EXPORT void setHTTPReferrer(const String&);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to