Revert of Revalidate using the same Resource, attempt #2

Source/core/fetch/Resource.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) 2006 Samuel Weinig (sam.weinig@gmail.com)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
 
     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 150 matching lines @@
     , m_requestedFromNetworkingLayer(false)
     , m_loading(false)
     , m_switchingClientsToRevalidatedResource(false)
     , m_type(type)
     , m_status(Pending)
     , m_wasPurged(false)
     , m_needsSynchronousCacheHit(false)
 #ifdef ENABLE_RESOURCE_IS_DELETED_CHECK
     , m_deleted(false)
 #endif
+    , m_resourceToRevalidate(nullptr)
+    , m_proxyResource(nullptr)
 {
     ASSERT(m_type == unsigned(type)); // m_type is a bitfield, so this tests careless updates of the enum.
     InstanceCounters::incrementCounter(InstanceCounters::ResourceCounter);
 #ifndef NDEBUG
     cachedResourceLeakCounter.increment();
 #endif
     memoryCache()->registerLiveResource(*this);
 
     // Currently we support the metadata caching only for HTTP family.
     if (m_resourceRequest.url().protocolIsInHTTPFamily())
         m_cacheHandler = CacheHandler::create(this);
 
     if (!m_resourceRequest.url().hasFragmentIdentifier())
         return;
     KURL urlForCache = MemoryCache::removeFragmentIdentifierIfNeeded(m_resourceRequest.url());
     if (urlForCache.hasFragmentIdentifier())
         return;
     m_fragmentIdentifierForRequest = m_resourceRequest.url().fragmentIdentifier();
     m_resourceRequest.setURL(urlForCache);
 }
 
 Resource::~Resource()
 {
+    ASSERT(!m_resourceToRevalidate); // Should be true because canDelete() checks this.
     ASSERT(canDelete());
     RELEASE_ASSERT(!memoryCache()->contains(this));
     RELEASE_ASSERT(!ResourceCallback::callbackHandler()->isScheduled(this));
     assertAlive();
 
 #ifdef ENABLE_RESOURCE_IS_DELETED_CHECK
     m_deleted = true;
 #endif
 #ifndef NDEBUG
     cachedResourceLeakCounter.decrement();
 #endif
     InstanceCounters::decrementCounter(InstanceCounters::ResourceCounter);
 }
 
 void Resource::dispose()
 {
 }
 
 DEFINE_TRACE(Resource)
 {
     visitor->trace(m_loader);
+    visitor->trace(m_resourceToRevalidate);
+    visitor->trace(m_proxyResource);
 #if ENABLE(OILPAN)
     visitor->trace(m_cacheHandler);
 #endif
 }
 
 void Resource::load(ResourceFetcher* fetcher, const ResourceLoaderOptions& options)
 {
     m_options = options;
     m_loading = true;
 
-    ResourceRequest request(m_revalidatingRequest.isNull() ? m_resourceRequest : m_revalidatingRequest);
     if (!accept().isEmpty())
-        request.setHTTPAccept(accept());
+        m_resourceRequest.setHTTPAccept(accept());
 
     // FIXME: It's unfortunate that the cache layer and below get to know anything about fragment identifiers.
     // We should look into removing the expectation of that knowledge from the platform network stacks.
+    ResourceRequest request(m_resourceRequest);
     if (!m_fragmentIdentifierForRequest.isNull()) {
         KURL url = request.url();
         url.setFragmentIdentifier(m_fragmentIdentifierForRequest);
         request.setURL(url);
         m_fragmentIdentifierForRequest = String();
     }
     m_status = Pending;
     if (m_loader) {
-        ASSERT(m_revalidatingRequest.isNull());
         RELEASE_ASSERT(m_options.synchronousPolicy == RequestSynchronously);
         m_loader->changeToSynchronous();
         return;
     }
     m_loader = ResourceLoader::create(fetcher, this, request, options);
     m_loader->start();
 }
 
 void Resource::checkNotify()
 {
     if (isLoading())
         return;
 
     ResourceClientWalker<ResourceClient> w(m_clients);
     while (ResourceClient* c = w.next())
         c->notifyFinished(this);
 }
 
 void Resource::appendData(const char* data, unsigned length)
 {
     TRACE_EVENT0("blink", "Resource::appendData");
-    ASSERT(m_revalidatingRequest.isNull());
+    ASSERT(!m_resourceToRevalidate);
     ASSERT(!errorOccurred());
     if (m_options.dataBufferingPolicy == DoNotBufferData)
         return;
     if (m_data)
         m_data->append(data, length);
     else
         m_data = SharedBuffer::createPurgeable(data, length);
     setEncodedSize(m_data->size());
 }
 
 void Resource::setResourceBuffer(PassRefPtr<SharedBuffer> resourceBuffer)
 {
-    ASSERT(m_revalidatingRequest.isNull());
+    ASSERT(!m_resourceToRevalidate);
     ASSERT(!errorOccurred());
     ASSERT(m_options.dataBufferingPolicy == BufferData);
     m_data = resourceBuffer;
     setEncodedSize(m_data->size());
 }
 
 void Resource::setDataBufferingPolicy(DataBufferingPolicy dataBufferingPolicy)
 {
     m_options.dataBufferingPolicy = dataBufferingPolicy;
     m_data.clear();
     setEncodedSize(0);
 }
 
 void Resource::error(Resource::Status status)
 {
-    if (!m_revalidatingRequest.isNull())
-        m_revalidatingRequest = ResourceRequest();
+    if (m_resourceToRevalidate)
+        revalidationFailed();
 
     if (!m_error.isNull() && (m_error.isCancellation() || !isPreloaded()))
         memoryCache()->remove(this);
 
     setStatus(status);
     ASSERT(errorOccurred());
     m_data.clear();
 
     setLoading(false);
     checkNotify();
 }
 
 void Resource::finishOnePart()
 {
     setLoading(false);
     checkNotify();
 }
 
 void Resource::finish()
 {
-    ASSERT(m_revalidatingRequest.isNull());
+    ASSERT(!m_resourceToRevalidate);
     ASSERT(!errorOccurred());
     finishOnePart();
     if (!errorOccurred())
         m_status = Cached;
 }
 
 bool Resource::passesAccessControlCheck(SecurityOrigin* securityOrigin) const
 {
     String ignoredErrorDescription;
     return passesAccessControlCheck(securityOrigin, ignoredErrorDescription);
@@ skipping 106 matching lines @@
 }
 
 bool Resource::unlock()
 {
     if (!m_data)
         return false;
 
     if (!m_data->isLocked())
         return true;
 
-    if (!memoryCache()->contains(this) || hasClients() || m_handleCount > 1 || !m_revalidatingRequest.isNull() || !m_loadFinishTime || !isSafeToUnlock())
+    if (!memoryCache()->contains(this) || hasClients() || m_handleCount > 1 || m_proxyResource || m_resourceToRevalidate || !m_loadFinishTime || !isSafeToUnlock())
         return false;
 
     m_data->unlock();
     return true;
 }
 
 bool Resource::hasRightHandleCountApartFromCache(unsigned targetCount) const
 {
     return m_handleCount == targetCount + (memoryCache()->contains(this) ? 1 : 0);
 }
 
 void Resource::responseReceived(const ResourceResponse& response, PassOwnPtr<WebDataConsumerHandle>)
 {
+    setResponse(response);
     m_responseTimestamp = currentTime();
-
-    if (!m_revalidatingRequest.isNull()) {
-        if (response.httpStatusCode() == 304) {
-            revalidationSucceeded(response);
-            return;
-        }
-        revalidationFailed();
-    }
-
-    setResponse(response);
     String encoding = response.textEncodingName();
     if (!encoding.isNull())
         setEncoding(encoding);
+
+    if (!m_resourceToRevalidate)
+        return;
+    if (response.httpStatusCode() == 304)
+        revalidationSucceeded(response);
+    else
+        revalidationFailed();
 }
 
 void Resource::setSerializedCachedMetadata(const char* data, size_t size)
 {
     // We only expect to receive cached metadata from the platform once.
     // If this triggers, it indicates an efficiency problem which is most
     // likely unexpected in code designed to improve performance.
     ASSERT(!m_cachedMetadata);
-    ASSERT(m_revalidatingRequest.isNull());
+    ASSERT(!m_resourceToRevalidate);
 
     m_cachedMetadata = CachedMetadata::deserialize(data, size);
 }
 
 CachedMetadataHandler* Resource::cacheHandler()
 {
     return m_cacheHandler.get();
 }
 
 void Resource::setCachedMetadata(unsigned dataTypeID, const char* data, size_t size, CachedMetadataHandler::CacheType cacheType)
