Title: [214472] trunk
Revision
214472
Author
y...@yoav.ws
Date
2017-03-28 04:56:52 -0700 (Tue, 28 Mar 2017)

Log Message

Add a warning for unused link preloads.
https://bugs.webkit.org/show_bug.cgi?id=165670

Reviewed by Youenn Fablet.

Source/WebCore:

Tests: http/tests/preload/single_download_preload_headers_charset.php
       http/tests/preload/unused_preload_warning.html

* dom/Document.cpp:
(WebCore::Document::prepareForDestruction): Stop the timer once the document is destructed.
* loader/LinkPreloadResourceClients.h: Add shouldMarkAsReferenced overides for the LinkPreloadResourceClient classes.
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::addClientToSet): Make sure LinkPreloadResourceClients don't set resource to be referenced.
* loader/cache/CachedResourceClient.h:
(WebCore::CachedResourceClient::shouldMarkAsReferenced): Make sure that ResourceClients mark preloads as referenced by default.
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::CachedResourceLoader): Initialize timer.
(WebCore::CachedResourceLoader::~CachedResourceLoader): Stop timer.
(WebCore::CachedResourceLoader::documentDidFinishLoadEvent): Trigger a timer if preloads weren't cleared at load time.
(WebCore::CachedResourceLoader::stopUnusedPreloadsTimer): Stop the timer.
(WebCore::CachedResourceLoader::warnUnusedPreloads): Iterate over m_preloads and issue a warning for non-referenced preloads.
* loader/cache/CachedResourceLoader.h:
* page/DOMWindow.cpp:
(WebCore::DOMWindow::willDetachDocumentFromFrame): Clear Resource Timing buffer when document detaches, to avoid test flakiness.

LayoutTests:

* TestExpectations: Added a "Failure Pass" for the flaky charset header test.
* http/tests/preload/download_resources-expected.txt:
* http/tests/preload/download_resources.html: Added references to preloaded resources.
* http/tests/preload/onerror_event-expected.txt:
* http/tests/preload/onerror_event.html: Added references to preloaded resources.
* http/tests/preload/onload_event-expected.txt:
* http/tests/preload/onload_event.html: Added references to preloaded resources.
* http/tests/preload/single_download_preload.html: Deflaked.
* http/tests/preload/single_download_preload_headers.php: Removed the charset to avoid double download bug.
* http/tests/preload/single_download_preload_headers_charset-expected.txt: Added.
* http/tests/preload/single_download_preload_headers_charset.php: Flaky test showing the double download bug when charset is declared.
* http/tests/preload/unused_preload_warning-expected.txt: Added.
* http/tests/preload/unused_preload_warning.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (214471 => 214472)


--- trunk/LayoutTests/ChangeLog	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/ChangeLog	2017-03-28 11:56:52 UTC (rev 214472)
@@ -1,3 +1,24 @@
+2017-03-28  Yoav Weiss  <y...@yoav.ws>
+
+        Add a warning for unused link preloads.
+        https://bugs.webkit.org/show_bug.cgi?id=165670
+
+        Reviewed by Youenn Fablet.
+
+        * TestExpectations: Added a "Failure Pass" for the flaky charset header test.
+        * http/tests/preload/download_resources-expected.txt:
+        * http/tests/preload/download_resources.html: Added references to preloaded resources.
+        * http/tests/preload/onerror_event-expected.txt:
+        * http/tests/preload/onerror_event.html: Added references to preloaded resources.
+        * http/tests/preload/onload_event-expected.txt:
+        * http/tests/preload/onload_event.html: Added references to preloaded resources.
+        * http/tests/preload/single_download_preload.html: Deflaked.
+        * http/tests/preload/single_download_preload_headers.php: Removed the charset to avoid double download bug.
+        * http/tests/preload/single_download_preload_headers_charset-expected.txt: Added.
+        * http/tests/preload/single_download_preload_headers_charset.php: Flaky test showing the double download bug when charset is declared.
+        * http/tests/preload/unused_preload_warning-expected.txt: Added.
+        * http/tests/preload/unused_preload_warning.html: Added.
+
 2017-03-28  Antoine Quint  <grao...@apple.com>
 
         REGRESSION: Double-clicking the captions button while the captions popover is open prevents the popover from being opened again

Modified: trunk/LayoutTests/TestExpectations (214471 => 214472)


--- trunk/LayoutTests/TestExpectations	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/TestExpectations	2017-03-28 11:56:52 UTC (rev 214472)
@@ -1086,6 +1086,8 @@
 
 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/http/tests/preload/download_resources-expected.txt (214471 => 214472)


--- trunk/LayoutTests/http/tests/preload/download_resources-expected.txt	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/http/tests/preload/download_resources-expected.txt	2017-03-28 11:56:52 UTC (rev 214472)
@@ -1,13 +1,5 @@
-CONSOLE MESSAGE: line 11: <link rel=preload> must have a valid `as` value
-PASS internals.isPreloaded('../resources/dummy.js'); is true
-PASS internals.isPreloaded('../resources/dummy.css'); is true
-PASS internals.isPreloaded('../resources/square.png'); is true
-PASS internals.isPreloaded('../resources/Ahem.ttf'); is true
-PASS internals.isPreloaded('../resources/test.mp4'); is true
-PASS internals.isPreloaded('../security/resources/captions.vtt'); is true
-PASS internals.isPreloaded('../resources/dummy.xml?badvalue'); is false
-PASS internals.isPreloaded('../resources/dummy.xml'); is true
-PASS successfullyParsed is true
+CONSOLE MESSAGE: line 13: <link rel=preload> must have a valid `as` value
+This test makes sure that link preload preloads resources
 
-TEST COMPLETE
+PASS Makes sure that preloaded resources are downloaded 
 

Modified: trunk/LayoutTests/http/tests/preload/download_resources.html (214471 => 214472)


--- trunk/LayoutTests/http/tests/preload/download_resources.html	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/http/tests/preload/download_resources.html	2017-03-28 11:56:52 UTC (rev 214472)
@@ -1,27 +1,42 @@
 <!DOCTYPE html>
-<html>
-<head>
-<script src=""
+<script src=""
+<script src=""
+<script>
+    var t = async_test('Makes sure that preloaded resources are downloaded');
+</script>
 <link rel=preload href="" as=script>
 <link rel=preload href="" as=style>
-<link rel=preload href="" as=image>
-<link rel=preload href="" as=font crossorigin>
+<link rel=preload href="" as=image>
+<link rel=preload href="" as=font crossorigin>
 <link rel=preload href="" as=media>
 <link rel=preload href="" as=track>
 <link rel=preload href="" as=foobarxmlthing>
 <link rel=preload href=""
-<script src=""
-</head>
-<body>
+<script src=""
 <script>
-    if (window.testRunner)
-        testRunner.dumpAsText();
-    shouldBeTrue("internals.isPreloaded('../resources/dummy.js');");
-    shouldBeTrue("internals.isPreloaded('../resources/dummy.css');");
-    shouldBeTrue("internals.isPreloaded('../resources/square.png');");
-    shouldBeTrue("internals.isPreloaded('../resources/Ahem.ttf');");
-    shouldBeTrue("internals.isPreloaded('../resources/test.mp4');");
-    shouldBeTrue("internals.isPreloaded('../security/resources/captions.vtt');");
-    shouldBeFalse("internals.isPreloaded('../resources/dummy.xml?badvalue');");
-    shouldBeTrue("internals.isPreloaded('../resources/dummy.xml');");
+    assert_true(internals.isPreloaded("../resources/dummy.js"));
+    assert_true(internals.isPreloaded("../resources/dummy.css"));
+    assert_true(internals.isPreloaded("../resources/square100.png"));
+    // FIXME: RT doesn't show downloads for the resources below. Need to investigate why.
+    assert_true(internals.isPreloaded("../resources/Ahem.woff"));
+    assert_true(internals.isPreloaded("../resources/test.mp4"));
+    assert_true(internals.isPreloaded("../security/resources/captions.vtt"));
+
+    assert_false(internals.isPreloaded("../resources/dummy.xml?badvalue"));
+    assert_true(internals.isPreloaded("../resources/dummy.xml"));
+    document.write('<script src="" + 'ipt>' +
+                   '<link rel=stylesheet href="" +
+                   '<img src="" +
+                   '<video><source src="" +
+                   '<track kind=subtitles src="" srclang=en>' +
+                   '</video>' +
+                   '<style>' +
+                   '    @font-face { font-family:ahem; src: url(../resources/Ahem.woff); }' +
+                   '    span { font-family: ahem, Arial; }' +
+                   '</style>' +
+                   '<span>This test makes sure that link preload preloads resources</span>');
+    var xhr = new XMLHttpRequest();
+    xhr.open("GET", "../resources/dummy.xml");
+    xhr.send();
+    t.done();
 </script>

