Title: [213672] trunk
Revision
213672
Author
y...@yoav.ws
Date
2017-03-09 14:29:39 -0800 (Thu, 09 Mar 2017)

Log Message

[link preload] Double downloads of preloaded CSS
https://bugs.webkit.org/show_bug.cgi?id=169274

Reviewed by Antti Koivisto.

Source/WebCore:

Avoid reloading link preloads in case of a charset mismatch.

Charset mismatch can happen for header based preloads, as they are requested before
the HTML's `<meta charset>` tag is processed. This change makes sure that in those
cases, we modify the resource's encoding setting instead of reloading it.

Test: http/tests/preload/single_download_preload_headers.php

* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::CachedResource): Initialize m_unknownCharset to be the same as the preload flag.
* loader/cache/CachedResource.h:
(WebCore::CachedResource::hasUnknownEncoding):
(WebCore::CachedResource::setHasUnknownEncoding):
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::determineRevalidationPolicy): In case of a charset
mismatch, set the encoding of the Resource instead of reloading it if the charset is unknown.

LayoutTests:

Added tests making sure that header based preloads also trigger a single download,
and that we properly handle multiple charsets for the same preloaded resource.

* http/tests/preload/single_download_preload_headers-expected.txt: Added.
* http/tests/preload/single_download_preload_headers.php: Added.
* http/tests/preload/preload-encoding-expected.txt: Added.
* http/tests/preload/preload-encoding.php: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (213671 => 213672)


--- trunk/LayoutTests/ChangeLog	2017-03-09 22:24:26 UTC (rev 213671)
+++ trunk/LayoutTests/ChangeLog	2017-03-09 22:29:39 UTC (rev 213672)
@@ -1,3 +1,18 @@
+2017-03-09  Yoav Weiss  <y...@yoav.ws>
+
+        [link preload] Double downloads of preloaded CSS
+        https://bugs.webkit.org/show_bug.cgi?id=169274
+
+        Reviewed by Antti Koivisto.
+
+        Added tests making sure that header based preloads also trigger a single download,
+        and that we properly handle multiple charsets for the same preloaded resource.
+
+        * http/tests/preload/single_download_preload_headers-expected.txt: Added.
+        * http/tests/preload/single_download_preload_headers.php: Added.
+        * http/tests/preload/preload-encoding-expected.txt: Added.
+        * http/tests/preload/preload-encoding.php: Added.
+
 2017-03-09  Jiewen Tan  <jiewen_...@apple.com>
 
         Implement PBKDF2 in WebCrypto

Added: trunk/LayoutTests/http/tests/preload/preload-encoding-expected.txt (0 => 213672)


--- trunk/LayoutTests/http/tests/preload/preload-encoding-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/preload-encoding-expected.txt	2017-03-09 22:29:39 UTC (rev 213672)
@@ -0,0 +1,11 @@
+CONSOLE MESSAGE: line 1: SyntaxError: Invalid character '\u8307'
+First load a script with a wrong charset then again with the right one. Second attempt should work and 'scriptSuccess' should be set to true. 'successfullyParsed' will be undefined.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS scriptSuccess is true
+FAIL successfullyParsed should be true (of type boolean). Was undefined (of type undefined).
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/http/tests/preload/preload-encoding.php (0 => 213672)


--- trunk/LayoutTests/http/tests/preload/preload-encoding.php	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/preload-encoding.php	2017-03-09 22:29:39 UTC (rev 213672)
@@ -0,0 +1,31 @@
+<?php
+header("Link: <resources/success.js>; rel=preload; as=script", false);
+?>
+<script src=""
+<script>
+jsTestIsAsync = true;
+
+description("First load a script with a wrong charset then again with the right one. Second attempt should work and 'scriptSuccess' should be set to true. 'successfullyParsed' will be undefined.");
+
+function appendScriptWithCharset(charset, onload)
+{
+    var script = document.createElement("script");
+    script.src = ""
+    script.setAttribute("charset", charset);
+    script._onload_ = onload;
+    script._onerror_ = onload;
+    document.body.appendChild(script);
+}
+
+function test()
+{
+    appendScriptWithCharset("utf-16", function () {
+        appendScriptWithCharset("utf-8", function () {
+            shouldBeTrue("scriptSuccess");
+            finishJSTest();
+        });
+    });
+}
+</script>
+<body _onload_="test()">
+<script src=""

