Title: [215229] trunk
Revision
215229
Author
y...@yoav.ws
Date
2017-04-11 08:10:10 -0700 (Tue, 11 Apr 2017)

Log Message

[link preload] Double downloads of preloaded content when it's in MemoryCache
https://bugs.webkit.org/show_bug.cgi?id=170122

Reviewed by Antti Koivisto.

Source/WebCore:

No new tests, but unflaked http/tests/preload/single_download_preload_headers_charset.html.

The test was flaky because it appears as if MemoryCache is not being evicted between runs,
and running multiple iterations of the test resulted in preloaded being taken out of MemoryCache
and not having the unknown encoding flag. In those cases, the result was a double download and
a failed test.

* loader/TextResourceDecoder.cpp:
(WebCore::TextResourceDecoder::setEncoding): Set the m_encodingSet flag.
* loader/TextResourceDecoder.h: Added an m_encodingSet flag initialized to false.
* loader/cache/CachedCSSStyleSheet.cpp:
(WebCore::CachedCSSStyleSheet::setEncoding): Assert that stylesheets don't maintain decoded text.
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::CachedResource): Remove initialization of hasUnknownEncoding flag.
* loader/cache/CachedResource.h:
(WebCore::CachedResource::hasUnknownEncoding): Remove.
(WebCore::CachedResource::setHasUnknownEncoding): Remove.
(WebCore::CachedResource::CachedResource): Remove initialization of hasUnknownEncoding flag.
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::determineRevalidationPolicy): Set the encoding in case it changed.

LayoutTests:

* TestExpectations: Removed flakiness label from the header preload charset test.
* fast/loader/cache-encoding-expected.txt: Changed expectation.
* fast/loader/cache-encoding.html: Modified behavior to stick with the first decoded string.
* http/tests/preload/preload-encoding-expected.txt: Changed expectation.
* http/tests/preload/preload-encoding.html: Modified behavior to stick with the first decoded string.
* imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/script-charset-01-expected.txt: This test refers to the same file
twice and expects different decoding for it each time. This is the behavior that we modified, and therefore the test expectation is changed as well.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (215228 => 215229)


--- trunk/LayoutTests/ChangeLog	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/LayoutTests/ChangeLog	2017-04-11 15:10:10 UTC (rev 215229)
@@ -1,3 +1,18 @@
+2017-04-11  Yoav Weiss  <y...@yoav.ws>
+
+        [link preload] Double downloads of preloaded content when it's in MemoryCache
+        https://bugs.webkit.org/show_bug.cgi?id=170122
+
+        Reviewed by Antti Koivisto.
+
+        * TestExpectations: Removed flakiness label from the header preload charset test.
+        * fast/loader/cache-encoding-expected.txt: Changed expectation.
+        * fast/loader/cache-encoding.html: Modified behavior to stick with the first decoded string.
+        * http/tests/preload/preload-encoding-expected.txt: Changed expectation.
+        * http/tests/preload/preload-encoding.html: Modified behavior to stick with the first decoded string.
+        * imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/script-charset-01-expected.txt: This test refers to the same file
+        twice and expects different decoding for it each time. This is the behavior that we modified, and therefore the test expectation is changed as well.
+
 2017-04-11  Manuel Rego Casasnovas  <r...@igalia.com>
 
         [css-grid] Fix fast/css-grid-layout/grid-simplified-layout-positioned.html

Modified: trunk/LayoutTests/TestExpectations (215228 => 215229)


--- trunk/LayoutTests/TestExpectations	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/LayoutTests/TestExpectations	2017-04-11 15:10:10 UTC (rev 215229)
@@ -1092,8 +1092,6 @@
 
 webkit.org/b/168238 imported/w3c/web-platform-tests/dom/events/EventListener-invoke-legacy.html [ Pass Failure ]
 
-webkit.org/b/170122 http/tests/preload/single_download_preload_headers_charset.php [ Pass Failure ]
-
 ########################################
 ### START OF -disabled tests
 

Modified: trunk/LayoutTests/fast/loader/cache-encoding-expected.txt (215228 => 215229)