Modified: trunk/LayoutTests/http/tests/preload/onerror_event-expected.txt (214471 => 214472)


--- trunk/LayoutTests/http/tests/preload/onerror_event-expected.txt	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/http/tests/preload/onerror_event-expected.txt	2017-03-28 11:56:52 UTC (rev 214472)
@@ -1,12 +1,5 @@
-CONSOLE MESSAGE: line 27: <link rel=preload> must have a valid `as` value
-PASS successfullyParsed is true
+CONSOLE MESSAGE: line 25: <link rel=preload> must have a valid `as` value
+This test makes sure that link preload triggers error events for non-existing resources.
 
-TEST COMPLETE
-PASS styleFailed is true
-PASS scriptFailed is true
-PASS imageFailed is true
-PASS fontFailed is true
-PASS trackFailed is true
-PASS gibrishFailed is true
-PASS noTypeFailed is true
+PASS Makes sure that preloaded resources trigger onerror 
 

Modified: trunk/LayoutTests/http/tests/preload/onerror_event.html (214471 => 214472)


--- trunk/LayoutTests/http/tests/preload/onerror_event.html	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/http/tests/preload/onerror_event.html	2017-03-28 11:56:52 UTC (rev 214472)
@@ -1,11 +1,9 @@
 <!DOCTYPE html>
+<script src=""
+<script src=""
 <script>
-    if (window.testRunner) {
-        testRunner.dumpAsText()
-        testRunner.waitUntilDone();
-    }
+    var t = async_test('Makes sure that preloaded resources trigger onerror');
 </script>
-<script src=""
 <script>
     var scriptFailed = false;
     var styleFailed = false;
@@ -27,20 +25,36 @@
 <link rel=preload href="" as=foobarxmlthing _onerror_="count();gibrishFailed = true;" _onload_="count();">
 <link rel=preload href="" _onerror_="count();noTypeFailed = true;" _onload_="count();">
 <script>
+    document.write('<script src="" + 'ipt>' +
+                   '<link rel=stylesheet href="" +
+                   '<img src="" +
+                   '<video><source src="" +
+                   '<track kind=subtitles src="" srclang=en>' +
+                   '</video>' +
+                   '<style>' +
+                   '    @font-face { font-family:ahem; src: url(../../non-existent/Ahem.ttf); }' +
+                   '    span { font-family: ahem, Arial; }' +
+                   '</style>' +
+                   '<span>This test makes sure that link preload triggers error events for non-existing resources.</span>');
+    var xhr = new XMLHttpRequest();
+    xhr.open("GET", "../non-existent/dummy.xml");
+    xhr.send();
+    var xhr2 = new XMLHttpRequest();
+    xhr2.open("GET", "http://127.0.0.1:9999/non-existent/dummy.xml");
+    xhr2.send();
     function test() {
-        shouldBeTrue("styleFailed");
-        shouldBeTrue("scriptFailed");
-        shouldBeTrue("imageFailed");
-        shouldBeTrue("fontFailed");
-        shouldBeTrue("trackFailed");
-        shouldBeTrue("gibrishFailed");
-        shouldBeTrue("noTypeFailed");
-        if (window.testRunner)
-            testRunner.notifyDone();
+        assert_true(styleFailed);
+        assert_true(scriptFailed);
+        assert_true(imageFailed);
+        assert_true(fontFailed);
+        assert_true(trackFailed);
+        assert_true(gibrishFailed);
+        assert_true(noTypeFailed);
+        t.done();
     };
