Changeset 74107 in webkit


Ignore:
Timestamp:
Dec 15, 2010 3:27:52 AM (13 years ago)
Author:
Antti Koivisto
Message:

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

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r74101 r74107  
     12010-12-14  Antti Koivisto  <antti@apple.com>
     2
     3        Reviewed by Alexey Proskuryakov.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=49548
     6        WebCore cache stores duplicate copies of subresources with URL fragments
     7
     8        * http/tests/cache/subresource-fragment-identifier-expected.txt: Added.
     9        * http/tests/cache/subresource-fragment-identifier.html: Added.
     10
    1112010-12-15  Emil Eklund  <eae@chromium.org>
    212
  • trunk/WebCore/ChangeLog

    r74106 r74107  
     12010-12-14  Antti Koivisto  <antti@apple.com>
     2
     3        Reviewed by Alexey Proskuryakov.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=49548
     6        WebCore cache stores duplicate copies of subresources with URL fragments
     7       
     8        - Strip fragment identifiers from HTTP and file URLs for the memory cache.
     9        - Changed some CachedResourceLoader and MemoryCache interfaces to use KURLs
     10          instead of strings to reduce repeated URL parsing.
     11
     12        Test: http/tests/cache/subresource-fragment-identifier.html
     13
     14        * inspector/InspectorResourceAgent.cpp:
     15        (WebCore::InspectorResourceAgent::cachedResource):
     16        * loader/FrameLoader.cpp:
     17        (WebCore::FrameLoader::tellClientAboutPastMemoryCacheLoads):
     18        * loader/cache/CachedResource.cpp:
     19        (WebCore::CachedResource::~CachedResource):
     20        * loader/cache/CachedResourceLoader.cpp:
     21        (WebCore::CachedResourceLoader::cachedResource):
     22        (WebCore::CachedResourceLoader::checkForReload):
     23        (WebCore::CachedResourceLoader::requestUserCSSStyleSheet):
     24        (WebCore::CachedResourceLoader::requestResource):
     25        * loader/cache/CachedResourceLoader.h:
     26        * loader/cache/MemoryCache.cpp:
     27        (WebCore::MemoryCache::requestResource):
     28        (WebCore::MemoryCache::requestUserCSSStyleSheet):
     29        (WebCore::MemoryCache::removeFragmentIdentifierIfNeeded):
     30        (WebCore::MemoryCache::resourceForURL):
     31        * loader/cache/MemoryCache.h:
     32
    1332010-12-15  Anton Muhin  <antonm@chromium.org>
    234
  • trunk/WebCore/inspector/InspectorResourceAgent.cpp

    r73607 r74107  
    131131CachedResource* InspectorResourceAgent::cachedResource(Frame* frame, const KURL& url)
    132132{
    133     const String& urlString = url.string();
    134     CachedResource* cachedResource = frame->document()->cachedResourceLoader()->cachedResource(urlString);
     133    CachedResource* cachedResource = frame->document()->cachedResourceLoader()->cachedResource(url);
    135134    if (!cachedResource)
    136         cachedResource = cache()->resourceForURL(urlString);
     135        cachedResource = cache()->resourceForURL(url);
    137136    return cachedResource;
    138137}
  • trunk/WebCore/loader/FrameLoader.cpp

    r73749 r74107  
    34223422    size_t size = pastLoads.size();
    34233423    for (size_t i = 0; i < size; ++i) {
    3424         CachedResource* resource = cache()->resourceForURL(pastLoads[i]);
     3424        CachedResource* resource = cache()->resourceForURL(KURL(ParsedURLString, pastLoads[i]));
    34253425
    34263426        // FIXME: These loads, loaded from cache, but now gone from the cache by the time
  • trunk/WebCore/loader/cache/CachedResource.cpp

    r74049 r74107  
    117117    ASSERT(!inCache());
    118118    ASSERT(!m_deleted);
    119     ASSERT(url().isNull() || cache()->resourceForURL(url()) != this);
     119    ASSERT(url().isNull() || cache()->resourceForURL(KURL(ParsedURLString, url())) != this);
    120120#ifndef NDEBUG
    121121    m_deleted = true;
  • trunk/WebCore/loader/cache/CachedResourceLoader.cpp

    r74049 r74107  
    7474}
    7575
     76CachedResource* CachedResourceLoader::cachedResource(const String& resourceURL) const
     77{
     78    KURL url = m_document->completeURL(resourceURL);
     79    return cachedResource(url);
     80}
     81
     82CachedResource* CachedResourceLoader::cachedResource(const KURL& resourceURL) const
     83{
     84    KURL url = MemoryCache::removeFragmentIdentifierIfNeeded(resourceURL);
     85    return m_documentResources.get(url).get();
     86}
     87
    7688Frame* CachedResourceLoader::frame() const
    7789{
     
    90102        return;
    91103   
    92     CachedResource* existing = cache()->resourceForURL(fullURL.string());
     104    CachedResource* existing = cache()->resourceForURL(fullURL);
    93105    if (!existing || existing->isPreloaded())
    94106        return;
     
    152164CachedCSSStyleSheet* CachedResourceLoader::requestUserCSSStyleSheet(const String& url, const String& charset)
    153165{
    154     return cache()->requestUserCSSStyleSheet(this, url, charset);
     166    return cache()->requestUserCSSStyleSheet(this, KURL(KURL(), url), charset);
    155167}
    156168
     
    246258{
    247259    KURL fullURL = m_document->completeURL(url);
     260   
     261    // If only the fragment identifiers differ, it is the same resource.
     262    fullURL = MemoryCache::removeFragmentIdentifierIfNeeded(fullURL);
    248263
    249264    if (!fullURL.isValid() || !canRequest(type, fullURL))
  • trunk/WebCore/loader/cache/CachedResourceLoader.h

    r74049 r74107  
    7474    void printAccessDeniedMessage(const KURL& url) const;
    7575
    76     CachedResource* cachedResource(const String& url) const { return m_documentResources.get(url).get(); }
     76    CachedResource* cachedResource(const String& url) const;
     77    CachedResource* cachedResource(const KURL& url) const;
    7778   
    7879    typedef HashMap<String, CachedResourceHandle<CachedResource> > DocumentResourceMap;
  • trunk/WebCore/loader/cache/MemoryCache.cpp

    r73938 r74107  
    9696}
    9797
    98 CachedResource* MemoryCache::requestResource(CachedResourceLoader* cachedResourceLoader, CachedResource::Type type, const KURL& url, const String& charset, ResourceLoadPriority priority, bool requestIsPreload, bool forHistory)
    99 {
    100     LOG(ResourceLoading, "MemoryCache::requestResource '%s', charset '%s', priority=%d, preload=%u, forHistory=%u", url.string().latin1().data(), charset.latin1().data(), priority, requestIsPreload, forHistory);
     98CachedResource* MemoryCache::requestResource(CachedResourceLoader* cachedResourceLoader, CachedResource::Type type, const KURL& requestURL, const String& charset, ResourceLoadPriority priority, bool requestIsPreload, bool forHistory)
     99{
     100    LOG(ResourceLoading, "MemoryCache::requestResource '%s', charset '%s', priority=%d, preload=%u, forHistory=%u", requestURL.string().latin1().data(), charset.latin1().data(), priority, requestIsPreload, forHistory);
    101101   
    102102    // FIXME: Do we really need to special-case an empty URL?
    103103    // Would it be better to just go on with the cache code and let it fail later?
    104     if (url.isEmpty())
     104    if (requestURL.isEmpty())
    105105        return 0;
     106   
     107    // Ensure this is the pure primary resource URL.
     108    KURL url = removeFragmentIdentifierIfNeeded(requestURL);
    106109
    107110    // Look up the resource in our map.
    108     CachedResource* resource = resourceForURL(url.string());
     111    CachedResource* resource = resourceForURL(url);
    109112
    110113    // Non https "no-store" resources are left in the cache to be used for back/forward navigation only.
     
    185188}
    186189   
    187 CachedCSSStyleSheet* MemoryCache::requestUserCSSStyleSheet(CachedResourceLoader* cachedResourceLoader, const String& url, const String& charset)
    188 {
     190CachedCSSStyleSheet* MemoryCache::requestUserCSSStyleSheet(CachedResourceLoader* cachedResourceLoader, const KURL& requestURL, const String& charset)
     191{
     192    // Ensure this is the pure primary resource URL.
     193    KURL url = removeFragmentIdentifierIfNeeded(requestURL);
     194
    189195    CachedCSSStyleSheet* userSheet;
    190196    if (CachedResource* existing = resourceForURL(url)) {
     
    213219    return userSheet;
    214220}
    215    
     221
     222KURL MemoryCache::removeFragmentIdentifierIfNeeded(const KURL& originalURL)
     223{
     224    if (!originalURL.hasFragmentIdentifier())
     225        return originalURL;
     226    // Strip away fragment identifier from HTTP and file urls.
     227    // Data urls must be unmodified and it is also safer to keep them for custom protocols.
     228    if (!(originalURL.protocolInHTTPFamily() || originalURL.isLocalFile()))
     229        return originalURL;
     230    KURL url = originalURL;
     231    url.removeFragmentIdentifier();
     232    return url;
     233}
     234
    216235void MemoryCache::revalidateResource(CachedResource* resource, CachedResourceLoader* cachedResourceLoader)
    217236{
     
    270289}
    271290
    272 CachedResource* MemoryCache::resourceForURL(const String& url)
    273 {
     291CachedResource* MemoryCache::resourceForURL(const KURL& resourceURL)
     292{
     293    KURL url = removeFragmentIdentifierIfNeeded(resourceURL);
    274294    CachedResource* resource = m_resources.get(url);
    275295    bool wasPurgeable = MemoryCache::shouldMakeResourcePurgeableOnEviction() && resource && resource->isPurgeable();
  • trunk/WebCore/loader/cache/MemoryCache.h

    r73938 r74107  
    108108    CachedResource* requestResource(CachedResourceLoader*, CachedResource::Type, const KURL& url, const String& charset, ResourceLoadPriority, bool isPreload = false, bool forHistory = false);
    109109
    110     CachedCSSStyleSheet* requestUserCSSStyleSheet(CachedResourceLoader*, const String& url, const String& charset);
     110    CachedCSSStyleSheet* requestUserCSSStyleSheet(CachedResourceLoader*, const KURL& url, const String& charset);
     111   
     112    static KURL removeFragmentIdentifierIfNeeded(const KURL& originalURL);
    111113   
    112114    void revalidateResource(CachedResource*, CachedResourceLoader*);
     
    145147    void removeCachedResourceLoader(CachedResourceLoader*);
    146148
    147     CachedResource* resourceForURL(const String&);
     149    CachedResource* resourceForURL(const KURL&);
    148150
    149151    // Calls to put the cached resource into and out of LRU lists.
Note: See TracChangeset for help on using the changeset viewer.