CSSPreloadResourceClient should register with Resource via ResourceFetcher

Bug: 793028
Change-Id: Ice4ec0acdd179f2a7e2b17a497ee3bdf6e91fff4
Reviewed-on: https://chromium-review.googlesource.com/822916Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523607}

third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp

@@ -257,16 +257,13 @@ void CSSPreloadScanner::EmitRule(const SegmentedString& source) {
 }
 
 CSSPreloaderResourceClient::CSSPreloaderResourceClient(
-    Resource* resource,
     HTMLResourcePreloader* preloader)
     : policy_(preloader->GetDocument()
                       ->GetSettings()
                       ->GetCSSExternalScannerPreload()
                   ? kScanAndPreload
                   : kScanOnly),
-      preloader_(preloader) {
-  SetResource(resource);
-}
+      preloader_(preloader) {}
 
 CSSPreloaderResourceClient::~CSSPreloaderResourceClient() {}
 

third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.h

@@ -106,7 +106,7 @@ class CORE_EXPORT CSSPreloaderResourceClient
   USING_GARBAGE_COLLECTED_MIXIN(CSSPreloaderResourceClient);
 
  public:
-  CSSPreloaderResourceClient(Resource*, HTMLResourcePreloader*);
+  CSSPreloaderResourceClient(HTMLResourcePreloader*);
   ~CSSPreloaderResourceClient() override;
   void NotifyFinished(Resource*) override;
   void DataReceived(Resource*, const char*, size_t) override;

third_party/WebKit/Source/core/html/parser/CSSPreloadScannerTest.cpp