@@ skipping 20 matching lines @@
 {
     m_cachedMetadata.clear();
 
     if (cacheType == CachedMetadataHandler::SendToPlatform)
         Platform::current()->cacheMetadata(m_response.url(), m_response.responseTime(), 0, 0);
 }
 
 bool Resource::canDelete() const
 {
     return !hasClients() && !m_loader && !m_preloadCount && hasRightHandleCountApartFromCache(0)
-        && !m_protectorCount;
+        && !m_protectorCount && !m_resourceToRevalidate && !m_proxyResource;
 }
 
 bool Resource::hasOneHandle() const
 {
     return hasRightHandleCountApartFromCache(1);
 }
 
 CachedMetadata* Resource::cachedMetadata(unsigned dataTypeID) const
 {
     if (!m_cachedMetadata || m_cachedMetadata->dataTypeID() != dataTypeID)
@@ skipping 46 matching lines @@
             m_preloadResult = PreloadReferencedWhileComplete;
         else if (m_requestedFromNetworkingLayer)
             m_preloadResult = PreloadReferencedWhileLoading;
         else
             m_preloadResult = PreloadReferenced;
     }
     if (!hasClients())
         memoryCache()->makeLive(this);
 
     // If we have existing data to send to the new client and the resource type supprts it, send it asynchronously.
