Diff
Modified: trunk/LayoutTests/ChangeLog (211648 => 211649)
--- trunk/LayoutTests/ChangeLog 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/LayoutTests/ChangeLog 2017-02-03 21:52:50 UTC (rev 211649)
@@ -1,3 +1,16 @@
+2017-02-03 Yoav Weiss <[email protected]>
+
+ Avoid evicting link preload resources when parsing is done.
+ https://bugs.webkit.org/show_bug.cgi?id=167415
+
+ Reviewed by Ryosuke Niwa.
+
+ * http/tests/preload/dynamic_remove_preload_href.html: Test passed before for the wrong reasons. Cache-busting in order for it to genuinely pass.
+ * http/tests/preload/not_delaying_window_onload_before_discovery.html: Test passed before for the wrong reasons. Cache-busting in order for it to genuinely pass.
+ * http/tests/preload/not_evicting_preload_at_onload-expected.txt: Added.
+ * http/tests/preload/not_evicting_preload_at_onload.html: Added.
+ * platform/mac/TestExpectations: Skipping http/tests/preload/dynamic_removing_preload.html due to https://bugs.webkit.org/show_bug.cgi?id=167792
+
2017-02-03 Zalan Bujtas <[email protected]>
Simple line layout: Removing adjacent trailing whitespace runs should not crash.
Modified: trunk/LayoutTests/TestExpectations (211648 => 211649)
--- trunk/LayoutTests/TestExpectations 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/LayoutTests/TestExpectations 2017-02-03 21:52:50 UTC (rev 211649)
@@ -1045,6 +1045,7 @@
webkit.org/b/81826 fast/table/double-height-table-no-tbody.html [ Skip ]
webkit.org/b/56140 fast/text/large-text-composed-char-dos.html [ Skip ]
webkit.org/b/63268 http/tests/multipart/win-boundary-crash.html [ Skip ]
+webkit.org/b/167792 http/tests/preload/dynamic_removing_preload.html [ Skip ]
webkit.org/b/35700 java/lc3/ArrayMethods/object-001.html [ Skip ]
webkit.org/b/35700 java/lc3/forin/array-001.html [ Skip ]
webkit.org/b/56080 jquery/effects.html [ Skip ]
Modified: trunk/LayoutTests/http/tests/preload/dynamic_remove_preload_href-expected.txt (211648 => 211649)
--- trunk/LayoutTests/http/tests/preload/dynamic_remove_preload_href-expected.txt 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/LayoutTests/http/tests/preload/dynamic_remove_preload_href-expected.txt 2017-02-03 21:52:50 UTC (rev 211649)
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 17: <link rel=preload> has an invalid `href` value
+CONSOLE MESSAGE: line 18: <link rel=preload> has an invalid `href` value
PASS downloadedImage is false
PASS successfullyParsed is true
Modified: trunk/LayoutTests/http/tests/preload/dynamic_remove_preload_href.html (211648 => 211649)
--- trunk/LayoutTests/http/tests/preload/dynamic_remove_preload_href.html 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/LayoutTests/http/tests/preload/dynamic_remove_preload_href.html 2017-02-03 21:52:50 UTC (rev 211649)
@@ -11,7 +11,8 @@
var link = document.createElement("link");
link.as = "image";
link.rel = "preload";
- link.href = ""
+ // Cache-busting is needed for slow resources.
+ link.href = "" + Math.random();
var downloadedImage = false;
document.body.appendChild(link);
link.href = ""
Modified: trunk/LayoutTests/http/tests/preload/not_delaying_window_onload_before_discovery.html (211648 => 211649)
--- trunk/LayoutTests/http/tests/preload/not_delaying_window_onload_before_discovery.html 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/LayoutTests/http/tests/preload/not_delaying_window_onload_before_discovery.html 2017-02-03 21:52:50 UTC (rev 211649)
@@ -17,10 +17,11 @@
testRunner.notifyDone();
}
};
-
+ // The rand value is necessary to cache-bust slow resources.
+ var rand = Math.random();
+ document.write("<link rel=preload href='' as=script _onload_='window.finishedScript = true; test();'>");
+ document.write("<link rel=preload href='' as=image _onload_='window.finishedImage = true; test();'>");
</script>
-<link rel=preload href="" as=script _onload_="window.finishedScript = true; test();">
-<link rel=preload href="" as=image _onload_="window.finishedImage = true; test();">
<body>
<script>
window.addEventListener("load", function() {
Added: trunk/LayoutTests/http/tests/preload/not_evicting_preload_at_onload-expected.txt (0 => 211649)
--- trunk/LayoutTests/http/tests/preload/not_evicting_preload_at_onload-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/preload/not_evicting_preload_at_onload-expected.txt 2017-02-03 21:52:50 UTC (rev 211649)
@@ -0,0 +1,5 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS 1 is 1
+
Added: trunk/LayoutTests/http/tests/preload/not_evicting_preload_at_onload.html (0 => 211649)
--- trunk/LayoutTests/http/tests/preload/not_evicting_preload_at_onload.html (rev 0)
+++ trunk/LayoutTests/http/tests/preload/not_evicting_preload_at_onload.html 2017-02-03 21:52:50 UTC (rev 211649)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<script>
+ if (window.internals)
+ window.internals.settings.setResourceTimingEnabled(true);
+ if (window.testRunner) {
+ testRunner.dumpAsText()
+ testRunner.waitUntilDone();
+ }
+</script>
+<script src=""
+<script>
+ // The rand value is necessary to cache-bust slow resources.
+ var imageURL = "http://127.0.0.1:8000/resources/slow-image.php?rand=" + Math.random();
+ function test() {
+ setTimeout(function() {
+ var entries = performance.getEntriesByName(imageURL).length;
+ shouldBe(entries.toString(), "1");
+
+ if (window.internals)
+ window.internals.settings.setResourceTimingEnabled(false);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+ };
+ document.write("<link rel=preload href='' as=image>");
+</script>
+<body>
+<script>
+ window.addEventListener("load", function() {
+ var img = new Image();
+ img._onload_ = test;
+ img.src = ""
+ });
+</script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (211648 => 211649)
--- trunk/Source/WebCore/ChangeLog 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/Source/WebCore/ChangeLog 2017-02-03 21:52:50 UTC (rev 211649)
@@ -1,3 +1,43 @@
+2017-02-03 Yoav Weiss <[email protected]>
+
+ Avoid evicting link preload resources when parsing is done.
+ https://bugs.webkit.org/show_bug.cgi?id=167415
+
+ Reviewed by Ryosuke Niwa.
+
+ Currently all preloads (speculative and link preload) are being cleared when the document has finished parsing.
+ When it comes to link preloads, it can result in resources being cleared before the page had a chance to use them. (e.g. resources
+ that are preloaded in order to be loaded through script after DOMContentLoaded)
+
+ This patch fixes that by marking link preload resources as such, so that they can be handled separately inside clearPreloads().
+ As this fix also exposed an issue with load cancelation with invalid hrefs (which tests were passing before due to the preloads
+ being cleared), said issue is also fixed by clearing previousely preloaded resources if an invalid link preload is later detected.
+
+ Test: http/tests/preload/not_evicting_preload_at_onload.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::finishedParsing): Only clear speculative preloads when parsing is finished.
+ * loader/LinkLoader.cpp:
+ (WebCore::LinkLoader::preloadIfNeeded): Set request flag indicating link preload.
+ (WebCore::LinkLoader::loadLink): Clear previousely preloaded resource to cancel their load.
+ * loader/LinkPreloadResourceClients.h:
+ (WebCore::LinkPreloadResourceClient::clearResource): Call cancelLoad() when the client is cleared.
+ * loader/cache/CachedResource.h:
+ (WebCore::CachedResource::isLinkPreload):
+ (WebCore::CachedResource::setLinkPreload):
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::CachedResource): Initialize m_isLinkPreload with the request's value.
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::~CachedResourceLoader):
+ (WebCore::CachedResourceLoader::requestResource): Mirror the request link preload flag to the resource if it's fetched from cache.
+ (WebCore::CachedResourceLoader::clearPreloads): Add a "speculative only" mode, which doesn't clear link preloads.
+ * loader/cache/CachedResourceLoader.h:
+ * loader/cache/CachedResourceRequest.cpp:
+ (WebCore::CachedResourceRequest::CachedResourceRequest):
+ * loader/cache/CachedResourceRequest.h:
+ (WebCore::CachedResourceRequest::isLinkPreload):
+ (WebCore::CachedResourceRequest::setIsLinkPreload):
+
2017-02-03 Zalan Bujtas <[email protected]>
Simple line layout: Removing adjacent trailing whitespace runs should not crash.
Modified: trunk/Source/WebCore/dom/Document.cpp (211648 => 211649)
--- trunk/Source/WebCore/dom/Document.cpp 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/Source/WebCore/dom/Document.cpp 2017-02-03 21:52:50 UTC (rev 211649)
@@ -5062,8 +5062,8 @@
static const int timeToKeepSharedObjectPoolAliveAfterParsingFinishedInSeconds = 10;
m_sharedObjectPoolClearTimer.startOneShot(timeToKeepSharedObjectPoolAliveAfterParsingFinishedInSeconds);
- // Parser should have picked up all preloads by now
- m_cachedResourceLoader->clearPreloads();
+ // Parser should have picked up all speculative preloads by now
+ m_cachedResourceLoader->clearPreloads(CachedResourceLoader::ClearPreloadsMode::ClearSpeculativePreloads);
}
void Document::clearSharedObjectPool()
Modified: trunk/Source/WebCore/loader/LinkLoader.cpp (211648 => 211649)
--- trunk/Source/WebCore/loader/LinkLoader.cpp 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/Source/WebCore/loader/LinkLoader.cpp 2017-02-03 21:52:50 UTC (rev 211649)
@@ -180,6 +180,7 @@
resourceRequest.setIgnoreForRequestCount(true);
CachedResourceRequest linkRequest(WTFMove(resourceRequest), CachedResourceLoader::defaultCachedResourceOptions(), CachedResource::defaultPriorityForResourceType(type.value()));
linkRequest.setInitiator("link");
+ linkRequest.setIsLinkPreload();
linkRequest.setAsPotentiallyCrossOrigin(crossOriginMode, document);
CachedResourceHandle<CachedResource> cachedLinkResource = document.cachedResourceLoader().preload(type.value(), WTFMove(linkRequest));
@@ -203,6 +204,8 @@
auto resourceClient = preloadIfNeeded(relAttribute, href, document, as, crossOrigin, this, &m_client);
if (resourceClient)
m_preloadResourceClient = WTFMove(resourceClient);
+ else if (m_preloadResourceClient)
+ m_preloadResourceClient->clear();
}
#if ENABLE(LINK_PREFETCH)
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (211648 => 211649)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2017-02-03 21:52:50 UTC (rev 211649)
@@ -121,6 +121,7 @@
, m_responseTimestamp(std::chrono::system_clock::now())
, m_fragmentIdentifierForRequest(request.releaseFragmentIdentifier())
, m_origin(request.releaseOrigin())
+ , m_isLinkPreload(request.isLinkPreload())
, m_type(type)
{
ASSERT(sessionID.isValid());
Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (211648 => 211649)
--- trunk/Source/WebCore/loader/cache/CachedResource.h 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h 2017-02-03 21:52:50 UTC (rev 211649)
@@ -232,6 +232,8 @@
bool isPreloaded() const { return m_preloadCount; }
void increasePreloadCount() { ++m_preloadCount; }
void decreasePreloadCount() { ASSERT(m_preloadCount); --m_preloadCount; }
+ bool isLinkPreload() { return m_isLinkPreload; }
+ void setLinkPreload() { m_isLinkPreload = true; }
void registerHandle(CachedResourceHandleBase*);
WEBCORE_EXPORT void unregisterHandle(CachedResourceHandleBase*);
@@ -336,6 +338,7 @@
bool m_inCache { false };
bool m_loading { false };
+ bool m_isLinkPreload { false };
bool m_switchingClientsToRevalidatedResource { false };
Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (211648 => 211649)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2017-02-03 21:52:50 UTC (rev 211649)
@@ -140,7 +140,7 @@
m_documentLoader = nullptr;
m_document = nullptr;
- clearPreloads();
+ clearPreloads(ClearPreloadsMode::ClearAllPreloads);
for (auto& resource : m_documentResources.values())
resource->setOwningCachedResourceLoader(nullptr);
@@ -741,6 +741,9 @@
if (request.allowsCaching())
resource = memoryCache.resourceForRequest(request.resourceRequest(), sessionID());
+ if (resource && request.isLinkPreload() && !resource->isLinkPreload())
+ resource->setLinkPreload();
+
logMemoryCacheResourceRequest(frame(), DiagnosticLoggingKeys::memoryCacheUsageKey(), resource ? DiagnosticLoggingKeys::inMemoryCacheKey() : DiagnosticLoggingKeys::notInMemoryCacheKey());
RevalidationPolicy policy = determineRevalidationPolicy(type, request, resource.get(), forPreload, defer);
@@ -1243,7 +1246,7 @@
return false;
}
-void CachedResourceLoader::clearPreloads()
+void CachedResourceLoader::clearPreloads(ClearPreloadsMode mode)
{
#if PRELOAD_DEBUG
printPreloadStats();
@@ -1252,12 +1255,16 @@
return;
for (auto* resource : *m_preloads) {
+ if (mode == ClearPreloadsMode::ClearSpeculativePreloads && resource->isLinkPreload())
+ continue;
resource->decreasePreloadCount();
bool deleted = resource->deleteIfPossible();
if (!deleted && resource->preloadResult() == CachedResource::PreloadNotReferenced)
MemoryCache::singleton().remove(*resource);
+ m_preloads->remove(resource);
}
- m_preloads = nullptr;
+ if (!m_preloads->size())
+ m_preloads = nullptr;
}
#if PRELOAD_DEBUG
Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.h (211648 => 211649)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.h 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.h 2017-02-03 21:52:50 UTC (rev 211649)
@@ -128,7 +128,8 @@
int requestCount() const { return m_requestCount; }
WEBCORE_EXPORT bool isPreloaded(const String& urlString) const;
- void clearPreloads();
+ enum class ClearPreloadsMode { ClearSpeculativePreloads, ClearAllPreloads };
+ void clearPreloads(ClearPreloadsMode);
CachedResourceHandle<CachedResource> preload(CachedResource::Type, CachedResourceRequest&&);
void printPreloadStats();
Modified: trunk/Source/WebCore/loader/cache/CachedResourceRequest.h (211648 => 211649)
--- trunk/Source/WebCore/loader/cache/CachedResourceRequest.h 2017-02-03 21:48:23 UTC (rev 211648)
+++ trunk/Source/WebCore/loader/cache/CachedResourceRequest.h 2017-02-03 21:52:50 UTC (rev 211649)
@@ -78,6 +78,8 @@
#if ENABLE(CACHE_PARTITIONING)
void setDomainForCachePartition(Document&);
#endif
+ bool isLinkPreload() const { return m_isLinkPreload; }
+ void setIsLinkPreload() { m_isLinkPreload = true; }
void setOrigin(Ref<SecurityOrigin>&& origin) { m_origin = WTFMove(origin); }
RefPtr<SecurityOrigin> releaseOrigin() { return WTFMove(m_origin); }
@@ -97,6 +99,7 @@
AtomicString m_initiatorName;
RefPtr<SecurityOrigin> m_origin;
String m_fragmentIdentifier;
+ bool m_isLinkPreload { false };
};
void upgradeInsecureResourceRequestIfNeeded(ResourceRequest&, Document&);