Added: trunk/LayoutTests/http/tests/preload/resources/success.js (0 => 213672)


--- trunk/LayoutTests/http/tests/preload/resources/success.js	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/resources/success.js	2017-03-09 22:29:39 UTC (rev 213672)
@@ -0,0 +1 @@
+scriptSuccess = true;

Added: trunk/LayoutTests/http/tests/preload/single_download_preload_headers-expected.txt (0 => 213672)


--- trunk/LayoutTests/http/tests/preload/single_download_preload_headers-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/single_download_preload_headers-expected.txt	2017-03-09 22:29:39 UTC (rev 213672)
@@ -0,0 +1,5 @@
+CONSOLE MESSAGE: <link rel=preload> must have a valid `as` value
+ 
+
+PASS Makes sure that preloaded resources are not downloaded again when used 
+

Added: trunk/LayoutTests/http/tests/preload/single_download_preload_headers.php (0 => 213672)


--- trunk/LayoutTests/http/tests/preload/single_download_preload_headers.php	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/single_download_preload_headers.php	2017-03-09 22:29:39 UTC (rev 213672)
@@ -0,0 +1,64 @@
+<?php
+header("Link: <http://127.0.0.1:8000/resources/dummy.js>; rel=preload; as=script", false);
+header("LiNk:<http://127.0.0.1:8000/resources/dummy.css>; rel=preload; as=style", false);
+header("Link: <http://127.0.0.1:8000/resources/square100.png>;rel=preload;as=image", false);
+header("Link: <http://127.0.0.1:8000/resources/square100.png?background>;rel=preload;as=image", false);
+header("Link: <http://127.0.0.1:8000/resources/Ahem.woff>; rel=preload; as=font; crossorigin", false);
+header("Link: <http://127.0.0.1:8000/resources/test.mp4>; rel=preload; as=media", false);
+header("Link: <http://127.0.0.1:8000/resources/test.oga>; rel=preload; as=media", false);
+header("link: <http://127.0.0.1:8000/security/resources/captions.vtt>; rel=preload; as=track", false);
+header("Link: <http://127.0.0.1:8000/resources/dummy.xml?foobar>; rel=preload; as=foobar", false);
+header("Link: <http://127.0.0.1:8000/resources/dummy.xml>; crossorigin; rel=preload", false);
+?>
+<!DOCTYPE html>
+<meta charset="utf-8">
+<script src=""
+<script src=""
+<script>
+    var t = async_test('Makes sure that preloaded resources are not downloaded again when used');
+</script>
+<script src=""
+<style>
+    #background {
+        width: 200px;
+        height: 200px;
+        background-image: url(http://127.0.0.1:8000/resources/square100.png?background);
+    }
+    @font-face {
+      font-family:ahem;
+      src: url(http://127.0.0.1:8000/resources/Ahem.woff);
+    }
+    span { font-family: ahem, Arial; }
+</style>
+<link rel="stylesheet" href=""
+<script src=""
+<div id="background"></div>
+<img src=""
+<video src=""
+    <track kind=subtitles src="" srclang=en>
+</video>
+<audio src=""
+<script>
+    var xhr = new XMLHttpRequest();
+    xhr.open("GET", "http://127.0.0.1:8000/resources/dummy.xml");
+    xhr.send();
+
+    window.addEventListener("load", t.step_func(function() {
+        function verifyDownloadNumber(url, number) {
+            assert_equals(performance.getEntriesByName(url).length, number, url);
+        }
+        setTimeout(t.step_func(function() {
+            verifyDownloadNumber("http://127.0.0.1:8000/resources/dummy.js", 1);
+            verifyDownloadNumber("http://127.0.0.1:8000/resources/dummy.css", 1);
+            verifyDownloadNumber("http://127.0.0.1:8000/resources/square100.png", 1);
+            verifyDownloadNumber("http://127.0.0.1:8000/resources/square100.png?background", 1);
+            verifyDownloadNumber("http://127.0.0.1:8000/resources/Ahem.woff", 1);
+            verifyDownloadNumber("http://127.0.0.1:8000/resources/dummy.xml?foobar", 0);
+            verifyDownloadNumber("http://127.0.0.1:8000/security/resources/captions.vtt", 1);
+            // FIXME: XHR should trigger a single download, but it downloads 2 resources instead.
+            verifyDownloadNumber("http://127.0.0.1:8000/resources/dummy.xml", 2);
+            // FIXME: We should verify for video and audio as well, but they seem to (flakily?) trigger multiple partial requests.
+            t.done();
+            }), 100);
+    }));
+</script>

Modified: trunk/Source/WebCore/ChangeLog (213671 => 213672)


--- trunk/Source/WebCore/ChangeLog	2017-03-09 22:24:26 UTC (rev 213671)
+++ trunk/Source/WebCore/ChangeLog	2017-03-09 22:29:39 UTC (rev 213672)
@@ -1,3 +1,27 @@
+2017-03-09  Yoav Weiss  <y...@yoav.ws>
+
+        [link preload] Double downloads of preloaded CSS
+        https://bugs.webkit.org/show_bug.cgi?id=169274
+
+        Reviewed by Antti Koivisto.
+
+        Avoid reloading link preloads in case of a charset mismatch.
+
+        Charset mismatch can happen for header based preloads, as they are requested before
+        the HTML's `<meta charset>` tag is processed. This change makes sure that in those
+        cases, we modify the resource's encoding setting instead of reloading it.
+
+        Test: http/tests/preload/single_download_preload_headers.php
+
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::CachedResource): Initialize m_unknownCharset to be the same as the preload flag.
+        * loader/cache/CachedResource.h:
+        (WebCore::CachedResource::hasUnknownEncoding):
+        (WebCore::CachedResource::setHasUnknownEncoding):
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::determineRevalidationPolicy): In case of a charset
+        mismatch, set the encoding of the Resource instead of reloading it if the charset is unknown.
+
 2017-03-09  Jiewen Tan  <jiewen_...@apple.com>
 
         Implement PBKDF2 in WebCrypto

Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (213671 => 213672)