-    setInterval(function() {
+    setInterval(t.step_func(function() {
         if (window.counter >= 7)
             test();
-    }, 100);
+    }, 100));
 </script>
 

Modified: trunk/LayoutTests/http/tests/preload/onload_event-expected.txt (214471 => 214472)


--- trunk/LayoutTests/http/tests/preload/onload_event-expected.txt	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/http/tests/preload/onload_event-expected.txt	2017-03-28 11:56:52 UTC (rev 214472)
@@ -15,4 +15,4 @@
 PASS xsltErrored is true
 PASS noTypeLoaded is true
 PASS emptyTypeLoaded is true
-
+This test makes sure that link preload events are fired

Modified: trunk/LayoutTests/http/tests/preload/onload_event.html (214471 => 214472)


--- trunk/LayoutTests/http/tests/preload/onload_event.html	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/http/tests/preload/onload_event.html	2017-03-28 11:56:52 UTC (rev 214472)
@@ -29,7 +29,7 @@
 <link rel=preload href="" as=image _onload_="count(); imageLoaded = true;" _onerror_="count()">
 <link rel=preload href="" as=font crossorigin _onload_="count(); fontLoaded = true;" _onerror_="count()">
 <link rel=preload href="" as=media _onload_="count(); mediaLoaded = true;" _onerror_="count()">
-<link rel=preload href="" as=track _onload_="count(); trackLoaded = true;" _onerror_="count()">
+<link rel=preload href="" as=track _onload_="count(); trackLoaded = true;" _onerror_="count();">
 <link rel=preload href="" as=foobarxmlthing _onload_="count(); gibberishLoaded = true;" _onerror_="count(); gibberishErrored = true;">
 <link rel=preload href="" as=xslt _onload_="count(); xsltLoaded = true;" _onerror_="count(); xsltErrored = true;">
 <link rel=preload href="" _onload_="count(); noTypeLoaded = true;" _onerror_="count()">
@@ -51,7 +51,24 @@
         if (window.testRunner)
             testRunner.notifyDone();
     }
-     addEventListener("load", function(){
+    document.write('<script src="" + 'ipt>' +
+                   '<link rel=stylesheet href="" +
+                   '<img src="" +
+                   '<video><source src="" +
+                   '<track kind=subtitles src="" srclang=en>' +
+                   '</video>' +
+                   '<style>' +
+                   '    @font-face { font-family:ahem; src: url(../../w3c/webperf/resources/Ahem.ttf); }' +
+                   '    span { font-family: ahem, Arial; }' +
+                   '</style>' +
+                   '<span>This test makes sure that link preload events are fired</span>');
+    var xhr = new XMLHttpRequest();
+    xhr.open("GET", "../resources/dummy.xml");
+    xhr.send();
+    var xhr2 = new XMLHttpRequest();
+    xhr2.open("GET", "../resources/dummy.xml?badvalue");
+    xhr2.send();
+    addEventListener("load", function(){
         setInterval(function() {
             if (window.counter >= 10)
                 test();

Modified: trunk/LayoutTests/http/tests/preload/single_download_preload.html (214471 => 214472)


--- trunk/LayoutTests/http/tests/preload/single_download_preload.html	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/http/tests/preload/single_download_preload.html	2017-03-28 11:56:52 UTC (rev 214472)
@@ -55,6 +55,12 @@
             // 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.
+            var xhr = new XMLHttpRequest();
+            xhr.open("GET", "../resources/dummy.xml");
+            xhr.send();
+            var xhr2 = new XMLHttpRequest();
+            xhr2.open("GET", "../resources/dummy.xml?badvalue");
+            xhr2.send();
             t.done();
             }), 100);
     }));

