[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-10851-g50815da
antti at apple.com
antti at apple.com
Wed Dec 22 18:39:36 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 76bf3f2b4291c0c5f511b989ed3794303147dcc1
Author: antti at apple.com <antti at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Dec 15 11:27:52 2010 +0000
WebCore: https://bugs.webkit.org/show_bug.cgi?id=49548
WebCore cache stores duplicate copies of subresources with URL fragments
Reviewed by Alexey Proskuryakov.
- Strip fragment identifiers from HTTP and file URLs for the memory cache.
- Changed some CachedResourceLoader and MemoryCache interfaces to use KURLs
instead of strings to reduce repeated URL parsing.
Test: http/tests/cache/subresource-fragment-identifier.html
* inspector/InspectorResourceAgent.cpp:
(WebCore::InspectorResourceAgent::cachedResource):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::tellClientAboutPastMemoryCacheLoads):
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::~CachedResource):
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::cachedResource):
(WebCore::CachedResourceLoader::checkForReload):
(WebCore::CachedResourceLoader::requestUserCSSStyleSheet):
(WebCore::CachedResourceLoader::requestResource):
* loader/cache/CachedResourceLoader.h:
* loader/cache/MemoryCache.cpp:
(WebCore::MemoryCache::requestResource):
(WebCore::MemoryCache::requestUserCSSStyleSheet):
(WebCore::MemoryCache::removeFragmentIdentifierIfNeeded):
(WebCore::MemoryCache::resourceForURL):
* loader/cache/MemoryCache.h:
LayoutTests: https://bugs.webkit.org/show_bug.cgi?id=49548
WebCore cache stores duplicate copies of subresources with URL fragments
Reviewed by Alexey Proskuryakov.
* http/tests/cache/subresource-fragment-identifier-expected.txt: Added.
* http/tests/cache/subresource-fragment-identifier.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@74107 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index d8cbe84..354fb76 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2010-12-14 Antti Koivisto <antti at apple.com>
+
+ Reviewed by Alexey Proskuryakov.
+
+ https://bugs.webkit.org/show_bug.cgi?id=49548
+ WebCore cache stores duplicate copies of subresources with URL fragments
+
+ * http/tests/cache/subresource-fragment-identifier-expected.txt: Added.
+ * http/tests/cache/subresource-fragment-identifier.html: Added.
+
2010-12-15 Emil Eklund <eae at chromium.org>
Reviewed by Adam Barth.
diff --git a/LayoutTests/http/tests/cache/subresource-fragment-identifier-expected.txt b/LayoutTests/http/tests/cache/subresource-fragment-identifier-expected.txt
new file mode 100644
index 0000000..cd39098
--- /dev/null
+++ b/LayoutTests/http/tests/cache/subresource-fragment-identifier-expected.txt
@@ -0,0 +1,5 @@
+ Test that resources with fragment identifiers are loaded only once.
+Resource requests:
+
+
+bg_pattern.jpg
diff --git a/LayoutTests/http/tests/cache/subresource-fragment-identifier.html b/LayoutTests/http/tests/cache/subresource-fragment-identifier.html
new file mode 100644
index 0000000..b33e7de
--- /dev/null
+++ b/LayoutTests/http/tests/cache/subresource-fragment-identifier.html
@@ -0,0 +1,62 @@
+<html>
+<head>
+ <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+ <script>
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ function CallCommand(cmd)
+ {
+ try {
+ var req = new XMLHttpRequest;
+ req.open("GET", "http://127.0.0.1:8000/resources/network-simulator.php?command=" + cmd, false);
+ req.send(null);
+ return req.responseText;
+ } catch (ex) {
+ return "";
+ }
+ }
+
+ function endTest()
+ {
+ getResourceLog();
+ CallCommand("clear-resource-request-log");
+
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }
+
+ function getResourceLog()
+ {
+ var log = CallCommand("get-resource-request-log");
+ var logLines = log.split('\n');
+ document.getElementById('result').innerText = logLines.join('\n');
+ }
+
+ CallCommand("start-resource-request-log");
+ window.addEventListener('load', endTest, false);
+ </script>
+
+ <style>
+ div {
+ background-image: url('http://127.0.0.1:8000/resources/network-simulator.php?command=log-resource-request&path=bg_pattern.jpg#div');
+ }
+ p {
+ background-image: url('http://127.0.0.1:8000/resources/network-simulator.php?command=log-resource-request&path=bg_pattern.jpg#p');
+ }
+ </style>
+
+</head>
+<body>
+ <div style="width: 100px; height: 100px;"></div>
+ <img src="http://127.0.0.1:8000/resources/network-simulator.php?command=log-resource-request&path=bg_pattern.jpg#img1">
+ <img src="http://127.0.0.1:8000/resources/network-simulator.php?command=log-resource-request&path=bg_pattern.jpg#img2">
+
+ Test that resources with fragment identifiers are loaded only once.
+
+ <h2>Resource requests:</h2>
+ <pre id="result">Request log goes here in DRT</pre>
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 485f54a..2471475 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,35 @@
+2010-12-14 Antti Koivisto <antti at apple.com>
+
+ Reviewed by Alexey Proskuryakov.
+
+ https://bugs.webkit.org/show_bug.cgi?id=49548
+ WebCore cache stores duplicate copies of subresources with URL fragments
+
+ - Strip fragment identifiers from HTTP and file URLs for the memory cache.
+ - Changed some CachedResourceLoader and MemoryCache interfaces to use KURLs
+ instead of strings to reduce repeated URL parsing.
+
+ Test: http/tests/cache/subresource-fragment-identifier.html
+
+ * inspector/InspectorResourceAgent.cpp:
+ (WebCore::InspectorResourceAgent::cachedResource):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::tellClientAboutPastMemoryCacheLoads):
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::~CachedResource):
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::cachedResource):
+ (WebCore::CachedResourceLoader::checkForReload):
+ (WebCore::CachedResourceLoader::requestUserCSSStyleSheet):
+ (WebCore::CachedResourceLoader::requestResource):
+ * loader/cache/CachedResourceLoader.h:
+ * loader/cache/MemoryCache.cpp:
+ (WebCore::MemoryCache::requestResource):
+ (WebCore::MemoryCache::requestUserCSSStyleSheet):
+ (WebCore::MemoryCache::removeFragmentIdentifierIfNeeded):
+ (WebCore::MemoryCache::resourceForURL):
+ * loader/cache/MemoryCache.h:
+
2010-12-15 Anton Muhin <antonm at chromium.org>
Reviewed by David Levin.
diff --git a/WebCore/inspector/InspectorResourceAgent.cpp b/WebCore/inspector/InspectorResourceAgent.cpp
index 3ed14f7..8f54578 100644
--- a/WebCore/inspector/InspectorResourceAgent.cpp
+++ b/WebCore/inspector/InspectorResourceAgent.cpp
@@ -130,10 +130,9 @@ PassRefPtr<SharedBuffer> InspectorResourceAgent::resourceData(Frame* frame, cons
CachedResource* InspectorResourceAgent::cachedResource(Frame* frame, const KURL& url)
{
- const String& urlString = url.string();
- CachedResource* cachedResource = frame->document()->cachedResourceLoader()->cachedResource(urlString);
+ CachedResource* cachedResource = frame->document()->cachedResourceLoader()->cachedResource(url);
if (!cachedResource)
- cachedResource = cache()->resourceForURL(urlString);
+ cachedResource = cache()->resourceForURL(url);
return cachedResource;
}
diff --git a/WebCore/loader/FrameLoader.cpp b/WebCore/loader/FrameLoader.cpp
index 9d5e7d0..3c80b29 100644
--- a/WebCore/loader/FrameLoader.cpp
+++ b/WebCore/loader/FrameLoader.cpp
@@ -3421,7 +3421,7 @@ void FrameLoader::tellClientAboutPastMemoryCacheLoads()
size_t size = pastLoads.size();
for (size_t i = 0; i < size; ++i) {
- CachedResource* resource = cache()->resourceForURL(pastLoads[i]);
+ CachedResource* resource = cache()->resourceForURL(KURL(ParsedURLString, pastLoads[i]));
// FIXME: These loads, loaded from cache, but now gone from the cache by the time
// Page::setMemoryCacheClientCallsEnabled(true) is called, will not be seen by the client.
diff --git a/WebCore/loader/cache/CachedResource.cpp b/WebCore/loader/cache/CachedResource.cpp
index 9e9ad04..19e535f 100644
--- a/WebCore/loader/cache/CachedResource.cpp
+++ b/WebCore/loader/cache/CachedResource.cpp
@@ -116,7 +116,7 @@ CachedResource::~CachedResource()
ASSERT(canDelete());
ASSERT(!inCache());
ASSERT(!m_deleted);
- ASSERT(url().isNull() || cache()->resourceForURL(url()) != this);
+ ASSERT(url().isNull() || cache()->resourceForURL(KURL(ParsedURLString, url())) != this);
#ifndef NDEBUG
m_deleted = true;
cachedResourceLeakCounter.decrement();
diff --git a/WebCore/loader/cache/CachedResourceLoader.cpp b/WebCore/loader/cache/CachedResourceLoader.cpp
index 05ee70e..5443815 100644
--- a/WebCore/loader/cache/CachedResourceLoader.cpp
+++ b/WebCore/loader/cache/CachedResourceLoader.cpp
@@ -73,6 +73,18 @@ CachedResourceLoader::~CachedResourceLoader()
ASSERT(m_requestCount == 0);
}
+CachedResource* CachedResourceLoader::cachedResource(const String& resourceURL) const
+{
+ KURL url = m_document->completeURL(resourceURL);
+ return cachedResource(url);
+}
+
+CachedResource* CachedResourceLoader::cachedResource(const KURL& resourceURL) const
+{
+ KURL url = MemoryCache::removeFragmentIdentifierIfNeeded(resourceURL);
+ return m_documentResources.get(url).get();
+}
+
Frame* CachedResourceLoader::frame() const
{
return m_document->frame();
@@ -89,7 +101,7 @@ void CachedResourceLoader::checkForReload(const KURL& fullURL)
if (m_reloadedURLs.contains(fullURL.string()))
return;
- CachedResource* existing = cache()->resourceForURL(fullURL.string());
+ CachedResource* existing = cache()->resourceForURL(fullURL);
if (!existing || existing->isPreloaded())
return;
@@ -151,7 +163,7 @@ CachedCSSStyleSheet* CachedResourceLoader::requestCSSStyleSheet(const String& ur
CachedCSSStyleSheet* CachedResourceLoader::requestUserCSSStyleSheet(const String& url, const String& charset)
{
- return cache()->requestUserCSSStyleSheet(this, url, charset);
+ return cache()->requestUserCSSStyleSheet(this, KURL(KURL(), url), charset);
}
CachedScript* CachedResourceLoader::requestScript(const String& url, const String& charset)
@@ -245,6 +257,9 @@ bool CachedResourceLoader::canRequest(CachedResource::Type type, const KURL& url
CachedResource* CachedResourceLoader::requestResource(CachedResource::Type type, const String& url, const String& charset, ResourceLoadPriority priority, bool isPreload)
{
KURL fullURL = m_document->completeURL(url);
+
+ // If only the fragment identifiers differ, it is the same resource.
+ fullURL = MemoryCache::removeFragmentIdentifierIfNeeded(fullURL);
if (!fullURL.isValid() || !canRequest(type, fullURL))
return 0;
diff --git a/WebCore/loader/cache/CachedResourceLoader.h b/WebCore/loader/cache/CachedResourceLoader.h
index 2957db1..0a44f73 100644
--- a/WebCore/loader/cache/CachedResourceLoader.h
+++ b/WebCore/loader/cache/CachedResourceLoader.h
@@ -73,7 +73,8 @@ public:
// Logs an access denied message to the console for the specified URL.
void printAccessDeniedMessage(const KURL& url) const;
- CachedResource* cachedResource(const String& url) const { return m_documentResources.get(url).get(); }
+ CachedResource* cachedResource(const String& url) const;
+ CachedResource* cachedResource(const KURL& url) const;
typedef HashMap<String, CachedResourceHandle<CachedResource> > DocumentResourceMap;
const DocumentResourceMap& allCachedResources() const { return m_documentResources; }
diff --git a/WebCore/loader/cache/MemoryCache.cpp b/WebCore/loader/cache/MemoryCache.cpp
index 7b46fca..8826cec 100644
--- a/WebCore/loader/cache/MemoryCache.cpp
+++ b/WebCore/loader/cache/MemoryCache.cpp
@@ -95,17 +95,20 @@ static CachedResource* createResource(CachedResource::Type type, const KURL& url
return 0;
}
-CachedResource* MemoryCache::requestResource(CachedResourceLoader* cachedResourceLoader, CachedResource::Type type, const KURL& url, const String& charset, ResourceLoadPriority priority, bool requestIsPreload, bool forHistory)
+CachedResource* MemoryCache::requestResource(CachedResourceLoader* cachedResourceLoader, CachedResource::Type type, const KURL& requestURL, const String& charset, ResourceLoadPriority priority, bool requestIsPreload, bool forHistory)
{
- LOG(ResourceLoading, "MemoryCache::requestResource '%s', charset '%s', priority=%d, preload=%u, forHistory=%u", url.string().latin1().data(), charset.latin1().data(), priority, requestIsPreload, forHistory);
+ LOG(ResourceLoading, "MemoryCache::requestResource '%s', charset '%s', priority=%d, preload=%u, forHistory=%u", requestURL.string().latin1().data(), charset.latin1().data(), priority, requestIsPreload, forHistory);
// FIXME: Do we really need to special-case an empty URL?
// Would it be better to just go on with the cache code and let it fail later?
- if (url.isEmpty())
+ if (requestURL.isEmpty())
return 0;
+
+ // Ensure this is the pure primary resource URL.
+ KURL url = removeFragmentIdentifierIfNeeded(requestURL);
// Look up the resource in our map.
- CachedResource* resource = resourceForURL(url.string());
+ CachedResource* resource = resourceForURL(url);
// Non https "no-store" resources are left in the cache to be used for back/forward navigation only.
// If this is not a request forHistory and the resource was served with "no-store" we should evict
@@ -184,8 +187,11 @@ CachedResource* MemoryCache::requestResource(CachedResourceLoader* cachedResourc
return resource;
}
-CachedCSSStyleSheet* MemoryCache::requestUserCSSStyleSheet(CachedResourceLoader* cachedResourceLoader, const String& url, const String& charset)
+CachedCSSStyleSheet* MemoryCache::requestUserCSSStyleSheet(CachedResourceLoader* cachedResourceLoader, const KURL& requestURL, const String& charset)
{
+ // Ensure this is the pure primary resource URL.
+ KURL url = removeFragmentIdentifierIfNeeded(requestURL);
+
CachedCSSStyleSheet* userSheet;
if (CachedResource* existing = resourceForURL(url)) {
if (existing->type() != CachedResource::CSSStyleSheet)
@@ -212,7 +218,20 @@ CachedCSSStyleSheet* MemoryCache::requestUserCSSStyleSheet(CachedResourceLoader*
return userSheet;
}
-
+
+KURL MemoryCache::removeFragmentIdentifierIfNeeded(const KURL& originalURL)
+{
+ if (!originalURL.hasFragmentIdentifier())
+ return originalURL;
+ // Strip away fragment identifier from HTTP and file urls.
+ // Data urls must be unmodified and it is also safer to keep them for custom protocols.
+ if (!(originalURL.protocolInHTTPFamily() || originalURL.isLocalFile()))
+ return originalURL;
+ KURL url = originalURL;
+ url.removeFragmentIdentifier();
+ return url;
+}
+
void MemoryCache::revalidateResource(CachedResource* resource, CachedResourceLoader* cachedResourceLoader)
{
ASSERT(resource);
@@ -269,8 +288,9 @@ void MemoryCache::revalidationFailed(CachedResource* revalidatingResource)
revalidatingResource->clearResourceToRevalidate();
}
-CachedResource* MemoryCache::resourceForURL(const String& url)
+CachedResource* MemoryCache::resourceForURL(const KURL& resourceURL)
{
+ KURL url = removeFragmentIdentifierIfNeeded(resourceURL);
CachedResource* resource = m_resources.get(url);
bool wasPurgeable = MemoryCache::shouldMakeResourcePurgeableOnEviction() && resource && resource->isPurgeable();
if (resource && !resource->makePurgeable(false)) {
diff --git a/WebCore/loader/cache/MemoryCache.h b/WebCore/loader/cache/MemoryCache.h
index b9564b3..15f4014 100644
--- a/WebCore/loader/cache/MemoryCache.h
+++ b/WebCore/loader/cache/MemoryCache.h
@@ -107,7 +107,9 @@ public:
// found in the cache.
CachedResource* requestResource(CachedResourceLoader*, CachedResource::Type, const KURL& url, const String& charset, ResourceLoadPriority, bool isPreload = false, bool forHistory = false);
- CachedCSSStyleSheet* requestUserCSSStyleSheet(CachedResourceLoader*, const String& url, const String& charset);
+ CachedCSSStyleSheet* requestUserCSSStyleSheet(CachedResourceLoader*, const KURL& url, const String& charset);
+
+ static KURL removeFragmentIdentifierIfNeeded(const KURL& originalURL);
void revalidateResource(CachedResource*, CachedResourceLoader*);
void revalidationSucceeded(CachedResource* revalidatingResource, const ResourceResponse&);
@@ -144,7 +146,7 @@ public:
void addCachedResourceLoader(CachedResourceLoader*);
void removeCachedResourceLoader(CachedResourceLoader*);
- CachedResource* resourceForURL(const String&);
+ CachedResource* resourceForURL(const KURL&);
// Calls to put the cached resource into and out of LRU lists.
void insertInLRUList(CachedResource*);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list