Title: [87901] trunk/Source/WebCore
- Revision
- 87901
- Author
- commit-qu...@webkit.org
- Date
- 2011-06-02 05:52:37 -0700 (Thu, 02 Jun 2011)
Log Message
2011-06-02 James Robinson <jam...@chromium.org>
Reviewed by Brady Eidson.
DocumentLoader keeps a reference to all URL strings ever loaded leading to lots of memory waste
https://bugs.webkit.org/show_bug.cgi?id=61894
DocumentLoader::m_resourcesClientKnowsAbout is a set of all the URLs that have passed through
FrameLoader::dispatchWillSendRequest() and is used by FrameLoader::loadedResourceFromMemoryCached to decide
whether to inform the FrameLoader's m_client about this load. Unfortunately, this set holds a reference to the
URL string for every resource loaded, so on pages that use data URLs to "load" large amounts of data this leaks
lots of memory.
This set only has an effect on the Mac port, as the only two ports that implement
FrameLoaderClient::dispatchDidLoadResourceFromMemoryCache() are Chromium and Mac and the Chromium implementation
can correctly handle receiving multiple callbacks. This patch limits the set to only PLATFORM(MAC) so other
ports do not have to pay this memory cost. It's possible that a better fix exists specifically for the Mac port
implementation, but that would have to determined by someone who works on that port specifically.
* loader/DocumentLoader.h:
(WebCore::DocumentLoader::didTellClientAboutLoad):
(WebCore::DocumentLoader::haveToldClientAboutLoad):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (87900 => 87901)
--- trunk/Source/WebCore/ChangeLog 2011-06-02 12:52:17 UTC (rev 87900)
+++ trunk/Source/WebCore/ChangeLog 2011-06-02 12:52:37 UTC (rev 87901)
@@ -1,3 +1,26 @@
+2011-06-02 James Robinson <jam...@chromium.org>
+
+ Reviewed by Brady Eidson.
+
+ DocumentLoader keeps a reference to all URL strings ever loaded leading to lots of memory waste
+ https://bugs.webkit.org/show_bug.cgi?id=61894
+
+ DocumentLoader::m_resourcesClientKnowsAbout is a set of all the URLs that have passed through
+ FrameLoader::dispatchWillSendRequest() and is used by FrameLoader::loadedResourceFromMemoryCached to decide
+ whether to inform the FrameLoader's m_client about this load. Unfortunately, this set holds a reference to the
+ URL string for every resource loaded, so on pages that use data URLs to "load" large amounts of data this leaks
+ lots of memory.
+
+ This set only has an effect on the Mac port, as the only two ports that implement
+ FrameLoaderClient::dispatchDidLoadResourceFromMemoryCache() are Chromium and Mac and the Chromium implementation
+ can correctly handle receiving multiple callbacks. This patch limits the set to only PLATFORM(MAC) so other
+ ports do not have to pay this memory cost. It's possible that a better fix exists specifically for the Mac port
+ implementation, but that would have to determined by someone who works on that port specifically.
+
+ * loader/DocumentLoader.h:
+ (WebCore::DocumentLoader::didTellClientAboutLoad):
+ (WebCore::DocumentLoader::haveToldClientAboutLoad):
+
2011-06-02 Aparna Nandyal <aparna.n...@wipro.com>
Reviewed by Andreas Kling.
Modified: trunk/Source/WebCore/loader/DocumentLoader.h (87900 => 87901)
--- trunk/Source/WebCore/loader/DocumentLoader.h 2011-06-02 12:52:17 UTC (rev 87900)
+++ trunk/Source/WebCore/loader/DocumentLoader.h 2011-06-02 12:52:37 UTC (rev 87901)
@@ -224,12 +224,18 @@
void setDeferMainResourceDataLoad(bool defer) { m_deferMainResourceDataLoad = defer; }
bool deferMainResourceDataLoad() const { return m_deferMainResourceDataLoad; }
+#if PLATFORM(MAC)
void didTellClientAboutLoad(const String& url)
- {
+ {
if (!url.isEmpty())
m_resourcesClientKnowsAbout.add(url);
}
- bool haveToldClientAboutLoad(const String& url) { return m_resourcesClientKnowsAbout.contains(url); }
+ bool haveToldClientAboutLoad(const String& url)
+ {
+ return m_resourcesClientKnowsAbout.contains(url);
+ }
+#endif
+
void recordMemoryCacheLoadForFutureClientNotification(const String& url);
void takeMemoryCacheLoadsForClientNotification(Vector<String>& loads);
@@ -328,7 +334,9 @@
RefPtr<SharedBuffer> m_parsedArchiveData;
#endif
+#if PLATFORM(MAC)
HashSet<String> m_resourcesClientKnowsAbout;
+#endif
Vector<String> m_resourcesLoadedFromMemoryCacheForClientNotification;
String m_clientRedirectSourceForHistory;
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (87900 => 87901)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2011-06-02 12:52:17 UTC (rev 87900)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2011-06-02 12:52:37 UTC (rev 87901)
@@ -3141,20 +3141,29 @@
if (!page)
return;
- if (!resource->sendResourceLoadCallbacks() || m_documentLoader->haveToldClientAboutLoad(resource->url()))
+ if (!resource->sendResourceLoadCallbacks())
return;
+#if PLATFORM(MAC)
+ if (m_documentLoader->haveToldClientAboutLoad(resource->url()))
+ return;
+#endif
+
if (!page->areMemoryCacheClientCallsEnabled()) {
InspectorInstrumentation::didLoadResourceFromMemoryCache(page, m_documentLoader.get(), resource);
m_documentLoader->recordMemoryCacheLoadForFutureClientNotification(resource->url());
+#if PLATFORM(MAC)
m_documentLoader->didTellClientAboutLoad(resource->url());
+#endif
return;
}
ResourceRequest request(resource->url());
if (m_client->dispatchDidLoadResourceFromMemoryCache(m_documentLoader.get(), request, resource->response(), resource->encodedSize())) {
InspectorInstrumentation::didLoadResourceFromMemoryCache(page, m_documentLoader.get(), resource);
+#if PLATFORM(MAC)
m_documentLoader->didTellClientAboutLoad(resource->url());
+#endif
return;
}
Modified: trunk/Source/WebCore/loader/ResourceLoadNotifier.cpp (87900 => 87901)
--- trunk/Source/WebCore/loader/ResourceLoadNotifier.cpp 2011-06-02 12:52:17 UTC (rev 87900)
+++ trunk/Source/WebCore/loader/ResourceLoadNotifier.cpp 2011-06-02 12:52:37 UTC (rev 87901)
@@ -107,14 +107,18 @@
void ResourceLoadNotifier::dispatchWillSendRequest(DocumentLoader* loader, unsigned long identifier, ResourceRequest& request, const ResourceResponse& redirectResponse)
{
+#if PLATFORM(MAC)
StringImpl* oldRequestURL = request.url().string().impl();
m_frame->loader()->documentLoader()->didTellClientAboutLoad(request.url());
+#endif
m_frame->loader()->client()->dispatchWillSendRequest(loader, identifier, request, redirectResponse);
+#if PLATFORM(MAC)
// If the URL changed, then we want to put that new URL in the "did tell client" set too.
if (!request.isNull() && oldRequestURL != request.url().string().impl())
m_frame->loader()->documentLoader()->didTellClientAboutLoad(request.url());
+#endif
InspectorInstrumentation::willSendRequest(m_frame, identifier, loader, request, redirectResponse);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
