Diff
Modified: trunk/LayoutTests/ChangeLog (214360 => 214361)
--- trunk/LayoutTests/ChangeLog 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/LayoutTests/ChangeLog 2017-03-24 18:41:42 UTC (rev 214361)
@@ -1,3 +1,22 @@
+2017-03-24 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.
+
+ This adds a test for the unused preload warning, and adapts some of the other preload tests,
+ to avoid warnings.
+
+ * http/tests/preload/download_resources-expected.txt: Changed.
+ * http/tests/preload/download_resources.html: Added references to preloaded resources.
+ * http/tests/preload/onerrer_event-expected.txt: Changed.
+ * http/tests/preload/onerror_event.html: Added references to preloaded resources.
+ * http/tests/preload/onload_event.html: Added references to preloaded resources.
+ * http/tests/preload/resources/single_download_preload.html: Deflaked.
+ * http/tests/preload/unused_preload_warning-expected.txt: Added.
+ * http/tests/preload/unused_preload_warning.html: Added.
+
2017-03-24 Brent Fulgham <bfulg...@apple.com>
Handle recursive calls to ProcessingInstruction::checkStyleSheet
Modified: trunk/LayoutTests/http/tests/preload/download_resources-expected.txt (214360 => 214361)
--- trunk/LayoutTests/http/tests/preload/download_resources-expected.txt 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/LayoutTests/http/tests/preload/download_resources-expected.txt 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/LayoutTests/http/tests/preload/download_resources.html 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/LayoutTests/http/tests/preload/download_resources.html 2017-03-24 18:41:42 UTC (rev 214361)
@@ -1,27 +1,43 @@
<!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');");
+ setTimeout(t.step_func(function() {
+ 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 (214360 => 214361)
--- trunk/LayoutTests/http/tests/preload/onerror_event-expected.txt 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/LayoutTests/http/tests/preload/onerror_event-expected.txt 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/LayoutTests/http/tests/preload/onerror_event.html 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/LayoutTests/http/tests/preload/onerror_event.html 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/LayoutTests/http/tests/preload/onload_event-expected.txt 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/LayoutTests/http/tests/preload/onload_event-expected.txt 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/LayoutTests/http/tests/preload/onload_event.html 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/LayoutTests/http/tests/preload/onload_event.html 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/LayoutTests/http/tests/preload/single_download_preload.html 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/LayoutTests/http/tests/preload/single_download_preload.html 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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);
}));
Added: trunk/LayoutTests/http/tests/preload/unused_preload_warning-expected.txt (0 => 214361)
--- trunk/LayoutTests/http/tests/preload/unused_preload_warning-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/preload/unused_preload_warning-expected.txt 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 => 214361)
--- trunk/LayoutTests/http/tests/preload/unused_preload_warning.html (rev 0)
+++ trunk/LayoutTests/http/tests/preload/unused_preload_warning.html 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/Source/WebCore/ChangeLog 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/Source/WebCore/ChangeLog 2017-03-24 18:41:42 UTC (rev 214361)
@@ -1,3 +1,31 @@
+2017-03-24 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.
+
+ This patch adds a warning message, to warn developers that are using
+ link preloads in cases where the downloaded resource is likely to
+ remain unused.
+
+ Test: 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::warnUnusedPreloads): Iterate over m_preloads and issue a warning for non-referenced preloads.
+ (WebCore::CachedResourceLoader::documentDidFinishLoadEvent): Trigger a timer if preloads weren't cleared at load time.
+ (WebCore::CachedResourceLoader::warnUnusedPreloads): Triggered by the timer, and called CachedResourceLoader::warnUnusedPreloads.
+ (WebCore::CachedResourceLoader::stopUnusedPreloadsTimer): Stop the timer.
+
2017-03-24 Brent Fulgham <bfulg...@apple.com>
Handle recursive calls to ProcessingInstruction::checkStyleSheet
Modified: trunk/Source/WebCore/dom/Document.cpp (214360 => 214361)
--- trunk/Source/WebCore/dom/Document.cpp 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/Source/WebCore/dom/Document.cpp 2017-03-24 18:41:42 UTC (rev 214361)
@@ -2276,6 +2276,8 @@
}
#endif
+ m_cachedResourceLoader->stopUnusedPreloadsTimer();
+
detachFromFrame();
m_hasPreparedForDestruction = true;
Modified: trunk/Source/WebCore/loader/LinkPreloadResourceClients.h (214360 => 214361)
--- trunk/Source/WebCore/loader/LinkPreloadResourceClients.h 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/Source/WebCore/loader/LinkPreloadResourceClients.h 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/Source/WebCore/loader/cache/CachedResourceClient.h 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/Source/WebCore/loader/cache/CachedResourceClient.h 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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 (214360 => 214361)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.h 2017-03-24 18:39:28 UTC (rev 214360)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.h 2017-03-24 18:41:42 UTC (rev 214361)
@@ -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;
