⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 92143 in webkit


Ignore:
Timestamp:
Aug 1, 2011, 2:22:08 PM (15 years ago)
Author:
commit-queue@webkit.org
Message:

.: 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:
Location:
trunk
Files:
2 added
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/ChangeLog

    r92124 r92143  
     12011-08-01  Scott Graham  <scottmg@chromium.org>
     2
     3        REGRESSION (r39725?): Resources removed from document can not be freed until the document is deleted
     4        https://bugs.webkit.org/show_bug.cgi?id=61006
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Update exports for test harness.
     9
     10        * Source/autotools/symbols.filter:
     11
    1122011-08-01  Hayato Ito  <hayato@chromium.org>
    213

  • trunk/LayoutTests/ChangeLog

    r92142 r92143  
     12011-08-01  Scott Graham  <scottmg@chromium.org>
     2
     3        https://bugs.webkit.org/show_bug.cgi?id=61006
     4
     5        Test for CachedResourceLoader. Not caused by cache-disabling but very
     6        difficult to reproduce when cache is active, so use cache disable in
     7        inspector to exercise code.
     8
     9        Reviewed by Antti Koivisto.
     10
     11        * http/tests/inspector/network/disabled-cache-crash-expected.txt: Added.
     12        * http/tests/inspector/network/disabled-cache-crash.html: Added.
     13        * platform/gtk/Skipped:
     14        * platform/mac/Skipped:
     15        * platform/qt/Skipped:
     16        * platform/win/Skipped:
     17
    1182011-08-01  Jochen Eisinger  <jochen@chromium.org>
    219

  • trunk/LayoutTests/platform/gtk/Skipped

    r92141 r92143  
    14611461http/tests/inspector/network/network-disable-cache-memory.html
    14621462http/tests/inspector/network/network-disable-cache-xhrs.html
     1463http/tests/inspector/network/disabled-cache-crash.html
    14631464
    14641465# https://bugs.webkit.org/show_bug.cgi?id=61437

  • trunk/LayoutTests/platform/mac/Skipped

    r92141 r92143  
    333333http/tests/inspector/network/network-disable-cache-memory.html
    334334http/tests/inspector/network/network-disable-cache-xhrs.html
     335http/tests/inspector/network/disabled-cache-crash.html
    335336
    336337# https://bugs.webkit.org/show_bug.cgi?id=58515

  • trunk/LayoutTests/platform/qt/Skipped

    r92141 r92143  
    18581858http/tests/inspector/network/network-disable-cache-memory.html
    18591859http/tests/inspector/network/network-disable-cache-xhrs.html
     1860http/tests/inspector/network/disabled-cache-crash.html
    18601861
    18611862# [Qt] media/video-playbackrate.html fails

  • trunk/LayoutTests/platform/win/Skipped

    r92141 r92143  
    5656http/tests/inspector/network/network-disable-cache-memory.html
    5757http/tests/inspector/network/network-disable-cache-xhrs.html
     58http/tests/inspector/network/disabled-cache-crash.html
    5859
    5960# Fails <rdar://problem/5674289>

  • trunk/Source/WebCore/ChangeLog

    r92142 r92143  
     12011-08-01  Scott Graham  <scottmg@chromium.org>
     2
     3        REGRESSION (r39725?): Resources removed from document can not be freed
     4        until the document is deleted
     5        https://bugs.webkit.org/show_bug.cgi?id=61006
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Upon completing a load start a Timer to iterate through
     10        CachedResourceLoader's m_documentResources map to check for any items
     11        that have only one reference (thus being the reference in the map
     12        itself). The map should really be weak, but because the
     13        CachedResourceHandle achieves bookkeeping work in addition to
     14        reference counting, this is a simpler and more localized way to free
     15        the used memory while maintaining the other behaviour (when
     16        CachedResource is used as proxy).
     17
     18        With this patch the testcase at
     19        https://bugs.webkit.org/attachment.cgi?id=93850 should no longer
     20        consume 400MB of ram on load. Test added for crash discovered in
     21        previous revision, but no tests for memory usage.
     22
     23        Test: http/tests/inspector/network/disabled-cache-crash.html
     24
     25        * WebCore.exp.in:
     26        * loader/cache/CachedResource.h:
     27        (WebCore::CachedResource::hasOneHandle):
     28        * loader/cache/CachedResourceLoader.cpp:
     29        (WebCore::CachedResourceLoader::CachedResourceLoader):
     30        (WebCore::CachedResourceLoader::loadDone):
     31        (WebCore::CachedResourceLoader::garbageCollectDocumentResourcesTimerFired):
     32        * loader/cache/CachedResourceLoader.h:
     33        * testing/Internals.cpp:
     34        (WebCore::Internals::disableMemoryCache):
     35        * testing/Internals.h:
     36        * testing/Internals.idl:
     37
    1382011-08-01  Jochen Eisinger  <jochen@chromium.org>
    239

  • trunk/Source/WebCore/WebCore.exp.in

    r92124 r92143  
    218218__ZN7WebCore11MemoryCache19getOriginsWithCacheERN3WTF7HashSetINS1_6RefPtrINS_14SecurityOriginEEENS_18SecurityOriginHashENS1_10HashTraitsIS5_EEEE
    219219__ZN7WebCore11MemoryCache25removeResourcesWithOriginEPNS_14SecurityOriginE
     220__ZN7WebCore11MemoryCache11setDisabledEb
    220221__ZN7WebCore11RenderLayer19scrollRectToVisibleERKNS_7IntRectERKNS_15ScrollAlignmentES6_
    221222__ZN7WebCore11globalPointERK8_NSPointP8NSWindow

  • trunk/Source/WebCore/loader/cache/CachedResource.h

    r91725 r92143  
    183183
    184184    bool canDelete() const { return !hasClients() && !m_request && !m_preloadCount && !m_handleCount && !m_resourceToRevalidate && !m_proxyResource; }
     185    bool hasOneHandle() const { return m_handleCount == 1; }
    185186
    186187    bool isExpired() const;

  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

    r91725 r92143  
    8787    : m_document(document)
    8888    , m_requestCount(0)
     89    , m_garbageCollectDocumentResourcesTimer(this, &CachedResourceLoader::garbageCollectDocumentResourcesTimerFired)
    8990    , m_autoLoadImages(true)
    9091    , m_loadFinishing(false)
    … …  
    573574        frame()->loader()->loadDone();
    574575    performPostLoadActions();
     576
     577    if (!m_garbageCollectDocumentResourcesTimer.isActive())
     578        m_garbageCollectDocumentResourcesTimer.startOneShot(0);
     579}
     580
     581// Garbage collecting m_documentResources is a workaround for the
     582// CachedResourceHandles on the RHS being strong references. Ideally this
     583// would be a weak map, however CachedResourceHandles perform additional
     584// bookkeeping on CachedResources, so instead pseudo-GC them -- when the
     585// reference count reaches 1, m_documentResources is the only reference, so
     586// remove it from the map.
     587void CachedResourceLoader::garbageCollectDocumentResourcesTimerFired(Timer<CachedResourceLoader>* timer)
     588{
     589    ASSERT_UNUSED(timer, timer == &m_garbageCollectDocumentResourcesTimer);
     590
     591    typedef Vector<String, 10> StringVector;
     592    StringVector resourcesToDelete;
     593
     594    for (DocumentResourceMap::iterator it = m_documentResources.begin(); it != m_documentResources.end(); ++it) {
     595        if (it->second->hasOneHandle()) {
     596            resourcesToDelete.append(it->first);
     597            it->second->setOwningCachedResourceLoader(0);
     598        }
     599    }
     600
     601    for (StringVector::const_iterator it = resourcesToDelete.begin(); it != resourcesToDelete.end(); ++it)
     602        m_documentResources.remove(*it);
    575603}
    576604

  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

    r91725 r92143  
    3131#include "CachePolicy.h"
    3232#include "ResourceLoadPriority.h"
     33#include "Timer.h"
    3334#include <wtf/Deque.h>
    3435#include <wtf/HashMap.h>
    … …  
    118119    bool canRequest(CachedResource::Type, const KURL&, bool forPreload = false);
    119120
     121    void garbageCollectDocumentResourcesTimerFired(Timer<CachedResourceLoader>*);
    120122    void performPostLoadActions();
    121123   
    … …  
    133135    };
    134136    Deque<PendingPreload> m_pendingPreloads;
     137
     138    Timer<CachedResourceLoader> m_garbageCollectDocumentResourcesTimer;
    135139
    136140    //29 bits left

  • trunk/Source/WebCore/testing/Internals.cpp

    r92124 r92143  
    3333#include "ExceptionCode.h"
    3434#include "InspectorController.h"
     35#include "MemoryCache.h"
    3536#include "NodeRenderingContext.h"
    3637#include "Page.h"
    … …  
    148149}
    149150
     151void Internals::disableMemoryCache(bool disabled)
     152{
     153    WebCore::memoryCache()->setDisabled(disabled);
     154}
     155
    150156#if ENABLE(INSPECTOR)
    151157void Internals::setInspectorResourcesDataSizeLimits(Document* document, int maximumResourcesContentSize, int maximumSingleResourceContentSize, ExceptionCode& ec)

  • trunk/Source/WebCore/testing/Internals.h

    r92124 r92143  
    5656    PassRefPtr<Element> createShadowContentElement(Document*, ExceptionCode&);
    5757    Element* getElementByIdInShadowRoot(Node* shadowRoot, const String& id, ExceptionCode&);
     58    void disableMemoryCache(bool disabled);
    5859
    5960#if ENABLE(INSPECTOR)

  • trunk/Source/WebCore/testing/Internals.idl

    r92124 r92143  
    3838        Element createShadowContentElement(in Document document) raises(DOMException);
    3939        Element getElementByIdInShadowRoot(in Node shadowRoot, in DOMString id) raises(DOMException);
     40        void disableMemoryCache(in boolean disabled);
    4041
    4142        void setInspectorResourcesDataSizeLimits(in Document document, in long maximumResourcesContentSize, in long maximumSingleResourceContentSize) raises(DOMException);

  • trunk/Source/WebKit2/ChangeLog

    r92124 r92143  
     12011-08-01  Scott Graham  <scottmg@chromium.org>
     2
     3        REGRESSION (r39725?): Resources removed from document can not be freed until the document is deleted
     4        https://bugs.webkit.org/show_bug.cgi?id=61006
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Update exports for test harness.
     9
     10        * win/WebKit2.def:
     11        * win/WebKit2CFLite.def:
     12
    1132011-08-01  Hayato Ito  <hayato@chromium.org>
    214

  • trunk/Source/WebKit2/win/WebKit2.def

    r92124 r92143  
    155155        ?isPreloaded@CachedResourceLoader@WebCore@@QBE_NABVString@WTF@@@Z
    156156        ?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
     157        ?memoryCache@WebCore@@YAPAVMemoryCache@1@XZ
    157158        ?page@Document@WebCore@@QBEPAVPage@2@XZ
    158159        ?removeShadowRoot@Element@WebCore@@QAEXXZ
     160        ?setDisabled@MemoryCache@WebCore@@QAEX_N@Z
    159161        ?setDOMException@WebCore@@YAXPAVExecState@JSC@@H@Z
    160162        ?setResourcesDataSizeLimitsFromInternals@InspectorController@WebCore@@QAEXHH@Z

  • trunk/Source/WebKit2/win/WebKit2CFLite.def

    r92124 r92143  
    149149        ?updateLayoutIgnorePendingStylesheets@Document@WebCore@@QAEXXZ
    150150        ?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
     151        ?memoryCache@WebCore@@YAPAVMemoryCache@1@XZ
    151152        ?page@Document@WebCore@@QBEPAVPage@2@XZ
    152153        ?removeShadowRoot@Element@WebCore@@QAEXXZ
     154        ?setDisabled@MemoryCache@WebCore@@QAEX_N@Z
    153155        ?setDOMException@WebCore@@YAXPAVExecState@JSC@@H@Z
    154156        ?setResourcesDataSizeLimitsFromInternals@InspectorController@WebCore@@QAEXHH@Z

  • trunk/Source/autotools/symbols.filter

    r92124 r92143  
    3636_ZN7WebCore11EventTarget17toGeneratedStreamEv;
    3737_ZN7WebCore11EventTarget8toStreamEv;
     38_ZN7WebCore11MemoryCache11setDisabledEb;
     39_ZN7WebCore11memoryCacheEv;
    3840_ZN7WebCore12JSDOMWrapper34virtualFunctionToPreventWeakVtableEv;
    3941_ZN7WebCore12RenderObject23absoluteBoundingBoxRectEb;
Note: See TracChangeset for help on using the changeset viewer.