Changeset 207459 in webkit


Ignore:
Timestamp:
Oct 18, 2016 5:30:40 AM (8 years ago)
Author:
commit-queue@webkit.org
Message:

CachedResourceLoader should not need to remove fragment identifier
​https://bugs.webkit.org/show_bug.cgi?id=163015

Patch by Youenn Fablet <​youenn@apple.com> on 2016-10-18
Reviewed by Darin Adler.

No expected change for non-window port.
For window port, CachedResourceLoader will strip the fragment identifier of the URL passed to subresourceForURL
before querying the memory cache.

Removing the fragment identifier from the request stored in CachedResourceRequest.
The fragment identifier is stored in a separate field.

This allows CachedResourceLoader to not care about fragment identifier.
CachedResource can then get access to it.

  • loader/cache/CachedResource.cpp:

(WebCore::CachedResource::CachedResource):
(WebCore::CachedResource::finishRequestInitialization): Deleted.

  • loader/cache/CachedResource.h:
  • loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::cachedResource):
Updated the method taking a const String& to strip the fragment identifier if needed.
Updated the method taking a const URL& to assert if the fragment identifier is present.
(WebCore::CachedResourceLoader::requestUserCSSStyleSheet):
(WebCore::CachedResourceLoader::requestResource):

  • loader/cache/CachedResourceRequest.cpp:

(WebCore::CachedResourceRequest::CachedResourceRequest):
(WebCore::CachedResourceRequest::splitFragmentIdentifierFromRequestURL):

  • loader/cache/CachedResourceRequest.h:

(WebCore::CachedResourceRequest::releaseFragmentIdentifier):
(WebCore::CachedResourceRequest::clearFragmentIdentifier):

  • loader/cache/MemoryCache.cpp:

(WebCore::MemoryCache::shouldRemoveFragmentIdentifier):
(WebCore::MemoryCache::removeFragmentIdentifierIfNeeded):
(WebCore::MemoryCache::revalidationSucceeded):
(WebCore::MemoryCache::resourceForRequest):

  • loader/cache/MemoryCache.h:
