Make ResourceFetcher::m_documentResources use WeakPtr

third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
     Copyright (C) 2009 Torch Mobile Inc. http://www.torchmobile.com/
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
@@ skipping 148 matching lines @@
         return WebURLRequest::RequestContextImage;
     case Resource::Media: // TODO: Split this.
         return WebURLRequest::RequestContextVideo;
     }
     ASSERT_NOT_REACHED();
     return WebURLRequest::RequestContextSubresource;
 }
 
 ResourceFetcher::ResourceFetcher(FetchContext* context)
     : m_context(context)
-    , m_garbageCollectDocumentResourcesTimer(this, &ResourceFetcher::garbageCollectDocumentResourcesTimerFired)
     , m_resourceTimingReportTimer(this, &ResourceFetcher::resourceTimingReportTimerFired)
     , m_autoLoadImages(true)
     , m_imagesEnabled(true)
     , m_allowStaleResources(false)
 {
 #if ENABLE(OILPAN)
     ThreadState::current()->registerPreFinalizer(this);
 #endif
 }
 
 ResourceFetcher::~ResourceFetcher()
 {
 #if !ENABLE(OILPAN)
     clearPreloads();
 #endif
 }
 
 WebTaskRunner* ResourceFetcher::loadingTaskRunner()
 {
     if (!m_context)
         return nullptr;
 
     return m_context->loadingTaskRunner();
 }
 
 Resource* ResourceFetcher::cachedResource(const KURL& resourceURL) const
 {
     KURL url = MemoryCache::removeFragmentIdentifierIfNeeded(resourceURL);
-    return m_documentResources.get(url).get();
+    const WeakPtrWillBeWeakMember<Resource>& resource = m_documentResources.get(url);

[Nate Chapin, 2015/12/14 23:12:16]

WeakMember is smarter than WeakPtr about HashMap return values and actually
returns a Resource* from get(url). I wasn't able to find a single-line return
statement that compiled both with and without Oilpan.

[sof, 2015/12/15 10:08:01]

Traits are in place to unwrap the WeakMember on get(); not worth #if'ing for,
the transition type will be removed soon enough.

+    return resource.get();
 }
 
 bool ResourceFetcher::canAccessResource(Resource* resource, SecurityOrigin* sourceOrigin, const KURL& url, AccessControlLoggingDecision logErrorsDecision) const
 {
     // Redirects can change the response URL different from one of request.
     bool forPreload = resource->isUnusedPreload();
     if (!context().canRequest(resource->type(), resource->resourceRequest(), url, resource->options(), forPreload, FetchRequest::UseDefaultOriginRestrictionForType))
         return false;
 
     if (!sourceOrigin)
@@ skipping 105 matching lines @@
     resource->setOptions(request.options());
     // FIXME: We should provide a body stream here.
     resource->responseReceived(response, nullptr);
     resource->setDataBufferingPolicy(BufferData);
     if (data->size())
         resource->setResourceBuffer(data);
     resource->setIdentifier(createUniqueIdentifier());
     resource->setCacheIdentifier(cacheIdentifier);
     resource->finish();
     memoryCache()->add(resource.get());
-    scheduleDocumentResourcesGC();
 }
 
 void ResourceFetcher::moveCachedNonBlockingResourceToBlocking(Resource* resource)
 {
     // TODO(yoav): Test that non-blocking resources (video/audio/track) continue to not-block even after being preloaded and discovered.
     if (resource && resource->loader() && resource->isNonBlockingResourceType() && resource->avoidBlockingOnLoad()) {
         if (m_nonBlockingLoaders)
             m_nonBlockingLoaders->remove(resource->loader());
         if (!m_loaders)
             m_loaders = ResourceLoaderSet::create();
@@ skipping 116 matching lines @@
     // Remove main resource from cache to prevent reuse.
     if (factory.type() == Resource::MainResource) {
         ASSERT(policy != Use || isStaticData);
         ASSERT(policy != Revalidate);
         memoryCache()->remove(resource.get());
     }
 
     requestLoadStarted(resource.get(), request, policy == Use ? ResourceLoadingFromCache : ResourceLoadingFromNetwork, isStaticData);
 
     ASSERT(resource->url() == url.string());
-    m_documentResources.set(resource->url(), resource);
+    m_documentResources.set(resource->url(), resource->asWeakPtr());
     return resource;
 }
 
 void ResourceFetcher::resourceTimingReportTimerFired(Timer<ResourceFetcher>* timer)
 {
     ASSERT_UNUSED(timer, timer == &m_resourceTimingReportTimer);
     Vector<OwnPtr<ResourceTimingInfo>> timingReports;
     timingReports.swap(m_scheduledResourceTimingReports);
     for (const auto& timingInfo : timingReports)
         context().addResourceTiming(*timingInfo);
@@ skipping 291 matching lines @@
     return !context().allowImage(m_imagesEnabled, url);
 }
 
 bool ResourceFetcher::shouldDeferImageLoad(const KURL& url) const
 {
     return clientDefersImage(url) || !m_autoLoadImages;
 }
 
 void ResourceFetcher::reloadImagesIfNotDeferred()
 {
     for (const auto& documentResource : m_documentResources) {

[sof, 2015/12/15 12:29:45]

This can be simplified some once we can assume Oilpan; add a TODO here also?

[Nate Chapin, 2015/12/15 20:30:59]

Done.

         Resource* resource = documentResource.value.get();
-        if (resource->type() == Resource::Image && resource->stillNeedsLoad() && !clientDefersImage(resource->url()))
+        if (resource && resource->type() == Resource::Image && resource->stillNeedsLoad() && !clientDefersImage(resource->url()))
             const_cast<Resource*>(resource)->load(this, defaultResourceOptions());
     }
 }
 
 void ResourceFetcher::redirectReceived(Resource* resource, const ResourceResponse& redirectResponse)
 {
     ResourceTimingInfoMap::iterator it = m_resourceTimingInfoMap.find(resource);
     if (it != m_resourceTimingInfoMap.end())
         it->value->addRedirect(redirectResponse);
 }
 
 void ResourceFetcher::didLoadResource()
 {
-    scheduleDocumentResourcesGC();
     context().didLoadResource();
 }
 
-void ResourceFetcher::scheduleDocumentResourcesGC()
-{
-    if (!m_garbageCollectDocumentResourcesTimer.isActive())
-        m_garbageCollectDocumentResourcesTimer.startOneShot(0, BLINK_FROM_HERE);
-}
-
-// Garbage collecting m_documentResources is a workaround for the
-// ResourcePtrs on the RHS being strong references. Ideally this
-// would be a weak map, however ResourcePtrs perform additional
-// bookkeeping on Resources, so instead pseudo-GC them -- when the
-// reference count reaches 1, m_documentResources is the only reference, so
-// remove it from the map.
-void ResourceFetcher::garbageCollectDocumentResourcesTimerFired(Timer<ResourceFetcher>* timer)
-{
-    ASSERT_UNUSED(timer, timer == &m_garbageCollectDocumentResourcesTimer);
-    garbageCollectDocumentResources();
-}
-
-void ResourceFetcher::garbageCollectDocumentResources()
-{
-    typedef Vector<String, 10> StringVector;
-    StringVector resourcesToDelete;
-
-    for (const auto& documentResource : m_documentResources) {
-        if (documentResource.value->hasOneHandle())
-            resourcesToDelete.append(documentResource.key);
-    }
-
-    m_documentResources.removeAll(resourcesToDelete);
-}
-
 int ResourceFetcher::requestCount() const
 {
     return m_loaders ? m_loaders->size() : 0;
 }
 
 void ResourceFetcher::preloadStarted(Resource* resource)
 {
     if (m_preloads && m_preloads->contains(resource))
         return;
     TRACE_EVENT_ASYNC_STEP_INTO0("blink.net", "Resource", resource, "Preload");
@@ skipping 341 matching lines @@
     }
 }
 
 DEFINE_TRACE(ResourceFetcher)
 {
     visitor->trace(m_context);
     visitor->trace(m_archiveResourceCollection);
     visitor->trace(m_loaders);
     visitor->trace(m_nonBlockingLoaders);
 #if ENABLE(OILPAN)
+    visitor->trace(m_documentResources);
     visitor->trace(m_preloads);
     visitor->trace(m_resourceTimingInfoMap);
 #endif
 }
 
 }