Modified: trunk/LayoutTests/http/tests/preload/single_download_preload_headers.php (214471 => 214472)


--- trunk/LayoutTests/http/tests/preload/single_download_preload_headers.php	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/LayoutTests/http/tests/preload/single_download_preload_headers.php	2017-03-28 11:56:52 UTC (rev 214472)
@@ -11,7 +11,6 @@
 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>

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


--- trunk/LayoutTests/http/tests/preload/single_download_preload_headers_charset-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/single_download_preload_headers_charset-expected.txt	2017-03-28 11:56:52 UTC (rev 214472)
@@ -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 
+

Copied: trunk/LayoutTests/http/tests/preload/single_download_preload_headers_charset.php (from rev 214471, trunk/LayoutTests/http/tests/preload/single_download_preload_headers.php) (0 => 214472)


--- trunk/LayoutTests/http/tests/preload/single_download_preload_headers_charset.php	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/single_download_preload_headers_charset.php	2017-03-28 11:56:52 UTC (rev 214472)
@@ -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>

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


--- trunk/LayoutTests/http/tests/preload/unused_preload_warning-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/unused_preload_warning-expected.txt	2017-03-28 11:56:52 UTC (rev 214472)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: The resource http://127.0.0.1:8000/resources/square100.png?name=value was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it wasn't preloaded for nothing.
+ This test verifies that unused preload resources (and only unused ones) issue a warning.

Added: trunk/LayoutTests/http/tests/preload/unused_preload_warning.html (0 => 214472)


--- trunk/LayoutTests/http/tests/preload/unused_preload_warning.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/preload/unused_preload_warning.html	2017-03-28 11:56:52 UTC (rev 214472)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+    window._onload_ = function() {
+        window.setTimeout(function() {
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, 4000);
+    };
+</script>
+<link rel="preload" as="image" href=""
+<link rel="preload" as="image" href=""
+<img src=""
+This test verifies that unused preload resources (and only unused ones) issue a warning.

Modified: trunk/Source/WebCore/ChangeLog (214471 => 214472)


--- trunk/Source/WebCore/ChangeLog	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/Source/WebCore/ChangeLog	2017-03-28 11:56:52 UTC (rev 214472)
@@ -1,3 +1,30 @@
+2017-03-28  Yoav Weiss  <y...@yoav.ws>
+
+        Add a warning for unused link preloads.
+        https://bugs.webkit.org/show_bug.cgi?id=165670
+
+        Reviewed by Youenn Fablet.
+
+        Tests: http/tests/preload/single_download_preload_headers_charset.php
+               http/tests/preload/unused_preload_warning.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::prepareForDestruction): Stop the timer once the document is destructed.
+        * loader/LinkPreloadResourceClients.h: Add shouldMarkAsReferenced overides for the LinkPreloadResourceClient classes.
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::addClientToSet): Make sure LinkPreloadResourceClients don't set resource to be referenced.
+        * loader/cache/CachedResourceClient.h:
+        (WebCore::CachedResourceClient::shouldMarkAsReferenced): Make sure that ResourceClients mark preloads as referenced by default.
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::CachedResourceLoader): Initialize timer.
+        (WebCore::CachedResourceLoader::~CachedResourceLoader): Stop timer.
+        (WebCore::CachedResourceLoader::documentDidFinishLoadEvent): Trigger a timer if preloads weren't cleared at load time.
+        (WebCore::CachedResourceLoader::stopUnusedPreloadsTimer): Stop the timer.
+        (WebCore::CachedResourceLoader::warnUnusedPreloads): Iterate over m_preloads and issue a warning for non-referenced preloads.
+        * loader/cache/CachedResourceLoader.h:
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::willDetachDocumentFromFrame): Clear Resource Timing buffer when document detaches, to avoid test flakiness.
+
 2017-03-28  Antoine Quint  <grao...@apple.com>
 
         REGRESSION: Double-clicking the captions button while the captions popover is open prevents the popover from being opened again