--- trunk/Source/WebCore/loader/cache/CachedResource.cpp	2017-03-09 22:24:26 UTC (rev 213671)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp	2017-03-09 22:29:39 UTC (rev 213672)
@@ -123,6 +123,7 @@
     , m_origin(request.releaseOrigin())
     , m_initiatorName(request.initiatorName())
     , m_isLinkPreload(request.isLinkPreload())
+    , m_hasUnknownEncoding(request.isLinkPreload())
     , m_type(type)
 {
     ASSERT(sessionID.isValid());

Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (213671 => 213672)


--- trunk/Source/WebCore/loader/cache/CachedResource.h	2017-03-09 22:24:26 UTC (rev 213671)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h	2017-03-09 22:29:39 UTC (rev 213672)
@@ -232,6 +232,8 @@
     void decreasePreloadCount() { ASSERT(m_preloadCount); --m_preloadCount; }
     bool isLinkPreload() { return m_isLinkPreload; }
     void setLinkPreload() { m_isLinkPreload = true; }
+    bool hasUnknownEncoding() { return m_hasUnknownEncoding; }
+    void setHasUnknownEncoding(bool hasUnknownEncoding) { m_hasUnknownEncoding = hasUnknownEncoding; }
 
     void registerHandle(CachedResourceHandleBase*);
     WEBCORE_EXPORT void unregisterHandle(CachedResourceHandleBase*);
@@ -334,6 +336,7 @@
     bool m_inCache { false };
     bool m_loading { false };
     bool m_isLinkPreload { false };
+    bool m_hasUnknownEncoding { false };
 
     bool m_switchingClientsToRevalidatedResource { false };
 

Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (213671 => 213672)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2017-03-09 22:24:26 UTC (rev 213671)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2017-03-09 22:29:39 UTC (rev 213672)
@@ -922,8 +922,12 @@
         return Reload;
 
     auto* textDecoder = existingResource->textResourceDecoder();
-    if (textDecoder && !textDecoder->hasEqualEncodingForCharset(cachedResourceRequest.charset()))
-        return Reload;
+    if (textDecoder && !textDecoder->hasEqualEncodingForCharset(cachedResourceRequest.charset())) {
+        if (!existingResource->hasUnknownEncoding())
+            return Reload;
+        existingResource->setHasUnknownEncoding(false);
+        existingResource->setEncoding(cachedResourceRequest.charset());
+    }
 
     // FIXME: We should use the same cache policy for all resource types. The raw resource policy is overly strict
     //        while the normal subresource policy is too loose.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to