--- trunk/LayoutTests/fast/loader/cache-encoding-expected.txt	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/LayoutTests/fast/loader/cache-encoding-expected.txt	2017-04-11 15:10:10 UTC (rev 215229)
@@ -1,4 +1,3 @@
-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".
@@ -5,7 +4,8 @@
 
 
 PASS scriptSuccess is true
-FAIL successfullyParsed should be true (of type boolean). Was undefined (of type undefined).
+PASS scriptSuccess is true
+PASS successfullyParsed is true
 
 TEST COMPLETE
 

Modified: trunk/LayoutTests/fast/loader/cache-encoding.html (215228 => 215229)


--- trunk/LayoutTests/fast/loader/cache-encoding.html	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/LayoutTests/fast/loader/cache-encoding.html	2017-04-11 15:10:10 UTC (rev 215229)
@@ -16,8 +16,9 @@
 
 function test()
 {
-    appendScriptWithCharset("utf-16", function () {
-        appendScriptWithCharset("utf-8", function () {
+    appendScriptWithCharset("utf-8", function () {
+        shouldBeTrue("scriptSuccess");
+        appendScriptWithCharset("utf-16", function () {
             shouldBeTrue("scriptSuccess");
             finishJSTest();
         });

Modified: trunk/LayoutTests/http/tests/preload/preload-encoding-expected.txt (215228 => 215229)


--- trunk/LayoutTests/http/tests/preload/preload-encoding-expected.txt	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/LayoutTests/http/tests/preload/preload-encoding-expected.txt	2017-04-11 15:10:10 UTC (rev 215229)
@@ -1,4 +1,3 @@
-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".
@@ -5,7 +4,8 @@
 
 
 PASS scriptSuccess is true
-FAIL successfullyParsed should be true (of type boolean). Was undefined (of type undefined).
+PASS scriptSuccess is true
+PASS successfullyParsed is true
 
 TEST COMPLETE
 

Modified: trunk/LayoutTests/http/tests/preload/preload-encoding.php (215228 => 215229)


--- trunk/LayoutTests/http/tests/preload/preload-encoding.php	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/LayoutTests/http/tests/preload/preload-encoding.php	2017-04-11 15:10:10 UTC (rev 215229)
@@ -19,8 +19,10 @@
 
 function test()
 {
-    appendScriptWithCharset("utf-16", function () {
-        appendScriptWithCharset("utf-8", function () {
+    appendScriptWithCharset("utf-8", function () {
+        shouldBeTrue("scriptSuccess");
+        scriptSuccess = false;
+        appendScriptWithCharset("utf-16", function () {
             shouldBeTrue("scriptSuccess");
             finishJSTest();
         });

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/script-charset-01-expected.txt (215228 => 215229)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/script-charset-01-expected.txt	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/script-charset-01-expected.txt	2017-04-11 15:10:10 UTC (rev 215229)
@@ -2,7 +2,7 @@
 PASS Script @type: unknown parameters 
 PASS Script @type: unknown parameters 1 
 PASS Script @type: unknown parameters 2 
-PASS Script @type: unknown parameters 3 
+FAIL Script @type: unknown parameters 3 assert_equals: expected "śćążź" but got "\ufffd湿\ufffd"
 PASS Script @type: unknown parameters 4 
 PASS Script @type: unknown parameters 5 
 

Modified: trunk/Source/WebCore/ChangeLog (215228 => 215229)


--- trunk/Source/WebCore/ChangeLog	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/Source/WebCore/ChangeLog	2017-04-11 15:10:10 UTC (rev 215229)
@@ -1,3 +1,31 @@
+2017-04-11  Yoav Weiss  <y...@yoav.ws>
+
+        [link preload] Double downloads of preloaded content when it's in MemoryCache
+        https://bugs.webkit.org/show_bug.cgi?id=170122
+
+        Reviewed by Antti Koivisto.
+
+        No new tests, but unflaked http/tests/preload/single_download_preload_headers_charset.html.
+
+        The test was flaky because it appears as if MemoryCache is not being evicted between runs,
+        and running multiple iterations of the test resulted in preloaded being taken out of MemoryCache
+        and not having the unknown encoding flag. In those cases, the result was a double download and
+        a failed test.
+
+        * loader/TextResourceDecoder.cpp:
+        (WebCore::TextResourceDecoder::setEncoding): Set the m_encodingSet flag.
+        * loader/TextResourceDecoder.h: Added an m_encodingSet flag initialized to false.
+        * loader/cache/CachedCSSStyleSheet.cpp:
+        (WebCore::CachedCSSStyleSheet::setEncoding): Assert that stylesheets don't maintain decoded text.
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::CachedResource): Remove initialization of hasUnknownEncoding flag.
+        * loader/cache/CachedResource.h:
+        (WebCore::CachedResource::hasUnknownEncoding): Remove.
+        (WebCore::CachedResource::setHasUnknownEncoding): Remove.
+        (WebCore::CachedResource::CachedResource): Remove initialization of hasUnknownEncoding flag.
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::determineRevalidationPolicy): Set the encoding in case it changed.
+
 2017-04-11  Miguel Gomez  <mago...@igalia.com>
 
         REGRESSION(r215211): [GTK] Lots of image related tests are timing out, causing the test bot to exit early

Modified: trunk/Source/WebCore/loader/TextResourceDecoder.cpp (215228 => 215229)


--- trunk/Source/WebCore/loader/TextResourceDecoder.cpp	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/Source/WebCore/loader/TextResourceDecoder.cpp	2017-04-11 15:10:10 UTC (rev 215229)
@@ -355,6 +355,7 @@
     else
         m_encoding = encoding;
 
+    m_encodingSet = true;
     m_codec = nullptr;
     m_source = source;
 }

Modified: trunk/Source/WebCore/loader/TextResourceDecoder.h (215228 => 215229)


--- trunk/Source/WebCore/loader/TextResourceDecoder.h	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/Source/WebCore/loader/TextResourceDecoder.h	2017-04-11 15:10:10 UTC (rev 215229)
@@ -68,6 +68,7 @@
    
     void useLenientXMLDecoding() { m_useLenientXMLDecoding = true; }
     bool sawError() const { return m_sawError; }
+    bool encodingSet() const { return m_encodingSet; }
 
 private:
     WEBCORE_EXPORT TextResourceDecoder(const String& mimeType, const TextEncoding& defaultEncoding, bool usesEncodingDetector);
@@ -95,6 +96,7 @@
     bool m_useLenientXMLDecoding; // Don't stop on XML decoding errors.
     bool m_sawError;
     bool m_usesEncodingDetector;
+    bool m_encodingSet { false };
 
     std::unique_ptr<HTMLMetaCharsetParser> m_charsetParser;
 };

Modified: trunk/Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp (215228 => 215229)


--- trunk/Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp	2017-04-11 15:10:10 UTC (rev 215229)
@@ -64,6 +64,7 @@
 
 void CachedCSSStyleSheet::setEncoding(const String& chs)
 {
+    ASSERT(m_decodedSheetText.isNull());
     m_decoder->setEncoding(chs, TextResourceDecoder::EncodingFromHTTPHeader);
 }
 

Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (215228 => 215229)


--- trunk/Source/WebCore/loader/cache/CachedResource.cpp	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp	2017-04-11 15:10:10 UTC (rev 215229)
@@ -123,7 +123,6 @@
     , 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 (215228 => 215229)


--- trunk/Source/WebCore/loader/cache/CachedResource.h	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h	2017-04-11 15:10:10 UTC (rev 215229)
@@ -232,8 +232,6 @@
     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*);
@@ -336,7 +334,6 @@
     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 (215228 => 215229)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2017-04-11 14:56:21 UTC (rev 215228)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2017-04-11 15:10:10 UTC (rev 215229)
@@ -935,12 +935,8 @@
         return Reload;
 
     auto* textDecoder = existingResource->textResourceDecoder();
-    if (textDecoder && !textDecoder->hasEqualEncodingForCharset(cachedResourceRequest.charset())) {
-        if (!existingResource->hasUnknownEncoding())
-            return Reload;
-        existingResource->setHasUnknownEncoding(false);
+    if (textDecoder && !textDecoder->hasEqualEncodingForCharset(cachedResourceRequest.charset()) && !textDecoder->encodingSet())
         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