Modified: trunk/Source/WebCore/dom/Document.cpp (214471 => 214472)


--- trunk/Source/WebCore/dom/Document.cpp	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/Source/WebCore/dom/Document.cpp	2017-03-28 11:56:52 UTC (rev 214472)
@@ -2265,6 +2265,8 @@
     }
 #endif
 
+    m_cachedResourceLoader->stopUnusedPreloadsTimer();
+
     detachFromFrame();
 
     m_hasPreparedForDestruction = true;

Modified: trunk/Source/WebCore/loader/LinkPreloadResourceClients.h (214471 => 214472)


--- trunk/Source/WebCore/loader/LinkPreloadResourceClients.h	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/Source/WebCore/loader/LinkPreloadResourceClients.h	2017-03-28 11:56:52 UTC (rev 214472)
@@ -88,6 +88,7 @@
     void notifyFinished(CachedResource& resource) override { triggerEvents(resource); }
 
     void clear() override { clearResource(*this); }
+    bool shouldMarkAsReferenced() const override { return false; }
 
 private:
     LinkPreloadScriptResourceClient(LinkLoader& loader, CachedScript& resource)
@@ -114,6 +115,7 @@
     }
 
     void clear() override { clearResource(*this); }
+    bool shouldMarkAsReferenced() const override { return false; }
 
 private:
     LinkPreloadStyleResourceClient(LinkLoader& loader, CachedCSSStyleSheet& resource)
@@ -135,6 +137,7 @@
     void notifyFinished(CachedResource& resource) override { triggerEvents(resource); }
 
     void clear() override { clearResource(*this); }
+    bool shouldMarkAsReferenced() const override { return false; }
 
 private:
     LinkPreloadImageResourceClient(LinkLoader& loader, CachedImage& resource)
@@ -160,6 +163,7 @@
     }
 
     void clear() override { clearResource(*this); }
+    bool shouldMarkAsReferenced() const override { return false; }
 
 private:
     LinkPreloadFontResourceClient(LinkLoader& loader, CachedFont& resource)
@@ -181,6 +185,7 @@
     void notifyFinished(CachedResource& resource) override { triggerEvents(resource); }
 
     void clear() override { clearResource(*this); }
+    bool shouldMarkAsReferenced() const override { return false; }
 
 private:
     LinkPreloadRawResourceClient(LinkLoader& loader, CachedRawResource& resource)

Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (214471 => 214472)


