Diff
Modified: trunk/Source/WebCore/ChangeLog (207458 => 207459)
--- trunk/Source/WebCore/ChangeLog 2016-10-18 12:28:55 UTC (rev 207458)
+++ trunk/Source/WebCore/ChangeLog 2016-10-18 12:30:40 UTC (rev 207459)
@@ -1,3 +1,43 @@
+2016-10-18 Youenn Fablet <you...@apple.com>
+
+ CachedResourceLoader should not need to remove fragment identifier
+ https://bugs.webkit.org/show_bug.cgi?id=163015
+
+ Reviewed by Darin Adler.
+
+ No expected change for non-window port.
+ For window port, CachedResourceLoader will strip the fragment identifier of the URL passed to subresourceForURL
+ before querying the memory cache.
+
+ Removing the fragment identifier from the request stored in CachedResourceRequest.
+ The fragment identifier is stored in a separate field.
+
+ This allows CachedResourceLoader to not care about fragment identifier.
+ CachedResource can then get access to it.
+
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::CachedResource):
+ (WebCore::CachedResource::finishRequestInitialization): Deleted.
+ * loader/cache/CachedResource.h:
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::cachedResource):
+ Updated the method taking a const String& to strip the fragment identifier if needed.
+ Updated the method taking a const URL& to assert if the fragment identifier is present.
+ (WebCore::CachedResourceLoader::requestUserCSSStyleSheet):
+ (WebCore::CachedResourceLoader::requestResource):
+ * loader/cache/CachedResourceRequest.cpp:
+ (WebCore::CachedResourceRequest::CachedResourceRequest):
+ (WebCore::CachedResourceRequest::splitFragmentIdentifierFromRequestURL):
+ * loader/cache/CachedResourceRequest.h:
+ (WebCore::CachedResourceRequest::releaseFragmentIdentifier):
+ (WebCore::CachedResourceRequest::clearFragmentIdentifier):
+ * loader/cache/MemoryCache.cpp:
+ (WebCore::MemoryCache::shouldRemoveFragmentIdentifier):
+ (WebCore::MemoryCache::removeFragmentIdentifierIfNeeded):
+ (WebCore::MemoryCache::revalidationSucceeded):
+ (WebCore::MemoryCache::resourceForRequest):
+ * loader/cache/MemoryCache.h:
+
2016-10-18 Antti Koivisto <an...@apple.com>
Rename setNeedsStyleRecalc to invalidateStyle
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (207458 => 207459)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2016-10-18 12:28:55 UTC (rev 207458)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2016-10-18 12:30:40 UTC (rev 207459)
@@ -121,6 +121,7 @@
, m_sessionID(sessionID)
, m_loadPriority(defaultPriorityForResourceType(type))
, m_responseTimestamp(std::chrono::system_clock::now())
+ , m_fragmentIdentifierForRequest(request.releaseFragmentIdentifier())
, m_origin(request.releaseOrigin())
, m_type(type)
{
@@ -127,7 +128,9 @@
ASSERT(sessionID.isValid());
setLoadPriority(request.priority());
- finishRequestInitialization();
+#ifndef NDEBUG
+ cachedResourceLeakCounter.increment();
+#endif
// FIXME: We should have a better way of checking for Navigation loads, maybe FetchMode::Options::Navigate.
ASSERT(m_origin || m_type == CachedResource::MainResource);
@@ -138,31 +141,20 @@
setCrossOrigin();
}
+// FIXME: For this constructor, we should probably mandate that the URL has no fragment identifier.
CachedResource::CachedResource(const URL& url, Type type, SessionID sessionID)
: m_resourceRequest(url)
, m_decodedDataDeletionTimer(*this, &CachedResource::destroyDecodedData, deadDecodedDataDeletionIntervalForResourceType(type))
, m_sessionID(sessionID)
, m_responseTimestamp(std::chrono::system_clock::now())
+ , m_fragmentIdentifierForRequest(CachedResourceRequest::splitFragmentIdentifierFromRequestURL(m_resourceRequest))
, m_type(type)
, m_status(Cached)
{
ASSERT(sessionID.isValid());
- finishRequestInitialization();
-}
-
-void CachedResource::finishRequestInitialization()
-{
#ifndef NDEBUG
cachedResourceLeakCounter.increment();
#endif
-
- if (!m_resourceRequest.url().hasFragmentIdentifier())
- return;
- URL urlForCache = MemoryCache::removeFragmentIdentifierIfNeeded(m_resourceRequest.url());
- if (urlForCache.hasFragmentIdentifier())
- return;
- m_fragmentIdentifierForRequest = m_resourceRequest.url().fragmentIdentifier();
- m_resourceRequest.setURL(urlForCache);
}
CachedResource::~CachedResource()
Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (207458 => 207459)
--- trunk/Source/WebCore/loader/cache/CachedResource.h 2016-10-18 12:28:55 UTC (rev 207458)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h 2016-10-18 12:30:40 UTC (rev 207459)
@@ -302,8 +302,6 @@
private:
class Callback;
- void finishRequestInitialization();
-
bool addClientToSet(CachedResourceClient&);
void decodedDataDeletionTimerFired();
Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (207458 => 207459)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2016-10-18 12:28:55 UTC (rev 207458)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2016-10-18 12:30:40 UTC (rev 207459)
@@ -148,17 +148,16 @@
ASSERT(m_requestCount == 0);
}
-CachedResource* CachedResourceLoader::cachedResource(const String& resourceURL) const
+CachedResource* CachedResourceLoader::cachedResource(const String& resourceURL) const
{
ASSERT(!resourceURL.isNull());
- URL url = ""
- return cachedResource(url);
+ return cachedResource(MemoryCache::removeFragmentIdentifierIfNeeded(m_document->completeURL(resourceURL)));
}
-CachedResource* CachedResourceLoader::cachedResource(const URL& resourceURL) const
+CachedResource* CachedResourceLoader::cachedResource(const URL& url) const
{
- URL url = ""
- return m_documentResources.get(url).get();
+ ASSERT(!MemoryCache::shouldRemoveFragmentIdentifier(url));
+ return m_documentResources.get(url).get();
}
Frame* CachedResourceLoader::frame() const
@@ -657,9 +656,6 @@
LOG(ResourceLoading, "CachedResourceLoader::requestResource '%s', charset '%s', priority=%d, forPreload=%u", url.stringCenterEllipsizedToLength().latin1().data(), request.charset().latin1().data(), request.priority() ? static_cast<int>(request.priority().value()) : -1, forPreload == ForPreload::Yes);
- // If only the fragment identifiers differ, it is the same resource.
- url = ""
-
if (!url.isValid()) {
RELEASE_LOG_IF_ALLOWED("requestResource: URL is invalid (frame = %p)", frame());
return nullptr;
Modified: trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp (207458 => 207459)
--- trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp 2016-10-18 12:28:55 UTC (rev 207458)
+++ trunk/Source/WebCore/loader/cache/CachedResourceRequest.cpp 2016-10-18 12:30:40 UTC (rev 207459)
@@ -42,9 +42,21 @@
, m_charset(WTFMove(charset))
, m_options(options)
, m_priority(priority)
+ , m_fragmentIdentifier(splitFragmentIdentifierFromRequestURL(m_resourceRequest))
{
}
+String CachedResourceRequest::splitFragmentIdentifierFromRequestURL(ResourceRequest& request)
+{
+ if (!MemoryCache::shouldRemoveFragmentIdentifier(request.url()))
+ return { };
+ URL url = ""
+ String fragmentIdentifier = url.fragmentIdentifier();
+ url.removeFragmentIdentifier();
+ request.setURL(url);
+ return fragmentIdentifier;
+}
+
void CachedResourceRequest::setInitiator(PassRefPtr<Element> element)
{
ASSERT(!m_initiatorElement && m_initiatorName.isEmpty());
Modified: trunk/Source/WebCore/loader/cache/CachedResourceRequest.h (207458 => 207459)
--- trunk/Source/WebCore/loader/cache/CachedResourceRequest.h 2016-10-18 12:28:55 UTC (rev 207458)
+++ trunk/Source/WebCore/loader/cache/CachedResourceRequest.h 2016-10-18 12:30:40 UTC (rev 207459)
@@ -78,6 +78,11 @@
RefPtr<SecurityOrigin> releaseOrigin() { return WTFMove(m_origin); }
SecurityOrigin* origin() const { return m_origin.get(); }
+ String&& releaseFragmentIdentifier() { return WTFMove(m_fragmentIdentifier); }
+ void clearFragmentIdentifier() { m_fragmentIdentifier = { }; }
+
+ static String splitFragmentIdentifierFromRequestURL(ResourceRequest&);
+
private:
ResourceRequest m_resourceRequest;
String m_charset;
@@ -86,6 +91,7 @@
RefPtr<Element> m_initiatorElement;
AtomicString m_initiatorName;
RefPtr<SecurityOrigin> m_origin;
+ String m_fragmentIdentifier;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/loader/cache/MemoryCache.cpp (207458 => 207459)
--- trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2016-10-18 12:28:55 UTC (rev 207458)
+++ trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2016-10-18 12:30:40 UTC (rev 207459)
@@ -89,14 +89,19 @@
return *map;
}
-URL MemoryCache::removeFragmentIdentifierIfNeeded(const URL& originalURL)
+bool MemoryCache::shouldRemoveFragmentIdentifier(const URL& originalURL)
{
if (!originalURL.hasFragmentIdentifier())
- return originalURL;
+ return false;
// Strip away fragment identifier from HTTP URLs.
- // Data URLs must be unmodified. For file and custom URLs clients may expect resources
+ // Data URLs must be unmodified. For file and custom URLs clients may expect resources
// to be unique even when they differ by the fragment identifier only.
- if (!originalURL.protocolIsInHTTPFamily())
+ return originalURL.protocolIsInHTTPFamily();
+}
+
+URL MemoryCache::removeFragmentIdentifierIfNeeded(const URL& originalURL)
+{
+ if (!shouldRemoveFragmentIdentifier(originalURL))
return originalURL;
URL url = ""
url.removeFragmentIdentifier();
@@ -154,7 +159,7 @@
insertInLiveDecodedResourcesList(resource);
if (delta)
adjustSize(resource.hasClients(), delta);
-
+
revalidatingResource.switchClientsToRevalidatedResource();
ASSERT(!revalidatingResource.m_deleted);
// this deletes the revalidating resource
@@ -171,6 +176,8 @@
CachedResource* MemoryCache::resourceForRequest(const ResourceRequest& request, SessionID sessionID)
{
+ // FIXME: Change all clients to make sure HTTP(s) URLs have no fragment identifiers before calling here.
+ // CachedResourceLoader is now doing this. Add an assertion once all other clients are doing it too.
auto* resources = sessionResourceMap(sessionID);
if (!resources)
return nullptr;
Modified: trunk/Source/WebCore/loader/cache/MemoryCache.h (207458 => 207459)
--- trunk/Source/WebCore/loader/cache/MemoryCache.h 2016-10-18 12:28:55 UTC (rev 207458)
+++ trunk/Source/WebCore/loader/cache/MemoryCache.h 2016-10-18 12:30:40 UTC (rev 207459)
@@ -97,8 +97,9 @@
bool add(CachedResource&);
void remove(CachedResource&);
- static URL removeFragmentIdentifierIfNeeded(const URL& originalURL);
-
+ static bool shouldRemoveFragmentIdentifier(const URL&);
+ static URL removeFragmentIdentifierIfNeeded(const URL&);
+
void revalidationSucceeded(CachedResource& revalidatingResource, const ResourceResponse&);
void revalidationFailed(CachedResource& revalidatingResource);
@@ -105,8 +106,8 @@
void forEachResource(const std::function<void(CachedResource&)>&);
void forEachSessionResource(SessionID, const std::function<void(CachedResource&)>&);
void destroyDecodedDataForAllImages();
-
- // Sets the cache's memory capacities, in bytes. These will hold only approximately,
+
+ // Sets the cache's memory capacities, in bytes. These will hold only approximately,
// since the decoded cost of resources like scripts and stylesheets is not known.
// - minDeadBytes: The maximum number of bytes that dead resources should consume when the cache is under pressure.
// - maxDeadBytes: The maximum number of bytes that dead resources should consume when the cache is not under pressure.
@@ -120,7 +121,7 @@
WEBCORE_EXPORT void evictResources();
WEBCORE_EXPORT void evictResources(SessionID);
-
+
void prune();
void pruneSoon();
unsigned size() const { return m_liveSize + m_deadSize; }