Location:
trunk/Source/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r207458 r207459  
     12016-10-18  Youenn Fablet  <youenn@apple.com>
     2
     3        CachedResourceLoader should not need to remove fragment identifier
     4        https://bugs.webkit.org/show_bug.cgi?id=163015
     5
     6        Reviewed by Darin Adler.
     7
     8        No expected change for non-window port.
     9        For window port, CachedResourceLoader will strip the fragment identifier of the URL passed to subresourceForURL
     10        before querying the memory cache.
     11
     12        Removing the fragment identifier from the request stored in CachedResourceRequest.
     13        The fragment identifier is stored in a separate field.
     14
     15        This allows CachedResourceLoader to not care about fragment identifier.
     16        CachedResource can then get access to it.
     17
     18        * loader/cache/CachedResource.cpp:
     19        (WebCore::CachedResource::CachedResource):
     20        (WebCore::CachedResource::finishRequestInitialization): Deleted.
     21        * loader/cache/CachedResource.h:
     22        * loader/cache/CachedResourceLoader.cpp:
     23        (WebCore::CachedResourceLoader::cachedResource):
     24        Updated the method taking a const String& to strip the fragment identifier if needed.
     25        Updated the method taking a const URL& to assert if the fragment identifier is present.
     26        (WebCore::CachedResourceLoader::requestUserCSSStyleSheet):
     27        (WebCore::CachedResourceLoader::requestResource):
     28        * loader/cache/CachedResourceRequest.cpp:
     29        (WebCore::CachedResourceRequest::CachedResourceRequest):
     30        (WebCore::CachedResourceRequest::splitFragmentIdentifierFromRequestURL):
     31        * loader/cache/CachedResourceRequest.h:
     32        (WebCore::CachedResourceRequest::releaseFragmentIdentifier):
     33        (WebCore::CachedResourceRequest::clearFragmentIdentifier):
     34        * loader/cache/MemoryCache.cpp:
     35        (WebCore::MemoryCache::shouldRemoveFragmentIdentifier):
     36        (WebCore::MemoryCache::removeFragmentIdentifierIfNeeded):
     37        (WebCore::MemoryCache::revalidationSucceeded):
     38        (WebCore::MemoryCache::resourceForRequest):
     39        * loader/cache/MemoryCache.h:
     40
    1412016-10-18  Antti Koivisto  <antti@apple.com>
    242

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

    r207281 r207459  
    122122    , m_loadPriority(defaultPriorityForResourceType(type))
    123123    , m_responseTimestamp(std::chrono::system_clock::now())
     124    , m_fragmentIdentifierForRequest(request.releaseFragmentIdentifier())
    124125    , m_origin(request.releaseOrigin())
    125126    , m_type(type)
    … …  
    128129
    129130    setLoadPriority(request.priority());
    130     finishRequestInitialization();
     131#ifndef NDEBUG
     132    cachedResourceLeakCounter.increment();
     133#endif
    131134
    132135    // FIXME: We should have a better way of checking for Navigation loads, maybe FetchMode::Options::Navigate.
    … …  
    139142}
    140143
     144// FIXME: For this constructor, we should probably mandate that the URL has no fragment identifier.
    141145CachedResource::CachedResource(const URL& url, Type type, SessionID sessionID)
    142146    : m_resourceRequest(url)
    … …  
    144148    , m_sessionID(sessionID)
    145149    , m_responseTimestamp(std::chrono::system_clock::now())
     150    , m_fragmentIdentifierForRequest(CachedResourceRequest::splitFragmentIdentifierFromRequestURL(m_resourceRequest))
    146151    , m_type(type)
    147152    , m_status(Cached)
    148153{
    149154    ASSERT(sessionID.isValid());
    150     finishRequestInitialization();
    151 }
    152 
    153 void CachedResource::finishRequestInitialization()
    154 {
    155155#ifndef NDEBUG
    156156    cachedResourceLeakCounter.increment();
    157157#endif
    158 
    159     if (!m_resourceRequest.url().hasFragmentIdentifier())
    160         return;
    161     URL urlForCache = MemoryCache::removeFragmentIdentifierIfNeeded(m_resourceRequest.url());
    162     if (urlForCache.hasFragmentIdentifier())
    163         return;
    164     m_fragmentIdentifierForRequest = m_resourceRequest.url().fragmentIdentifier();
    165     m_resourceRequest.setURL(urlForCache);
    166158}
    167159

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

    r206994 r207459  
    303303    class Callback;
    304304
    305     void finishRequestInitialization();
    306 
    307305    bool addClientToSet(CachedResourceClient&);
    308306

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

    r207331 r207459  
    149149}
    150150
    151 CachedResource* CachedResourceLoader::cachedResource(const String& resourceURL) const 
     151CachedResource* CachedResourceLoader::cachedResource(const String& resourceURL) const
    152152{
    153153    ASSERT(!resourceURL.isNull());
    154     URL url = m_document->completeURL(resourceURL);
    155     return cachedResource(url);
    156 }
    157 
    158 CachedResource* CachedResourceLoader::cachedResource(const URL& resourceURL) const
    159 {
    160     URL url = MemoryCache::removeFragmentIdentifierIfNeeded(resourceURL);
    161     return m_documentResources.get(url).get();
     154    return cachedResource(MemoryCache::removeFragmentIdentifierIfNeeded(m_document->completeURL(resourceURL)));
     155}
     156
     157CachedResource* CachedResourceLoader::cachedResource(const URL& url) const
     158{
     159    ASSERT(!MemoryCache::shouldRemoveFragmentIdentifier(url));
     160    return m_documentResources.get(url).get();
    162161}
    163162
    … …  
    657656
    658657    LOG(ResourceLoading, "CachedResourceLoader::requestResource '%s', charset '%s', priority=%d, forPreload=%u", url.stringCenterEllipsizedToLength().latin1().data(), request.charset().latin1().data(), request.priority() ? static_cast<int>(request.priority().value()) : -1, forPreload == ForPreload::Yes);
    659 
    660     // If only the fragment identifiers differ, it is the same resource.
    661     url = MemoryCache::removeFragmentIdentifierIfNeeded(url);
    662658
    663659    if (!url.isValid()) {

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

    r207330 r207459  
    4343    , m_options(options)
    4444    , m_priority(priority)
    45 {
     45    , m_fragmentIdentifier(splitFragmentIdentifierFromRequestURL(m_resourceRequest))
     46{
     47}
     48
     49String CachedResourceRequest::splitFragmentIdentifierFromRequestURL(ResourceRequest& request)
     50{
     51    if (!MemoryCache::shouldRemoveFragmentIdentifier(request.url()))
     52        return { };
     53    URL url = request.url();
     54    String fragmentIdentifier = url.fragmentIdentifier();
     55    url.removeFragmentIdentifier();
     56    request.setURL(url);
     57    return fragmentIdentifier;
    4658}
    4759

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

    r207281 r207459  
    7979    SecurityOrigin* origin() const { return m_origin.get(); }
    8080
     81    String&& releaseFragmentIdentifier() { return WTFMove(m_fragmentIdentifier); }
     82    void clearFragmentIdentifier() { m_fragmentIdentifier = { }; }
     83
     84    static String splitFragmentIdentifierFromRequestURL(ResourceRequest&);
     85
    8186private:
    8287    ResourceRequest m_resourceRequest;
    … …  
    8792    AtomicString m_initiatorName;
    8893    RefPtr<SecurityOrigin> m_origin;
     94    String m_fragmentIdentifier;
    8995};
    9096

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

    r206867 r207459  
    9090}
    9191
     92bool MemoryCache::shouldRemoveFragmentIdentifier(const URL& originalURL)
     93{
     94    if (!originalURL.hasFragmentIdentifier())
     95        return false;
     96    // Strip away fragment identifier from HTTP URLs.
     97    // Data URLs must be unmodified. For file and custom URLs clients may expect resources
     98    // to be unique even when they differ by the fragment identifier only.
     99    return originalURL.protocolIsInHTTPFamily();
     100}
     101
    92102URL MemoryCache::removeFragmentIdentifierIfNeeded(const URL& originalURL)
    93103{
    94     if (!originalURL.hasFragmentIdentifier())
    95         return originalURL;
    96     // Strip away fragment identifier from HTTP URLs.
    97     // Data URLs must be unmodified. For file and custom URLs clients may expect resources
    98     // to be unique even when they differ by the fragment identifier only.
    99     if (!originalURL.protocolIsInHTTPFamily())
     104    if (!shouldRemoveFragmentIdentifier(originalURL))
    100105        return originalURL;
    101106    URL url = originalURL;
    … …  
    155160    if (delta)
    156161        adjustSize(resource.hasClients(), delta);
    157    
     162
    158163    revalidatingResource.switchClientsToRevalidatedResource();
    159164    ASSERT(!revalidatingResource.m_deleted);
    … …  
    172177CachedResource* MemoryCache::resourceForRequest(const ResourceRequest& request, SessionID sessionID)
    173178{
     179    // FIXME: Change all clients to make sure HTTP(s) URLs have no fragment identifiers before calling here.
     180    // CachedResourceLoader is now doing this. Add an assertion once all other clients are doing it too.
    174181    auto* resources = sessionResourceMap(sessionID);
    175182    if (!resources)

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

    r206635 r207459  
    9898    void remove(CachedResource&);
    9999
    100     static URL removeFragmentIdentifierIfNeeded(const URL& originalURL);
    101    
     100    static bool shouldRemoveFragmentIdentifier(const URL&);
     101    static URL removeFragmentIdentifierIfNeeded(const URL&);
     102
    102103    void revalidationSucceeded(CachedResource& revalidatingResource, const ResourceResponse&);
    103104    void revalidationFailed(CachedResource& revalidatingResource);
    … …  
    106107    void forEachSessionResource(SessionID, const std::function<void(CachedResource&)>&);
    107108    void destroyDecodedDataForAllImages();
    108    
    109     // Sets the cache's memory capacities, in bytes. These will hold only approximately, 
     109
     110    // Sets the cache's memory capacities, in bytes. These will hold only approximately,
    110111    // since the decoded cost of resources like scripts and stylesheets is not known.
    111112    //  - minDeadBytes: The maximum number of bytes that dead resources should consume when the cache is under pressure.
    … …  
    121122    WEBCORE_EXPORT void evictResources();
    122123    WEBCORE_EXPORT void evictResources(SessionID);
    123    
     124
    124125    void prune();
    125126    void pruneSoon();
Note: See TracChangeset for help on using the changeset viewer.