--- trunk/Source/WebCore/loader/cache/CachedResource.cpp	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp	2017-03-28 11:56:52 UTC (rev 214472)
@@ -465,7 +465,7 @@
 
 bool CachedResource::addClientToSet(CachedResourceClient& client)
 {
-    if (m_preloadResult == PreloadNotReferenced) {
+    if (m_preloadResult == PreloadNotReferenced && client.shouldMarkAsReferenced()) {
         if (isLoaded())
             m_preloadResult = PreloadReferencedWhileComplete;
         else if (m_requestedFromNetworkingLayer)

Modified: trunk/Source/WebCore/loader/cache/CachedResourceClient.h (214471 => 214472)


--- trunk/Source/WebCore/loader/cache/CachedResourceClient.h	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/Source/WebCore/loader/cache/CachedResourceClient.h	2017-03-28 11:56:52 UTC (rev 214472)
@@ -45,6 +45,7 @@
 
     static CachedResourceClientType expectedType() { return BaseResourceType; }
     virtual CachedResourceClientType resourceClientType() const { return expectedType(); }
+    virtual bool shouldMarkAsReferenced() const { return true; }
 
 protected:
     CachedResourceClient() { }

Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (214471 => 214472)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2017-03-28 11:56:52 UTC (rev 214472)
@@ -85,6 +85,9 @@
 
 namespace WebCore {
 
+// Timeout for link preloads to be used after window.onload
+static const int unusedPreloadTimeoutInSeconds = 3;
+
 static CachedResource* createResource(CachedResource::Type type, CachedResourceRequest&& request, SessionID sessionID)
 {
     switch (type) {
@@ -129,6 +132,7 @@
     : m_document(nullptr)
     , m_documentLoader(documentLoader)
     , m_requestCount(0)
+    , m_unusedPreloadsTimer(*this, &CachedResourceLoader::warnUnusedPreloads)
     , m_garbageCollectDocumentResourcesTimer(*this, &CachedResourceLoader::garbageCollectDocumentResources)
     , m_autoLoadImages(true)
     , m_imagesEnabled(true)
@@ -147,6 +151,7 @@
 
     // Make sure no requests still point to this CachedResourceLoader
     ASSERT(m_requestCount == 0);
+    m_unusedPreloadsTimer.stop();
 }
 
 CachedResource* CachedResourceLoader::cachedResource(const String& resourceURL) const
@@ -821,8 +826,18 @@
 void CachedResourceLoader::documentDidFinishLoadEvent()
 {
     m_validatedURLs.clear();
+
+    // If m_preloads is not empty here, it's full of link preloads,
+    // as speculative preloads were cleared at DCL.
+    if (m_preloads && m_preloads->size() && !m_unusedPreloadsTimer.isActive())
+        m_unusedPreloadsTimer.startOneShot(unusedPreloadTimeoutInSeconds);
 }
 
+void CachedResourceLoader::stopUnusedPreloadsTimer()
+{
+    m_unusedPreloadsTimer.stop();
+}
+
 CachedResourceHandle<CachedResource> CachedResourceLoader::revalidateResource(CachedResourceRequest&& request, CachedResource& resource)
 {
     ASSERT(resource.inCache());
@@ -1228,6 +1243,19 @@
     return resource;
 }
 
+void CachedResourceLoader::warnUnusedPreloads()
+{
+    if (!m_preloads)
+        return;
+    for (const auto& resource : *m_preloads) {
+        if (resource && resource->isLinkPreload() && resource->preloadResult() == CachedResource::PreloadNotReferenced && document()) {
+            document()->addConsoleMessage(MessageSource::Other, MessageLevel::Warning,
+                "The resource " + resource->url().string() +
+                " was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it wasn't preloaded for nothing.");
+        }
+    }
+}
+
 bool CachedResourceLoader::isPreloaded(const String& urlString) const
 {
     const URL& url = ""

Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.h (214471 => 214472)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.h	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.h	2017-03-28 11:56:52 UTC (rev 214472)
@@ -133,6 +133,8 @@
     void clearPreloads(ClearPreloadsMode);
     CachedResourceHandle<CachedResource> preload(CachedResource::Type, CachedResourceRequest&&);
     void printPreloadStats();
+    void warnUnusedPreloads();
+    void stopUnusedPreloadsTimer();
 
     bool updateRequestAfterRedirection(CachedResource::Type, ResourceRequest&, const ResourceLoaderOptions&);
 
@@ -188,6 +190,7 @@
     int m_requestCount;
     
     std::unique_ptr<ListHashSet<CachedResource*>> m_preloads;
+    Timer m_unusedPreloadsTimer;
 
     Timer m_garbageCollectDocumentResourcesTimer;
 

Modified: trunk/Source/WebCore/page/DOMWindow.cpp (214471 => 214472)


--- trunk/Source/WebCore/page/DOMWindow.cpp	2017-03-28 10:42:39 UTC (rev 214471)
+++ trunk/Source/WebCore/page/DOMWindow.cpp	2017-03-28 11:56:52 UTC (rev 214472)
@@ -515,6 +515,9 @@
     copyToVector(m_properties, properties);
     for (auto& property : properties)
         property->willDetachGlobalObjectFromFrame();
+
+    if (m_performance)
+        m_performance->clearResourceTimings();
 }
 
 #if ENABLE(GAMEPAD)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to