-    if (!m_response.isNull() && !shouldSendCachedDataSynchronouslyForType(type()) && !m_needsSynchronousCacheHit) {
+    if (!m_response.isNull() && !m_proxyResource && !shouldSendCachedDataSynchronouslyForType(type()) && !m_needsSynchronousCacheHit) {
         m_clientsAwaitingCallback.add(client);
         ResourceCallback::callbackHandler()->schedule(this);
         return false;
     }
 
     m_clients.add(client);
     return true;
 }
 
 void Resource::removeClient(ResourceClient* client)
@@ skipping 123 matching lines @@
     // Prevent the case when there are clients waiting but no callback scheduled.
     ASSERT(m_clientsAwaitingCallback.isEmpty() || scheduled);
 }
 
 void Resource::prune()
 {
     destroyDecodedDataIfPossible();
     unlock();
 }
 
+void Resource::setResourceToRevalidate(Resource* resource)
+{
+    ASSERT(resource);
+    ASSERT(!m_resourceToRevalidate);
+    ASSERT(resource != this);
+    ASSERT(m_handlesToRevalidate.isEmpty());
+    ASSERT(resource->type() == type());
 
-void Resource::revalidationSucceeded(const ResourceResponse& validatingResponse)
+    WTF_LOG(ResourceLoading, "Resource %p setResourceToRevalidate %p", this, resource);
+
+    // The following assert should be investigated whenever it occurs. Although it should never fire, it currently does in rare circumstances.
+    // https://bugs.webkit.org/show_bug.cgi?id=28604.
+    // So the code needs to be robust to this assert failing thus the "if (m_resourceToRevalidate->m_proxyResource == this)" in Resource::clearResourceToRevalidate.
+    ASSERT(!resource->m_proxyResource);
+
+    resource->m_proxyResource = this;
+    m_resourceToRevalidate = resource;
+}
+
+void Resource::clearResourceToRevalidate()
 {
-    // Calling evict() can potentially delete revalidatingResource, which we use
-    // below. This mustn't be the case since revalidation means it is loaded
-    // and so canDelete() is false.
-    ASSERT(!canDelete());
+    ASSERT(m_resourceToRevalidate);
+    if (m_switchingClientsToRevalidatedResource)
+        return;
 
+    // A resource may start revalidation before this method has been called, so check that this resource is still the proxy resource before clearing it out.
+    if (m_resourceToRevalidate->m_proxyResource == this) {
+        m_resourceToRevalidate->m_proxyResource = nullptr;
+        m_resourceToRevalidate->deleteIfPossible();
+    }
+    m_handlesToRevalidate.clear();
+    m_resourceToRevalidate = nullptr;
+    deleteIfPossible();
+}
+
+void Resource::switchClientsToRevalidatedResource()
+{
+    ASSERT(m_resourceToRevalidate);
+    ASSERT(memoryCache()->contains(m_resourceToRevalidate));
+    ASSERT(!memoryCache()->contains(this));
+
+    WTF_LOG(ResourceLoading, "Resource %p switchClientsToRevalidatedResource %p", this, m_resourceToRevalidate.get());
+
+    m_resourceToRevalidate->m_identifier = m_identifier;
+
+    m_switchingClientsToRevalidatedResource = true;
+    for (ResourcePtrBase* handle : m_handlesToRevalidate) {
+        handle->m_resource = m_resourceToRevalidate;
+        m_resourceToRevalidate->registerHandle(handle);
+        --m_handleCount;
+    }
+    ASSERT(!m_handleCount);
+    m_handlesToRevalidate.clear();
+
+    Vector<ResourceClient*> clientsToMove;
+    for (const auto& clientHashEntry : m_clients) {
+        unsigned count = clientHashEntry.value;
+        while (count--)
+            clientsToMove.append(clientHashEntry.key);
+    }
+
+    unsigned moveCount = clientsToMove.size();
+    for (unsigned n = 0; n < moveCount; ++n)
+        removeClient(clientsToMove[n]);
+    ASSERT(m_clients.isEmpty());
+
+    for (unsigned n = 0; n < moveCount; ++n)
+        m_resourceToRevalidate->addClientToSet(clientsToMove[n]);
+    for (unsigned n = 0; n < moveCount; ++n) {
+        // Calling didAddClient may do anything, including trying to cancel revalidation.
+        // Assert that it didn't succeed.
+        ASSERT(m_resourceToRevalidate);
+        // Calling didAddClient for a client may end up removing another client. In that case it won't be in the set anymore.
+        if (m_resourceToRevalidate->m_clients.contains(clientsToMove[n]))
+            m_resourceToRevalidate->didAddClient(clientsToMove[n]);
+    }
+    m_switchingClientsToRevalidatedResource = false;
+}
+
+void Resource::updateResponseAfterRevalidation(const ResourceResponse& validatingResponse)
+{
     m_responseTimestamp = currentTime();
-    m_response.setResourceLoadTiming(validatingResponse.resourceLoadTiming());
 
     // RFC2616 10.3.5
     // Update cached headers from the 304 response
     const HTTPHeaderMap& newHeaders = validatingResponse.httpHeaderFields();
     for (const auto& header : newHeaders) {
         // Entity headers should not be sent by servers when generating a 304
         // response; misconfigured servers send them anyway. We shouldn't allow
         // such headers to update the original request. We'll base this on the
         // list defined by RFC2616 7.1, with a few additions for extension headers
         // we care about.
         if (!shouldUpdateHeaderAfterRevalidation(header.key))
             continue;
         m_response.setHTTPHeaderField(header.key, header.value);
     }
+}
 
+void Resource::revalidationSucceeded(const ResourceResponse& response)
+{
+    ASSERT(m_resourceToRevalidate);
+    ASSERT(!memoryCache()->contains(m_resourceToRevalidate));
+    ASSERT(m_resourceToRevalidate->isLoaded());
+
+    // Calling evict() can potentially delete revalidatingResource, which we use
+    // below. This mustn't be the case since revalidation means it is loaded
+    // and so canDelete() is false.
+    ASSERT(!canDelete());
+
+    m_resourceToRevalidate->updateResponseAfterRevalidation(response);
+    memoryCache()->replace(m_resourceToRevalidate, this);
+
+    switchClientsToRevalidatedResource();
     assertAlive();
-    m_resourceRequest = m_revalidatingRequest;
-    m_revalidatingRequest = ResourceRequest();
+    // clearResourceToRevalidate deletes this.
+    clearResourceToRevalidate();
 }
 
 void Resource::revalidationFailed()
 {
-    m_resourceRequest = m_revalidatingRequest;
-    m_revalidatingRequest = ResourceRequest();
-    m_data.clear();
-    destroyDecodedDataForFailedRevalidation();
+    ASSERT(WTF::isMainThread());
+    WTF_LOG(ResourceLoading, "Revalidation failed for %p", this);
+    ASSERT(resourceToRevalidate());
+    clearResourceToRevalidate();
 }
 
 void Resource::registerHandle(ResourcePtrBase* h)
 {
     assertAlive();
     ++m_handleCount;
+    if (m_resourceToRevalidate)
+        m_handlesToRevalidate.add(h);
 }
 
 void Resource::unregisterHandle(ResourcePtrBase* h)
 {
     assertAlive();
     ASSERT(m_handleCount > 0);
     --m_handleCount;
 
+    if (m_resourceToRevalidate)
+        m_handlesToRevalidate.remove(h);
+
     if (!m_handleCount) {
         if (deleteIfPossible())
             return;
         unlock();
     } else if (m_handleCount == 1 && memoryCache()->contains(this)) {
         unlock();
         if (!hasClients())
             memoryCache()->prune(this);
     }
 }
@@ skipping 208 matching lines @@
         return "ImportResource";
     case Resource::Media:
         return "Media";
     }
     ASSERT_NOT_REACHED();
     return "Unknown";
 }
 #endif // !LOG_DISABLED
 
 } // namespace blink