.: REGRESSION (r39725?): Resources removed from document can not be freed...

.: REGRESSION (r39725?): Resources removed from document can not be freed until the document is deleted
https://bugs.webkit.org/show_bug.cgi?id=61006

Patch by Scott Graham <scottmg@chromium.org> on 2011-08-01
Reviewed by Antti Koivisto.

Update exports for test harness.

* Source/autotools/symbols.filter:

Source/WebCore: REGRESSION (r39725?): Resources removed from document can not be freed
until the document is deleted
https://bugs.webkit.org/show_bug.cgi?id=61006

Patch by Scott Graham <scottmg@chromium.org> on 2011-08-01
Reviewed by Antti Koivisto.

Upon completing a load start a Timer to iterate through
CachedResourceLoader's m_documentResources map to check for any items
that have only one reference (thus being the reference in the map
itself). The map should really be weak, but because the
CachedResourceHandle achieves bookkeeping work in addition to
reference counting, this is a simpler and more localized way to free
the used memory while maintaining the other behaviour (when
CachedResource is used as proxy).

With this patch the testcase at
https://bugs.webkit.org/attachment.cgi?id=93850 should no longer
consume 400MB of ram on load. Test added for crash discovered in
previous revision, but no tests for memory usage.

Test: http/tests/inspector/network/disabled-cache-crash.html

* WebCore.exp.in:
* loader/cache/CachedResource.h:
(WebCore::CachedResource::hasOneHandle):
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::CachedResourceLoader):
(WebCore::CachedResourceLoader::loadDone):
(WebCore::CachedResourceLoader::garbageCollectDocumentResourcesTimerFired):
* loader/cache/CachedResourceLoader.h:
* testing/Internals.cpp:
(WebCore::Internals::disableMemoryCache):
* testing/Internals.h:
* testing/Internals.idl:

Source/WebKit2: REGRESSION (r39725?): Resources removed from document can not be freed until the document is deleted
https://bugs.webkit.org/show_bug.cgi?id=61006

Patch by Scott Graham <scottmg@chromium.org> on 2011-08-01
Reviewed by Antti Koivisto.

Update exports for test harness.

* win/WebKit2.def:
* win/WebKit2CFLite.def:

LayoutTests: https://bugs.webkit.org/show_bug.cgi?id=61006

Test for CachedResourceLoader. Not caused by cache-disabling but very
difficult to reproduce when cache is active, so use cache disable in
inspector to exercise code.

Patch by Scott Graham <scottmg@chromium.org> on 2011-08-01
Reviewed by Antti Koivisto.

