Revert "Separate preaload matching from MemoryCache"

third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

Index: third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
index 5b3016ed4dee6f4f740229658cc5aeba0768713c..a638d74e061bf8ef0505ffac18665d74dce644d9 100644
--- a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
+++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
@@ -493,26 +493,11 @@ void ResourceFetcher::UpdateMemoryCacheStats(Resource* resource,
   // dead if MemoryCache holds weak references to Resource). Currently we check
   // references to Resource from ResourceClient and |m_preloads| only, because
   // they are major sources of references.
-  if (resource && !resource->IsAlive() && !ContainsAsPreload(resource)) {
+  if (resource && !resource->IsAlive() && !preloads_.Contains(resource)) {
     DEFINE_RESOURCE_HISTOGRAM("Dead.");
   }
 }
 
-bool ResourceFetcher::ContainsAsPreload(Resource* resource) const {
-  auto it = preloads_.find(PreloadKey(resource->Url(), resource->GetType()));
-  return it != preloads_.end() && it->value == resource;
-}
-
-void ResourceFetcher::RemovePreload(Resource* resource) {
-  auto it = preloads_.find(PreloadKey(resource->Url(), resource->GetType()));
-  if (it == preloads_.end())
-    return;
-  if (it->value == resource) {
-    resource->DecreasePreloadCount();
-    preloads_.erase(it);
-  }
-}
-
 ResourceFetcher::PrepareRequestResult ResourceFetcher::PrepareRequest(
     FetchParameters& params,
     const ResourceFactory& factory,
@@ -619,30 +604,19 @@ Resource* ResourceFetcher::RequestResource(
     if (!resource && !is_data_url && archive_)
       return nullptr;
   }
-  RevalidationPolicy policy = kLoad;
-  bool preload_found = false;
   if (!resource) {
-    resource = MatchPreload(params, factory.GetType());
-    if (resource) {
-      preload_found = true;
-      policy = kUse;
-      // If |param| is for a blocking resource and a preloaded resource is
-      // found, we may need to make it block the onload event.
-      MakePreloadedResourceBlockOnloadIfNeeded(resource, params);
-    }
+    resource =
+        GetMemoryCache()->ResourceForURL(params.Url(), GetCacheIdentifier());
   }
-  if (!preload_found) {
-    if (!resource) {
-      resource =
-          GetMemoryCache()->ResourceForURL(params.Url(), GetCacheIdentifier());
-    }
 
-    policy = DetermineRevalidationPolicy(factory.GetType(), params, resource,
-                                         is_static_data);
-    TRACE_EVENT_INSTANT1(
-        "blink", "ResourceFetcher::determineRevalidationPolicy",
-        TRACE_EVENT_SCOPE_THREAD, "revalidationPolicy", policy);
-  }
+  // If we got a preloaded resource from the cache for a non-preload request,
+  // we may need to make it block the onload event.
+  MakePreloadedResourceBlockOnloadIfNeeded(resource, params);
+
+  const RevalidationPolicy policy = DetermineRevalidationPolicy(
+      factory.GetType(), params, resource, is_static_data);
+  TRACE_EVENT_INSTANT1("blink", "ResourceFetcher::determineRevalidationPolicy",
+                       TRACE_EVENT_SCOPE_THREAD, "revalidationPolicy", policy);
 
   UpdateMemoryCacheStats(resource, policy, params, factory, is_static_data);
 
@@ -695,17 +669,11 @@ Resource* ResourceFetcher::RequestResource(
   // loading immediately. If revalidation policy was determined as |Revalidate|,
   // the resource was already initialized for the revalidation here, but won't
   // start loading.
-  if (!ResourceNeedsLoad(resource, params, policy)) {
-    if (policy != kUse)
-      InsertAsPreloadIfNecessary(resource, params, factory.GetType());
+  if (!ResourceNeedsLoad(resource, params, policy))
     return resource;
-  }
 
   if (!StartLoad(resource))
     return nullptr;
-
-  if (policy != kUse)
-    InsertAsPreloadIfNecessary(resource, params, factory.GetType());
   scoped_resource_load_tracker.ResourceLoadContinuesBeyondScope();
 
   DCHECK(!resource->ErrorOccurred() ||
@@ -859,56 +827,61 @@ void ResourceFetcher::RecordResourceTimingOnRedirect(
   }
 }
 
-Resource* ResourceFetcher::MatchPreload(const FetchParameters& params,
-                                        Resource::Type type) {
-  auto it = preloads_.find(PreloadKey(params.Url(), type));
-  if (it == preloads_.end())
-    return nullptr;
-
-  Resource* resource = it->value;
+ResourceFetcher::RevalidationPolicy
+ResourceFetcher::DetermineRevalidationPolicy(
+    Resource::Type type,
+    const FetchParameters& fetch_params,
+    Resource* existing_resource,
+    bool is_static_data) const {
+  const ResourceRequest& request = fetch_params.GetResourceRequest();
 
-  RecordSriResourceIntegrityMismatchEvent(kCheckingForIntegrityMismatch);
-  if (resource->MustRefetchDueToIntegrityMetadata(params))
-    return nullptr;
+  if (!existing_resource)
+    return kLoad;
 
-  if (params.IsSpeculativePreload())
-    return resource;
-  if (params.IsLinkPreload()) {
-    resource->SetLinkPreload(true);
-    return resource;
+  // If the existing resource is loading and the associated fetcher is not equal
+  // to |this|, we must not use the resource. Otherwise, CSP violation may
+  // happen in redirect handling.
+  if (existing_resource->Loader() &&
+      existing_resource->Loader()->Fetcher() != this) {
+    return kReload;
   }
 
-  if (!IsReusableForPreloading(params, resource, false))
-    return nullptr;
-
-  resource->DecreasePreloadCount();
-  preloads_.erase(it);
-  return resource;
-}
+  // Checks if the resource has an explicit policy about integrity metadata.
+  //
+  // This is necessary because ScriptResource and CSSStyleSheetResource objects
+  // do not keep the raw data around after the source is accessed once, so if
+  // the resource is accessed from the MemoryCache for a second time, there is
+  // no way to redo an integrity check.
+  //
+  // Thus, Blink implements a scheme where it caches the integrity information
+  // for those resources after the first time it is checked, and if there is
+  // another request for that resource, with the same integrity metadata, Blink
+  // skips the integrity calculation. However, if the integrity metadata is a
+  // mismatch, the MemoryCache must be skipped here, and a new request for the
+  // resource must be made to get the raw data. This is expected to be an
+  // uncommon case, however, as it implies two same-origin requests to the same
+  // resource, but with different integrity metadata.
+  RecordSriResourceIntegrityMismatchEvent(kCheckingForIntegrityMismatch);
+  if (existing_resource->MustRefetchDueToIntegrityMetadata(fetch_params)) {
+    RecordSriResourceIntegrityMismatchEvent(kRefetchDueToIntegrityMismatch);
+    return kReload;
+  }
 
-void ResourceFetcher::InsertAsPreloadIfNecessary(Resource* resource,
-                                                 const FetchParameters& params,
-                                                 Resource::Type type) {
-  if (!params.IsSpeculativePreload() && !params.IsLinkPreload())
-    return;
-  // CSP layout tests verify that preloads are subject to access checks by
-  // seeing if they are in the `preload started` list. Therefore do not add
-  // them to the list if the load is immediately denied.
-  if (resource->GetResourceError().IsAccessCheck())
-    return;
-  PreloadKey key(params.Url(), type);
-  if (preloads_.find(key) == preloads_.end()) {
-    preloads_.insert(key, resource);
-    resource->IncreasePreloadCount();
-    if (preloaded_urls_for_test_)
-      preloaded_urls_for_test_->insert(resource->Url().GetString());
+  // If the same URL has been loaded as a different type, we need to reload.
+  if (existing_resource->GetType() != type) {
+    // FIXME: If existingResource is a Preload and the new type is LinkPrefetch
+    // We really should discard the new prefetch since the preload has more
+    // specific type information! crbug.com/379893
+    // fast/dom/HTMLLinkElement/link-and-subresource-test hits this case.
+    RESOURCE_LOADING_DVLOG(1) << "ResourceFetcher::determineRevalidationPolicy "
+                                 "reloading due to type mismatch.";
+    return kReload;
   }
-}
 
-bool ResourceFetcher::IsReusableForPreloading(const FetchParameters& params,
-                                              Resource* existing_resource,
-                                              bool is_static_data) const {
-  const ResourceRequest& request = params.GetResourceRequest();
+  // We already have a preload going for this URL.
+  if (fetch_params.IsSpeculativePreload() && existing_resource->IsPreloaded())
+    return kUse;
+
   // Do not load from cache if images are not enabled. There are two general
   // cases:
   //
@@ -925,13 +898,13 @@ bool ResourceFetcher::IsReusableForPreloading(const FetchParameters& params,
   // TODO(japhet): Can we get rid of one of these settings?
   if (existing_resource->IsImage() &&
       !Context().AllowImage(images_enabled_, existing_resource->Url())) {
-    return false;
+    return kReload;
   }
 
   // Never use cache entries for downloadToFile / useStreamOnResponse requests.
   // The data will be delivered through other paths.
   if (request.DownloadToFile() || request.UseStreamOnResponse())
-    return false;
+    return kReload;
 
   // Never reuse opaque responses from a service worker for requests that are
   // not no-cors. https://crbug.com/625575
@@ -939,11 +912,11 @@ bool ResourceFetcher::IsReusableForPreloading(const FetchParameters& params,
       existing_resource->GetResponse().ServiceWorkerResponseType() ==
           kWebServiceWorkerResponseTypeOpaque &&
       request.GetFetchRequestMode() != WebURLRequest::kFetchRequestModeNoCORS) {
-    return false;
+    return kReload;
   }
 
-  if (!is_static_data && !existing_resource->CanReuse(params))
-    return false;
+  if (!is_static_data && !existing_resource->CanReuse(fetch_params))
+    return kReload;
 
   // Certain requests (e.g., XHRs) might have manually set headers that require
   // revalidation. In theory, this should be a Revalidate case. In practice, the
@@ -959,72 +932,17 @@ bool ResourceFetcher::IsReusableForPreloading(const FetchParameters& params,
   if (!is_static_data &&
       (request.IsConditional() ||
        existing_resource->GetResponse().HttpStatusCode() == 304)) {
-    return false;
-  }
-
-  if (!is_static_data &&
-      !params.Options().CanReuseRequest(existing_resource->Options())) {
-    return false;
-  }
-
-  return true;
-}
-
-ResourceFetcher::RevalidationPolicy
-ResourceFetcher::DetermineRevalidationPolicy(
-    Resource::Type type,
-    const FetchParameters& fetch_params,
-    Resource* existing_resource,
-    bool is_static_data) const {
-  const ResourceRequest& request = fetch_params.GetResourceRequest();
-
-  if (!existing_resource)
-    return kLoad;
-
-  // If the existing resource is loading and the associated fetcher is not equal
-  // to |this|, we must not use the resource. Otherwise, CSP violation may
-  // happen in redirect handling.
-  if (existing_resource->Loader() &&
-      existing_resource->Loader()->Fetcher() != this) {
     return kReload;
   }
 
-  // Checks if the resource has an explicit policy about integrity metadata.
-  //
-  // This is necessary because ScriptResource and CSSStyleSheetResource objects
-  // do not keep the raw data around after the source is accessed once, so if
-  // the resource is accessed from the MemoryCache for a second time, there is
-  // no way to redo an integrity check.
-  //
-  // Thus, Blink implements a scheme where it caches the integrity information
-  // for those resources after the first time it is checked, and if there is
-  // another request for that resource, with the same integrity metadata, Blink
-  // skips the integrity calculation. However, if the integrity metadata is a
-  // mismatch, the MemoryCache must be skipped here, and a new request for the
-  // resource must be made to get the raw data. This is expected to be an
-  // uncommon case, however, as it implies two same-origin requests to the same
-  // resource, but with different integrity metadata.
-  RecordSriResourceIntegrityMismatchEvent(kCheckingForIntegrityMismatch);
-  if (existing_resource->MustRefetchDueToIntegrityMetadata(fetch_params)) {
-    RecordSriResourceIntegrityMismatchEvent(kRefetchDueToIntegrityMismatch);
-    return kReload;
-  }
-
-  // If the same URL has been loaded as a different type, we need to reload.
-  if (existing_resource->GetType() != type) {
-    // FIXME: If existingResource is a Preload and the new type is LinkPrefetch
-    // We really should discard the new prefetch since the preload has more
-    // specific type information! crbug.com/379893
-    // fast/dom/HTMLLinkElement/link-and-subresource-test hits this case.
-    RESOURCE_LOADING_DVLOG(1) << "ResourceFetcher::determineRevalidationPolicy "
-                                 "reloading due to type mismatch.";
+  if (!is_static_data &&
+      !fetch_params.Options().CanReuseRequest(existing_resource->Options())) {
     return kReload;
   }
 
-  // If |existing_resource| is not reusable as a preloaded resource, it should
-  // not be reusable as a normal resource as well.
-  if (!IsReusableForPreloading(fetch_params, existing_resource, is_static_data))
-    return kReload;
+  // Always use preloads.
+  if (existing_resource->IsPreloaded())
+    return kUse;
 
   // If resource was populated from a SubstituteData load or data: url, use it.
   if (is_static_data)
@@ -1185,15 +1103,24 @@ int ResourceFetcher::NonblockingRequestCount() const {
   return non_blocking_loaders_.size();
 }
 
+void ResourceFetcher::PreloadStarted(Resource* resource) {
+  if (preloads_.Contains(resource))
+    return;
+  resource->IncreasePreloadCount();
+
+  preloads_.insert(resource);
+
+  if (preloaded_urls_for_test_)
+    preloaded_urls_for_test_->insert(resource->Url().GetString());
+}
+
 void ResourceFetcher::EnableIsPreloadedForTest() {
   if (preloaded_urls_for_test_)
     return;
   preloaded_urls_for_test_ = WTF::WrapUnique(new HashSet<String>);
 
-  for (const auto& pair : preloads_) {
-    Resource* resource = pair.value;
+  for (const auto& resource : preloads_)
     preloaded_urls_for_test_->insert(resource->Url().GetString());
-  }
 }
 
 bool ResourceFetcher::IsPreloadedForTest(const KURL& url) const {
@@ -1204,22 +1131,18 @@ bool ResourceFetcher::IsPreloadedForTest(const KURL& url) const {
 void ResourceFetcher::ClearPreloads(ClearPreloadsPolicy policy) {
   LogPreloadStats(policy);
 
-  Vector<PreloadKey> keys_to_be_removed;
-  for (const auto& pair : preloads_) {
-    Resource* resource = pair.value;
+  for (const auto& resource : preloads_) {
     if (policy == kClearAllPreloads || !resource->IsLinkPreload()) {
       resource->DecreasePreloadCount();
       if (resource->GetPreloadResult() == Resource::kPreloadNotReferenced)
-        GetMemoryCache()->Remove(resource);
-      keys_to_be_removed.push_back(pair.key);
+        GetMemoryCache()->Remove(resource.Get());
+      preloads_.erase(resource);
     }
   }
-  preloads_.RemoveAll(keys_to_be_removed);
 }
 
 void ResourceFetcher::WarnUnusedPreloads() {
-  for (const auto& pair : preloads_) {
-    Resource* resource = pair.value;
+  for (const auto& resource : preloads_) {
     if (resource && resource->IsLinkPreload() &&
         resource->GetPreloadResult() == Resource::kPreloadNotReferenced) {
       Context().AddConsoleMessage(
@@ -1326,8 +1249,6 @@ void ResourceFetcher::HandleLoaderError(Resource* resource,
                             resource->GetResponse().EncodedDataLength(),
                             is_internal_request);
 
-  if (error.IsCancellation())
-    RemovePreload(resource);
   resource->FinishAsError(error);
 
   HandleLoadCompletion(resource);
@@ -1486,8 +1407,7 @@ void ResourceFetcher::LogPreloadStats(ClearPreloadsPolicy policy) {
   unsigned import_misses = 0;
   unsigned raws = 0;
   unsigned raw_misses = 0;
-  for (const auto& pair : preloads_) {
-    Resource* resource = pair.value;
+  for (const auto& resource : preloads_) {
     // Do not double count link rel preloads. These do not get cleared if the
     // ClearPreloadsPolicy is only clearing speculative markup preloads.
     if (resource->IsLinkPreload() &&