@@ -39,7 +39,7 @@ class CSSMockHTMLResourcePreloader : public HTMLResourcePreloader {
   void Preload(std::unique_ptr<PreloadRequest> preload_request,
                const NetworkHintsInterface&) override {
     if (expected_referrer_) {
-      Resource* resource = preload_request->Start(GetDocument());
+      Resource* resource = preload_request->Start(GetDocument(), nullptr);
       if (resource) {
         EXPECT_EQ(expected_referrer_,
                   resource->GetResourceRequest().HttpReferrer());
@@ -55,9 +55,8 @@ class CSSMockHTMLResourcePreloader : public HTMLResourcePreloader {
 class PreloadRecordingCSSPreloaderResourceClient final
     : public CSSPreloaderResourceClient {
  public:
-  PreloadRecordingCSSPreloaderResourceClient(Resource* resource,
-                                             HTMLResourcePreloader* preloader)
-      : CSSPreloaderResourceClient(resource, preloader) {}
+  PreloadRecordingCSSPreloaderResourceClient(HTMLResourcePreloader* preloader)
+      : CSSPreloaderResourceClient(preloader) {}
 
   void FetchPreloads(PreloadRequestStream& preloads) override {
     for (const auto& it : preloads) {
@@ -86,19 +85,20 @@ TEST_F(CSSPreloadScannerTest, ScanFromResourceClient) {
       new CSSMockHTMLResourcePreloader(dummy_page_holder->GetDocument());
 
   KURL url("http://127.0.0.1/foo.css");
-  CSSStyleSheetResource* resource =
-      CSSStyleSheetResource::CreateForTest(url, UTF8Encoding());
-  resource->SetStatus(ResourceStatus::kPending);
-
+  Platform::Current()->GetURLLoaderMockFactory()->RegisterURL(
+      url, WrappedResourceResponse(ResourceResponse()), "");
+  FetchParameters params{ResourceRequest(url)};
   PreloadRecordingCSSPreloaderResourceClient* resource_client =
-      new PreloadRecordingCSSPreloaderResourceClient(resource, preloader);
-
+      new PreloadRecordingCSSPreloaderResourceClient(preloader);
+  Resource* resource = CSSStyleSheetResource::Fetch(
+      params, dummy_page_holder->GetDocument().Fetcher(), resource_client);
   const char* data = "@import url('http://127.0.0.1/preload.css');";
   resource->AppendData(data, strlen(data));
 
   EXPECT_EQ(1u, resource_client->preload_urls_.size());
   EXPECT_EQ("http://127.0.0.1/preload.css",
             resource_client->preload_urls_.front());
+  Platform::Current()->GetURLLoaderMockFactory()->UnregisterURL(url);
 }
 
 // Regression test for crbug.com/608310 where the client is destroyed but was
@@ -114,11 +114,13 @@ TEST_F(CSSPreloadScannerTest, DestroyClientBeforeDataSent) {
       new CSSMockHTMLResourcePreloader(dummy_page_holder->GetDocument());
 
   KURL url("http://127.0.0.1/foo.css");
-  Persistent<CSSStyleSheetResource> resource =
-      CSSStyleSheetResource::CreateForTest(url, UTF8Encoding());
-  resource->SetStatus(ResourceStatus::kPending);
-
-  new PreloadRecordingCSSPreloaderResourceClient(resource, preloader);
+  Platform::Current()->GetURLLoaderMockFactory()->RegisterURL(
+      url, WrappedResourceResponse(ResourceResponse()), "");
+  FetchParameters params{ResourceRequest(url)};
+  PreloadRecordingCSSPreloaderResourceClient* resource_client =
+      new PreloadRecordingCSSPreloaderResourceClient(preloader);
+  Resource* resource = CSSStyleSheetResource::Fetch(
+      params, dummy_page_holder->GetDocument().Fetcher(), resource_client);
 
   // Destroys the resourceClient.
   ThreadState::Current()->CollectAllGarbage();
@@ -126,10 +128,11 @@ TEST_F(CSSPreloadScannerTest, DestroyClientBeforeDataSent) {
   const char* data = "@import url('http://127.0.0.1/preload.css');";
   // Should not crash.
   resource->AppendData(data, strlen(data));
+  Platform::Current()->GetURLLoaderMockFactory()->UnregisterURL(url);
 }
 
 // Regression test for crbug.com/646869 where the client's data is cleared
-// before didAppendFirstData is called.
+// before DataReceived() is called.
 TEST_F(CSSPreloadScannerTest, DontReadFromClearedData) {
   std::unique_ptr<DummyPageHolder> dummy_page_holder =
       DummyPageHolder::Create(IntSize(500, 500));
@@ -140,21 +143,18 @@ TEST_F(CSSPreloadScannerTest, DontReadFromClearedData) {
   CSSMockHTMLResourcePreloader* preloader =
       new CSSMockHTMLResourcePreloader(dummy_page_holder->GetDocument());
 
-  KURL url("http://127.0.0.1/foo.css");
-  CSSStyleSheetResource* resource =
-      CSSStyleSheetResource::CreateForTest(url, UTF8Encoding());
-
-  const char* data = "@import url('http://127.0.0.1/preload.css');";
-  resource->AppendData(data, strlen(data));
-  ResourceError error = ResourceError::Failure(url);
-  resource->FinishAsError(error, dummy_page_holder->GetDocument()
-                                     .GetTaskRunner(TaskType::kUnspecedLoading)
-                                     .get());
+  KURL url("data:text/css,@import url('http://127.0.0.1/preload.css');");
+  FetchParameters params{ResourceRequest(url)};
+  Resource* resource = CSSStyleSheetResource::Fetch(
+      params, dummy_page_holder->GetDocument().Fetcher(), nullptr);
+  ASSERT_FALSE(resource->ResourceBuffer());
 
   // Should not crash.
   PreloadRecordingCSSPreloaderResourceClient* resource_client =
-      new PreloadRecordingCSSPreloaderResourceClient(resource, preloader);
-
+      new PreloadRecordingCSSPreloaderResourceClient(preloader);
+  Resource* resource2 = CSSStyleSheetResource::Fetch(
+      params, dummy_page_holder->GetDocument().Fetcher(), resource_client);
+  ASSERT_EQ(resource, resource2);
   EXPECT_EQ(0u, resource_client->preload_urls_.size());
 }
 
@@ -171,12 +171,13 @@ TEST_F(CSSPreloadScannerTest, DoNotExpectValidDocument) {
       new CSSMockHTMLResourcePreloader(dummy_page_holder->GetDocument());
 
   KURL url("http://127.0.0.1/foo.css");
-  CSSStyleSheetResource* resource =
-      CSSStyleSheetResource::CreateForTest(url, UTF8Encoding());
-  resource->SetStatus(ResourceStatus::kPending);
-
+  Platform::Current()->GetURLLoaderMockFactory()->RegisterURL(
+      url, WrappedResourceResponse(ResourceResponse()), "");
+  FetchParameters params{ResourceRequest(url)};
   PreloadRecordingCSSPreloaderResourceClient* resource_client =
-      new PreloadRecordingCSSPreloaderResourceClient(resource, preloader);
+      new PreloadRecordingCSSPreloaderResourceClient(preloader);
+  Resource* resource = CSSStyleSheetResource::Fetch(
+      params, dummy_page_holder->GetDocument().Fetcher(), resource_client);
 
   dummy_page_holder->GetDocument().Shutdown();
 
@@ -186,6 +187,7 @@ TEST_F(CSSPreloadScannerTest, DoNotExpectValidDocument) {
   // Do not expect to gather any preloads, as the document loader is invalid,
   // which means we can't notify WebLoadingBehaviorData of the preloads.
   EXPECT_EQ(0u, resource_client->preload_urls_.size());
+  Platform::Current()->GetURLLoaderMockFactory()->UnregisterURL(url);
 }
 
 TEST_F(CSSPreloadScannerTest, ReferrerPolicyHeader) {
@@ -198,17 +200,20 @@ TEST_F(CSSPreloadScannerTest, ReferrerPolicyHeader) {
       dummy_page_holder->GetDocument(), "http://127.0.0.1/foo.css");
 
   KURL url("http://127.0.0.1/foo.css");
+  FetchParameters params{ResourceRequest(url)};
+  Platform::Current()->GetURLLoaderMockFactory()->RegisterURL(
+      url, WrappedResourceResponse(ResourceResponse()), "");
+
   ResourceResponse response;
   response.SetURL(url);
   response.SetHTTPStatusCode(200);
   response.SetHTTPHeaderField("referrer-policy", "unsafe-url");
-  CSSStyleSheetResource* resource =
-      CSSStyleSheetResource::CreateForTest(url, UTF8Encoding());
-  resource->SetStatus(ResourceStatus::kPending);
-  resource->SetResponse(response);
 
   PreloadRecordingCSSPreloaderResourceClient* resource_client =
-      new PreloadRecordingCSSPreloaderResourceClient(resource, preloader);
+      new PreloadRecordingCSSPreloaderResourceClient(preloader);
+  Resource* resource = CSSStyleSheetResource::Fetch(
+      params, dummy_page_holder->GetDocument().Fetcher(), resource_client);
+  resource->SetResponse(response);
 
   KURL preload_url("http://127.0.0.1/preload.css");
   Platform::Current()->GetURLLoaderMockFactory()->RegisterURL(
@@ -222,6 +227,8 @@ TEST_F(CSSPreloadScannerTest, ReferrerPolicyHeader) {
             resource_client->preload_urls_.front());
   EXPECT_EQ(kReferrerPolicyAlways,
             resource_client->preload_referrer_policies_.front());
+  Platform::Current()->GetURLLoaderMockFactory()->UnregisterURL(url);
+  Platform::Current()->GetURLLoaderMockFactory()->UnregisterURL(preload_url);
 }
 
 }  // namespace blink

third_party/WebKit/Source/core/html/parser/HTMLPreloadScannerTest.cpp

@@ -124,7 +124,7 @@ class HTMLMockHTMLResourcePreloader : public ResourcePreloader {
                                   Document* document,
                                   const char* expected_referrer) {
     PreloadRequestVerification(type, url, base_url, width, referrer_policy);
-    Resource* resource = preload_request_->Start(document);
+    Resource* resource = preload_request_->Start(document, nullptr);
     ASSERT_TRUE(resource);
     EXPECT_EQ(expected_referrer, resource->GetResourceRequest().HttpReferrer());
   }
@@ -144,7 +144,7 @@ class HTMLMockHTMLResourcePreloader : public ResourcePreloader {
       network::mojom::FetchRequestMode request_mode,
       network::mojom::FetchCredentialsMode credentials_mode) {
     ASSERT_TRUE(preload_request_.get());
-    Resource* resource = preload_request_->Start(document);
+    Resource* resource = preload_request_->Start(document, nullptr);
     ASSERT_TRUE(resource);
     EXPECT_EQ(request_mode,
               resource->GetResourceRequest().GetFetchRequestMode());

third_party/WebKit/Source/core/html/parser/HTMLResourcePreloader.cpp

@@ -76,19 +76,19 @@ void HTMLResourcePreloader::Preload(
   if (!document_->Loader())
     return;
 
-  Resource* resource = preload->Start(document_);
-
+  CSSPreloaderResourceClient* client = nullptr;
   // Don't scan a Resource more than once, to avoid a self-referencing
   // stlyesheet causing infinite recursion.
-  if (resource && !css_preloaders_.Contains(resource) &&
+  if (!css_preloaders_.Contains(preload->ResourceURL()) &&
       preload->ResourceType() == Resource::kCSSStyleSheet) {
     Settings* settings = document_->GetSettings();
     if (settings && (settings->GetCSSExternalScannerNoPreload() ||
                      settings->GetCSSExternalScannerPreload())) {
-      css_preloaders_.insert(resource,
-                             new CSSPreloaderResourceClient(resource, this));
+      client = new CSSPreloaderResourceClient(this);
+      css_preloaders_.insert(preload->ResourceURL(), client);
     }
   }
+  preload->Start(document_, client);
 }
 
 }  // namespace blink

third_party/WebKit/Source/core/html/parser/HTMLResourcePreloader.h

@@ -59,8 +59,7 @@ class CORE_EXPORT HTMLResourcePreloader
 
  private:
   Member<Document> document_;
-  HeapHashMap<Member<Resource>, Member<CSSPreloaderResourceClient>>
-      css_preloaders_;
+  HeapHashMap<String, Member<CSSPreloaderResourceClient>> css_preloaders_;
 
   DISALLOW_COPY_AND_ASSIGN(HTMLResourcePreloader);
 };

third_party/WebKit/Source/core/html/parser/PreloadRequest.cpp

@@ -30,7 +30,8 @@ KURL PreloadRequest::CompleteURL(Document* document) {
   return document->CompleteURL(resource_url_);
 }
 
-Resource* PreloadRequest::Start(Document* document) {
+Resource* PreloadRequest::Start(Document* document,
+                                CSSPreloaderResourceClient* client) {
   DCHECK(IsMainThread());
 
   FetchInitiatorInfo initiator_info;
@@ -105,7 +106,7 @@ Resource* PreloadRequest::Start(Document* document) {
     // the async request to the blocked script here.
   }
 
-  return document->Loader()->StartPreload(resource_type_, params);
+  return document->Loader()->StartPreload(resource_type_, params, client);
 }
 
 }  // namespace blink

third_party/WebKit/Source/core/html/parser/PreloadRequest.h

@@ -21,6 +21,7 @@
 
 namespace blink {
 
+class CSSPreloaderResourceClient;
 class Document;
 
 class CORE_EXPORT PreloadRequest {
@@ -67,7 +68,7 @@ class CORE_EXPORT PreloadRequest {
 
   bool IsSafeToSendToAnotherThread() const;
 
-  Resource* Start(Document*);
+  Resource* Start(Document*, CSSPreloaderResourceClient*);
 
   void SetDefer(FetchParameters::DeferOption defer) { defer_ = defer; }
   void SetCharset(const String& charset) { charset_ = charset.IsolatedCopy(); }

third_party/WebKit/Source/core/loader/DocumentLoader.cpp

@@ -43,6 +43,7 @@
 #include "core/frame/Settings.h"
 #include "core/frame/csp/ContentSecurityPolicy.h"
 #include "core/html/HTMLFrameOwnerElement.h"
+#include "core/html/parser/CSSPreloadScanner.h"
 #include "core/html/parser/HTMLParserIdioms.h"
 #include "core/html/parser/TextResourceDecoder.h"
 #include "core/inspector/ConsoleMessage.h"
@@ -196,8 +197,10 @@ const KURL& DocumentLoader::Url() const {
 }
 
 Resource* DocumentLoader::StartPreload(Resource::Type type,
-                                       FetchParameters& params) {
+                                       FetchParameters& params,
+                                       CSSPreloaderResourceClient* client) {
   Resource* resource = nullptr;
+  DCHECK(!client || type == Resource::kCSSStyleSheet);
   switch (type) {
     case Resource::kImage:
       if (frame_)
@@ -208,7 +211,7 @@ Resource* DocumentLoader::StartPreload(Resource::Type type,
       resource = ScriptResource::Fetch(params, Fetcher(), nullptr);
       break;
     case Resource::kCSSStyleSheet:
-      resource = CSSStyleSheetResource::Fetch(params, Fetcher(), nullptr);
+      resource = CSSStyleSheetResource::Fetch(params, Fetcher(), client);
       break;
     case Resource::kFont:
       resource = FontResource::Fetch(params, Fetcher());

third_party/WebKit/Source/core/loader/DocumentLoader.h

@@ -60,16 +60,17 @@
 namespace blink {
 
 class ApplicationCacheHost;
-class SubresourceFilter;
-class ResourceFetcher;
+class CSSPreloaderResourceClient;
 class Document;
 class DocumentParser;
+class FrameLoader;
 class HistoryItem;
 class LocalFrame;
 class LocalFrameClient;
-class FrameLoader;
+class ResourceFetcher;
 class ResourceTimingInfo;
 class SerializedScriptValue;
+class SubresourceFilter;
 class WebServiceWorkerNetworkProvider;
 struct ViewportDescriptionWrapper;
 
@@ -202,7 +203,9 @@ class CORE_EXPORT DocumentLoader
   void DispatchLinkHeaderPreloads(ViewportDescriptionWrapper*,
                                   LinkLoader::MediaPreloadPolicy);
 
-  Resource* StartPreload(Resource::Type, FetchParameters&);
+  Resource* StartPreload(Resource::Type,
+                         FetchParameters&,
+                         CSSPreloaderResourceClient*);
 
   void SetServiceWorkerNetworkProvider(
       std::unique_ptr<WebServiceWorkerNetworkProvider>);

third_party/WebKit/Source/core/loader/LinkLoader.cpp

@@ -392,7 +392,7 @@ static Resource* PreloadIfNeeded(const LinkRelAttribute& rel_attribute,
   }
   link_fetch_params.SetLinkPreload(true);
   return document.Loader()->StartPreload(resource_type.value(),
-                                         link_fetch_params);
+                                         link_fetch_params, nullptr);
 }
 
 // https://html.spec.whatwg.org/#link-type-modulepreload