* http/tests/inspector/network/disabled-cache-crash-expected.txt: Added.
* http/tests/inspector/network/disabled-cache-crash.html: Added.
* platform/gtk/Skipped:
* platform/mac/Skipped:
* platform/qt/Skipped:
* platform/win/Skipped:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@92143 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent dac140c8
2011-08-01 Scott Graham <scottmg@chromium.org>
REGRESSION (r39725?): Resources removed from document can not be freed until the document is deleted
https://bugs.webkit.org/show_bug.cgi?id=61006
Reviewed by Antti Koivisto.
Update exports for test harness.
* Source/autotools/symbols.filter:
2011-08-01 Hayato Ito <hayato@chromium.org>
Add support for getting an element in shadow root by its id into a window.internals object.
......
2011-08-01 Scott Graham <scottmg@chromium.org>
https://bugs.webkit.org/show_bug.cgi?id=61006
Test for CachedResourceLoader. Not caused by cache-disabling but very
difficult to reproduce when cache is active, so use cache disable in
inspector to exercise code.
Reviewed by Antti Koivisto.
* http/tests/inspector/network/disabled-cache-crash-expected.txt: Added.
* http/tests/inspector/network/disabled-cache-crash.html: Added.
* platform/gtk/Skipped:
* platform/mac/Skipped:
* platform/qt/Skipped:
* platform/win/Skipped:
2011-08-01 Jochen Eisinger <jochen@chromium.org>
Require explicit user action to override the policy URL on form submissions.
CONSOLE MESSAGE: line 20: window 1 ok
CONSOLE MESSAGE: line 26: window 2 ok
<html>
<head>
<script type="text/javascript">
if (window.layoutTestController)
layoutTestController.setCanOpenWindows();
var ga = document.createElement('script');
ga.type = 'text/javascript';
ga.async = true;
ga.src = 'script.js';
var s = document.getElementsByTagName('script')[0];
s.parentNode.insertBefore(ga, s);
</script>
<script src="../inspector-test.js"></script>
<script type="text/javascript">
function openWindow1()
{
window.open(window.location, "_blank");
console.log("window 1 ok");
}
function openWindow2()
{
window.open(window.location, "_blank");
console.log("window 2 ok");
}
function test()
{
if (!window.internals) {
console.log("This test cannot be run as window.internals is not available.");
return;
}
NetworkAgent.setCacheDisabled(true, step1);
internals.disableMemoryCache(true);
function step1(msg)
{
InspectorTest.addSniffer(WebInspector.ConsoleView.prototype, "addMessage", step2);
InspectorTest.evaluateInPage("openWindow1()");
}
function step2(msg)
{
InspectorTest.addSniffer(WebInspector.ConsoleView.prototype, "addMessage", step3);
InspectorTest.evaluateInPage("openWindow2()");
}
function step3(msg)
{
NetworkAgent.setCacheDisabled(false, step4);
}
function step4(msg)
{
internals.disableMemoryCache(false);
InspectorTest.completeTest();
}
}
</script>
<head>
<body onload="runTest()">
</body>
</html>
......@@ -1460,6 +1460,7 @@ http/tests/inspector/network/network-size.html
# https://bugs.webkit.org/show_bug.cgi?id=64097
http/tests/inspector/network/network-disable-cache-memory.html
http/tests/inspector/network/network-disable-cache-xhrs.html
http/tests/inspector/network/disabled-cache-crash.html
# https://bugs.webkit.org/show_bug.cgi?id=61437
http/tests/inspector/network/network-clear-after-disabled.html
......
......@@ -332,6 +332,7 @@ http/tests/inspector/network/network-size-sync.html
# https://bugs.webkit.org/show_bug.cgi?id=64097
http/tests/inspector/network/network-disable-cache-memory.html
http/tests/inspector/network/network-disable-cache-xhrs.html
http/tests/inspector/network/disabled-cache-crash.html
# https://bugs.webkit.org/show_bug.cgi?id=58515
compositing/overflow/clip-content-under-overflow-controls.html
......
......@@ -1857,6 +1857,7 @@ http/tests/inspector/network/network-size-sync.html
# https://bugs.webkit.org/show_bug.cgi?id=64097
http/tests/inspector/network/network-disable-cache-memory.html
http/tests/inspector/network/network-disable-cache-xhrs.html
http/tests/inspector/network/disabled-cache-crash.html
# [Qt] media/video-playbackrate.html fails
# https://bugs.webkit.org/show_bug.cgi?id=57476
......
......@@ -55,6 +55,7 @@ http/tests/inspector/network/network-size-sync.html
# https://bugs.webkit.org/show_bug.cgi?id=64097
http/tests/inspector/network/network-disable-cache-memory.html
http/tests/inspector/network/network-disable-cache-xhrs.html
http/tests/inspector/network/disabled-cache-crash.html
# Fails <rdar://problem/5674289>
media/video-seek-past-end-paused.html
......
2011-08-01 Scott Graham <scottmg@chromium.org>
REGRESSION (r39725?): Resources removed from document can not be freed
until the document is deleted
https://bugs.webkit.org/show_bug.cgi?id=61006
Reviewed by Antti Koivisto.
Upon completing a load start a Timer to iterate through
CachedResourceLoader's m_documentResources map to check for any items
that have only one reference (thus being the reference in the map
itself). The map should really be weak, but because the
CachedResourceHandle achieves bookkeeping work in addition to
reference counting, this is a simpler and more localized way to free
the used memory while maintaining the other behaviour (when
CachedResource is used as proxy).
With this patch the testcase at
https://bugs.webkit.org/attachment.cgi?id=93850 should no longer
consume 400MB of ram on load. Test added for crash discovered in
previous revision, but no tests for memory usage.
Test: http/tests/inspector/network/disabled-cache-crash.html
* WebCore.exp.in:
* loader/cache/CachedResource.h:
(WebCore::CachedResource::hasOneHandle):
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::CachedResourceLoader):
(WebCore::CachedResourceLoader::loadDone):
(WebCore::CachedResourceLoader::garbageCollectDocumentResourcesTimerFired):
* loader/cache/CachedResourceLoader.h:
* testing/Internals.cpp:
(WebCore::Internals::disableMemoryCache):
* testing/Internals.h:
* testing/Internals.idl:
2011-08-01 Jochen Eisinger <jochen@chromium.org>
Never override the policy URL on form submissions.
......@@ -217,6 +217,7 @@ __ZN7WebCore11MemoryCache13setCapacitiesEjjj
__ZN7WebCore11MemoryCache14evictResourcesEv
__ZN7WebCore11MemoryCache19getOriginsWithCacheERN3WTF7HashSetINS1_6RefPtrINS_14SecurityOriginEEENS_18SecurityOriginHashENS1_10HashTraitsIS5_EEEE
__ZN7WebCore11MemoryCache25removeResourcesWithOriginEPNS_14SecurityOriginE
__ZN7WebCore11MemoryCache11setDisabledEb
__ZN7WebCore11RenderLayer19scrollRectToVisibleERKNS_7IntRectERKNS_15ScrollAlignmentES6_
__ZN7WebCore11globalPointERK8_NSPointP8NSWindow
__ZN7WebCore11memoryCacheEv
......
......@@ -182,6 +182,7 @@ public:
CachedMetadata* cachedMetadata(unsigned dataTypeID) const;
bool canDelete() const { return !hasClients() && !m_request && !m_preloadCount && !m_handleCount && !m_resourceToRevalidate && !m_proxyResource; }
bool hasOneHandle() const { return m_handleCount == 1; }
bool isExpired() const;
......
......@@ -86,6 +86,7 @@ static CachedResource* createResource(CachedResource::Type type, ResourceRequest
CachedResourceLoader::CachedResourceLoader(Document* document)
: m_document(document)
, m_requestCount(0)
, m_garbageCollectDocumentResourcesTimer(this, &CachedResourceLoader::garbageCollectDocumentResourcesTimerFired)
, m_autoLoadImages(true)
, m_loadFinishing(false)
, m_allowStaleResources(false)
......@@ -572,6 +573,33 @@ void CachedResourceLoader::loadDone()
if (frame())
frame()->loader()->loadDone();
performPostLoadActions();
if (!m_garbageCollectDocumentResourcesTimer.isActive())
m_garbageCollectDocumentResourcesTimer.startOneShot(0);
}
// Garbage collecting m_documentResources is a workaround for the
// CachedResourceHandles on the RHS being strong references. Ideally this
// would be a weak map, however CachedResourceHandles perform additional
// bookkeeping on CachedResources, so instead pseudo-GC them -- when the
// reference count reaches 1, m_documentResources is the only reference, so
// remove it from the map.
void CachedResourceLoader::garbageCollectDocumentResourcesTimerFired(Timer<CachedResourceLoader>* timer)
{
ASSERT_UNUSED(timer, timer == &m_garbageCollectDocumentResourcesTimer);
typedef Vector<String, 10> StringVector;
StringVector resourcesToDelete;
for (DocumentResourceMap::iterator it = m_documentResources.begin(); it != m_documentResources.end(); ++it) {
if (it->second->hasOneHandle()) {
resourcesToDelete.append(it->first);
it->second->setOwningCachedResourceLoader(0);
}
}
for (StringVector::const_iterator it = resourcesToDelete.begin(); it != resourcesToDelete.end(); ++it)
m_documentResources.remove(*it);
}
void CachedResourceLoader::performPostLoadActions()
......
......@@ -30,6 +30,7 @@
#include "CachedResourceHandle.h"
#include "CachePolicy.h"
#include "ResourceLoadPriority.h"
#include "Timer.h"
#include <wtf/Deque.h>
#include <wtf/HashMap.h>
#include <wtf/HashSet.h>
......@@ -117,6 +118,7 @@ private:
void notifyLoadedFromMemoryCache(CachedResource*);
bool canRequest(CachedResource::Type, const KURL&, bool forPreload = false);
void garbageCollectDocumentResourcesTimerFired(Timer<CachedResourceLoader>*);
void performPostLoadActions();
HashSet<String> m_validatedURLs;
......@@ -133,6 +135,8 @@ private:
};
Deque<PendingPreload> m_pendingPreloads;
Timer<CachedResourceLoader> m_garbageCollectDocumentResourcesTimer;
//29 bits left
bool m_autoLoadImages : 1;
bool m_loadFinishing : 1;
......
......@@ -32,6 +32,7 @@
#include "Element.h"
#include "ExceptionCode.h"
#include "InspectorController.h"
#include "MemoryCache.h"
#include "NodeRenderingContext.h"
#include "Page.h"
#include "RenderObject.h"
......@@ -147,6 +148,11 @@ String Internals::shadowPseudoId(Element* element, ExceptionCode& ec)
return element->shadowPseudoId().string();
}
void Internals::disableMemoryCache(bool disabled)
{
WebCore::memoryCache()->setDisabled(disabled);
}
#if ENABLE(INSPECTOR)
void Internals::setInspectorResourcesDataSizeLimits(Document* document, int maximumResourcesContentSize, int maximumSingleResourceContentSize, ExceptionCode& ec)
{
......
......@@ -55,6 +55,7 @@ public:
String shadowPseudoId(Element*, ExceptionCode&);
PassRefPtr<Element> createShadowContentElement(Document*, ExceptionCode&);
Element* getElementByIdInShadowRoot(Node* shadowRoot, const String& id, ExceptionCode&);
void disableMemoryCache(bool disabled);
#if ENABLE(INSPECTOR)
void setInspectorResourcesDataSizeLimits(Document*, int maximumResourcesContentSize, int maximumSingleResourceContentSize, ExceptionCode&);
......
......@@ -37,6 +37,7 @@ module window {
DOMString shadowPseudoId(in Element element) raises (DOMException);
Element createShadowContentElement(in Document document) raises(DOMException);
Element getElementByIdInShadowRoot(in Node shadowRoot, in DOMString id) raises(DOMException);
void disableMemoryCache(in boolean disabled);
void setInspectorResourcesDataSizeLimits(in Document document, in long maximumResourcesContentSize, in long maximumSingleResourceContentSize) raises(DOMException);
......
2011-08-01 Scott Graham <scottmg@chromium.org>
REGRESSION (r39725?): Resources removed from document can not be freed until the document is deleted
https://bugs.webkit.org/show_bug.cgi?id=61006
Reviewed by Antti Koivisto.
Update exports for test harness.
* win/WebKit2.def:
* win/WebKit2CFLite.def:
2011-08-01 Hayato Ito <hayato@chromium.org>
Add support for getting an element in shadow root by its id into a window.internals object.
......
......@@ -154,8 +154,10 @@ EXPORTS
?getElementById@TreeScope@WebCore@@QBEPAVElement@2@ABVAtomicString@WTF@@@Z
?isPreloaded@CachedResourceLoader@WebCore@@QBE_NABVString@WTF@@@Z
?jsStringSlowCase@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@AAV?$HashMap@PAVStringImpl@WTF@@V?$Weak@VJSString@JSC@@@JSC@@UStringHash@2@U?$HashTraits@PAVStringImpl@WTF@@@2@U?$HashTraits@V?$Weak@VJSString@JSC@@@JSC@@@2@@WTF@@PAVStringImpl@6@@Z
?memoryCache@WebCore@@YAPAVMemoryCache@1@XZ
?page@Document@WebCore@@QBEPAVPage@2@XZ
?removeShadowRoot@Element@WebCore@@QAEXXZ
?setDisabled@MemoryCache@WebCore@@QAEX_N@Z
?setDOMException@WebCore@@YAXPAVExecState@JSC@@H@Z
?setResourcesDataSizeLimitsFromInternals@InspectorController@WebCore@@QAEXHH@Z
?shadowRoot@Element@WebCore@@QBEPAVShadowRoot@2@XZ
......
......@@ -148,8 +148,10 @@ EXPORTS
?toJS@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@PAVJSDOMGlobalObject@1@PAVClientRect@1@@Z
?updateLayoutIgnorePendingStylesheets@Document@WebCore@@QAEXXZ
?jsStringSlowCase@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@AAV?$HashMap@PAVStringImpl@WTF@@V?$Weak@VJSString@JSC@@@JSC@@UStringHash@2@U?$HashTraits@PAVStringImpl@WTF@@@2@U?$HashTraits@V?$Weak@VJSString@JSC@@@JSC@@@2@@WTF@@PAVStringImpl@6@@Z
?memoryCache@WebCore@@YAPAVMemoryCache@1@XZ
?page@Document@WebCore@@QBEPAVPage@2@XZ
?removeShadowRoot@Element@WebCore@@QAEXXZ
?setDisabled@MemoryCache@WebCore@@QAEX_N@Z
?setDOMException@WebCore@@YAXPAVExecState@JSC@@H@Z
?setResourcesDataSizeLimitsFromInternals@InspectorController@WebCore@@QAEXHH@Z
?shadowRoot@Element@WebCore@@QBEPAVShadowRoot@2@XZ
......
......@@ -35,6 +35,8 @@ _ZN7WebCore10ClientRectC1Ev;
_ZN7WebCore10ClientRectC1ERKNS_7IntRectE;
_ZN7WebCore11EventTarget17toGeneratedStreamEv;
_ZN7WebCore11EventTarget8toStreamEv;
_ZN7WebCore11MemoryCache11setDisabledEb;
_ZN7WebCore11memoryCacheEv;
_ZN7WebCore12JSDOMWrapper34virtualFunctionToPreventWeakVtableEv;
_ZN7WebCore12RenderObject23absoluteBoundingBoxRectEb;
_ZN7WebCore13createWrapperEPN3JSC9ExecStateEPNS_17JSDOMGlobalObjectEPNS_4NodeE;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment