Unify Resource loading status tracking

third_party/WebKit/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 140 matching lines @@
 #if !ENABLE(OILPAN)
     , m_weakPtrFactory(this)
 #endif
     , m_loadFinishTime(0)
     , m_identifier(0)
     , m_encodedSize(0)
     , m_decodedSize(0)
     , m_preloadCount(0)
     , m_cacheIdentifier(MemoryCache::defaultCacheIdentifier())
     , m_preloadResult(PreloadNotReferenced)
-    , m_requestedFromNetworkingLayer(false)
-    , m_loading(false)
     , m_type(type)
-    , m_status(Pending)
+    , m_status(NotStarted)
     , m_needsSynchronousCacheHit(false)
     , m_linkPreload(false)
 {
     ASSERT(m_type == unsigned(type)); // m_type is a bitfield, so this tests careless updates of the enum.
     InstanceCounters::incrementCounter(InstanceCounters::ResourceCounter);
 
     // Currently we support the metadata caching only for HTTP family.
     if (m_resourceRequest.url().protocolIsInHTTPFamily())
         m_cacheHandler = CacheHandler::create(this);
 
@@ skipping 21 matching lines @@
     visitor->trace(m_loader);
 #if ENABLE(OILPAN)
     visitor->trace(m_cacheHandler);
 #endif
 }
 
 void Resource::load(ResourceFetcher* fetcher, const ResourceLoaderOptions& options)
 {
     RELEASE_ASSERT(!m_loader);
     m_options = options;
-    m_loading = true;
     m_status = Pending;
 
     ResourceRequest& request(m_revalidatingRequest.isNull() ? m_resourceRequest : m_revalidatingRequest);
     if (!accept().isEmpty())
         request.setHTTPAccept(accept());
     request.setAllowStoredCredentials(m_options.allowCredentials == AllowStoredCredentials);
 
     // 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.
     KURL urlWithoutFragment = request.url();
@@ skipping 67 matching lines @@
 {
     if (!m_revalidatingRequest.isNull())
         m_revalidatingRequest = ResourceRequest();
 
     if (!m_error.isNull() && (m_error.isCancellation() || !isPreloaded()))
         memoryCache()->remove(this);
 
     setStatus(status);
     ASSERT(errorOccurred());
     m_data.clear();
-
-    setLoading(false);
     checkNotify();
     markClientsFinished();
 }
 
 void Resource::finish()
 {
     ASSERT(m_revalidatingRequest.isNull());
-    setLoading(false);
+    if (!errorOccurred())
+        m_status = Cached;
     checkNotify();
     markClientsFinished();
-    if (!errorOccurred())
-        m_status = Cached;
 }
 
 AtomicString Resource::httpContentType() const
 {
     return extractMIMETypeFromMediaType(m_response.httpHeaderField(HTTPNames::Content_Type).lower());
 }
 
 bool Resource::passesAccessControlCheck(SecurityOrigin* securityOrigin) const
 {
     String ignoredErrorDescription;
@@ skipping 97 matching lines @@
 {
     if (!m_redirectChain.size())
         return m_resourceRequest;
     return m_redirectChain.last().m_request;
 }
 
 void Resource::willFollowRedirect(ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
 {
     newRequest.setAllowStoredCredentials(m_options.allowCredentials == AllowStoredCredentials);
     m_redirectChain.append(RedirectPair(newRequest, redirectResponse));
-    m_requestedFromNetworkingLayer = true;
 }
 
 bool Resource::unlock()
 {
     if (!m_data)
         return false;
 
     if (!m_data->isLocked())
         return true;
 
@@ skipping 117 matching lines @@
     return m_cachedMetadata.get();
 }
 
 void Resource::clearLoader()
 {
     m_loader = nullptr;
 }
 
 void Resource::didAddClient(ResourceClient* c)
 {
-    if (!isLoading() && !stillNeedsLoad()) {
+    if (isLoaded()) {
         c->notifyFinished(this);
         if (m_clients.contains(c)) {
             m_finishedClients.add(c);
             m_clients.remove(c);
         }
     }
 }
 
 static bool shouldSendCachedDataSynchronouslyForType(Resource::Type type)
 {
@@ skipping 14 matching lines @@
     return false;
 }
 
 void Resource::addClient(ResourceClient* client)
 {
     ASSERT(!isPurgeable());
 
     if (m_preloadResult == PreloadNotReferenced) {
         if (isLoaded())
             m_preloadResult = PreloadReferencedWhileComplete;
-        else if (m_requestedFromNetworkingLayer)
+        else if (isLoading())
             m_preloadResult = PreloadReferencedWhileLoading;
         else
             m_preloadResult = PreloadReferenced;
     }
     if (!hasClients())
         memoryCache()->makeLive(this);
 
     if (!m_revalidatingRequest.isNull()) {
         m_clients.add(client);
         return;
@@ skipping 57 matching lines @@
     unlock();
 }
 
 void Resource::cancelTimerFired(Timer<Resource>* timer)
 {
     ASSERT_UNUSED(timer, timer == &m_cancelTimer);
     if (hasClients() || !m_loader)
         return;
     RefPtrWillBeRawPtr<Resource> protect(this);
     m_loader->cancelIfNotFinishing();
-    if (m_status != Cached)

[Nate Chapin, 2016/03/18 21:24:06]

This clause is weird. If we cancel *while finishing*, m_status used to be
Pending, but with this change, it's Cached. That means that we used to remove
resources from MemoryCache if they were being cancelled during completion. The
m_loader null check above ensures we won't reach this point if the load is
really, truly complete, so dropping this if() keeps the logic pretty close to
the same.

Leaving the if() in place breaks a layout test:
http/tests/inspector/network/network-xhr-same-url-as-main-resource.html

and 3 browser_tests:
ExtensionRequestLimitingThrottleBrowserTest.DoNotThrottleCachedResponse_Redirect
ExtensionRequestLimitingThrottleBrowserTest.DoNotThrottleCachedResponse
ExtensionRequestLimitingThrottleBrowserTest.DoNotThrottleCachedResponse_NonRedirectCached

The layout test breaks because the inspector tries to pre-cache the url in
question when it opens, then immediately drops its reference to it. The test
depends on dropping the reference removing the resource from MemoryCache, as it
then requests that url and breaks if it is a cache. The browser tests have a
similar (but not identical) root cause, but I didn't dig down to find the exact
details.

[hiroshige, 2016/03/18 23:21:20]

This is the case in XHR and might cause the test failures. Removing the |if|
looks good for keeping XHR's behavior.

The fact that |m_loader| is still non null and we can cancel Resource's loading
and prevent it from being cached in notifyFinished() looks awkward -- it means
Resource is not completely loaded in that sense -- but it's the current
behavior.

[Nate Chapin, 2016/03/21 22:04:01]

Yeah, it seems like a good opportunity for cleanup if the weak reference
MemoryCache doesn't work out.

-        memoryCache()->remove(this);
+    memoryCache()->remove(this);
 }
 
 void Resource::setDecodedSize(size_t decodedSize)
 {
     if (decodedSize == m_decodedSize)
         return;
     size_t oldSize = size();
     m_decodedSize = decodedSize;
     memoryCache()->update(this, oldSize, size());
     memoryCache()->updateDecodedResource(this, UpdateForPropertyChange);
@@ skipping 170 matching lines @@
     return !m_response.httpHeaderField(HTTPNames::Vary).isNull();
 }
 
 bool Resource::mustRevalidateDueToCacheHeaders()
 {
     return !canUseResponse(m_response, m_responseTimestamp) || m_resourceRequest.cacheControlContainsNoCache() || m_resourceRequest.cacheControlContainsNoStore();
 }
 
 bool Resource::canUseCacheValidator()
 {
-    if (m_loading || errorOccurred())
+    if (isLoading() || errorOccurred())
         return false;
 
     if (hasCacheControlNoStoreHeader())
         return false;
     return m_response.hasCacheValidatorFields() || m_resourceRequest.hasCacheValidatorFields();
 }
 
 bool Resource::isPurgeable() const
 {
     return m_data && !m_data->isLocked();
@@ skipping 221 matching lines @@
     case Resource::Media:
         return "Media";
     case Resource::Manifest:
         return "Manifest";
     }
     ASSERT_NOT_REACHED();
     return "Unknown";
 }
 
 } // namespace blink