Reland "Make DocumentThreadableLoader a ResourceOwner"

It had a custom implementation to support use of RawResourceClientStateChecker.
Separate these.

The original version (https://chromium.googlesource.com/chromium/src/+/ae15c13530084329ac2509ccded45428d7d7d2c8)
failed to update DocumentThreadableLoader::Trace(). It introduced an intermediate
subclass, ResourceOwner<RawResource>, between DocumentThreadableLoader and RawResourceClient, but
because Trace() wasn't updated, ResourceOwner::resource_ was left untraced. The GC clang plugin
would normally catch a mistake like this, but the plugin ignores ResourceOwner due to
https://crbug.com/652966, so it wasn't detected at compile time.

Bug: 790778, 640291
Change-Id: I114df7eaa97139fbb6aa70b22b2f41019988631f
Reviewed-on: https://chromium-review.googlesource.com/806753
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521490}

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

@@ -53,6 +53,7 @@
 #include "platform/loader/fetch/FetchUtils.h"
 #include "platform/loader/fetch/Resource.h"
 #include "platform/loader/fetch/ResourceFetcher.h"
+#include "platform/loader/fetch/ResourceLoader.h"
 #include "platform/loader/fetch/ResourceLoaderOptions.h"
 #include "platform/loader/fetch/ResourceRequest.h"
 #include "platform/weborigin/SchemeRegistry.h"
@@ -536,7 +537,7 @@ void DocumentThreadableLoader::MakeCrossOriginAccessRequestBlinkCORS(
 
 DocumentThreadableLoader::~DocumentThreadableLoader() {
   CHECK(!client_);
-  DCHECK(!resource_);
+  DCHECK(!GetResource());
 }
 
 void DocumentThreadableLoader::OverrideTimeout(
@@ -585,14 +586,16 @@ void DocumentThreadableLoader::Detach() {
 }
 
 void DocumentThreadableLoader::SetDefersLoading(bool value) {
-  if (GetResource())
-    GetResource()->SetDefersLoading(value);
+  if (GetResource() && GetResource()->Loader())
+    GetResource()->Loader()->SetDefersLoading(value);
 }
 
 void DocumentThreadableLoader::Clear() {
   client_ = nullptr;
   timeout_timer_.Stop();
   request_started_seconds_ = 0.0;
+  if (GetResource())
+    checker_.WillRemoveClient();
   ClearResource();
 }
 
@@ -734,6 +737,8 @@ bool DocumentThreadableLoader::RedirectReceivedBlinkCORS(
 
   // FIXME: consider combining this with CORS redirect handling performed by
   // CrossOriginAccessControl::handleRedirect().
+  if (GetResource())
+    checker_.WillRemoveClient();
   ClearResource();
 
   // If
@@ -1101,6 +1106,8 @@ void DocumentThreadableLoader::DidTimeout(TimerBase* timer) {
 }
 
 void DocumentThreadableLoader::LoadFallbackRequestForServiceWorker() {
+  if (GetResource())
+    checker_.WillRemoveClient();
   ClearResource();
   ResourceRequest fallback_request(fallback_request_for_service_worker_);
   fallback_request_for_service_worker_ = ResourceRequest();
@@ -1116,6 +1123,8 @@ void DocumentThreadableLoader::LoadActualRequest() {
   actual_request_ = ResourceRequest();
   actual_options_ = ResourceLoaderOptions();
 
+  if (GetResource())
+    checker_.WillRemoveClient();
   ClearResource();
 
   PrepareCrossOriginRequest(actual_request);
@@ -1200,6 +1209,8 @@ void DocumentThreadableLoader::LoadRequestAsync(
   } else {
     SetResource(RawResource::Fetch(new_params, fetcher));
   }
+  if (GetResource())
+    checker_.WillAddClient();
 
   if (!GetResource()) {
     probe::documentThreadableLoaderFailedToStartLoadingForClient(
@@ -1365,10 +1376,9 @@ ExecutionContext* DocumentThreadableLoader::GetExecutionContext() const {
 }
 
 void DocumentThreadableLoader::Trace(blink::Visitor* visitor) {
-  visitor->Trace(resource_);
   visitor->Trace(loading_context_);
   ThreadableLoader::Trace(visitor);
-  RawResourceClient::Trace(visitor);
+  ResourceOwner<RawResource>::Trace(visitor);
 }
 
 }  // namespace blink

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

@@ -57,8 +57,9 @@ class ThreadableLoadingContext;
 
 // TODO(horo): We are using this class not only in documents, but also in
 // workers. We should change the name to ThreadableLoaderImpl.
-class CORE_EXPORT DocumentThreadableLoader final : public ThreadableLoader,
-                                                   private RawResourceClient {
+class CORE_EXPORT DocumentThreadableLoader final
+    : public ThreadableLoader,
+      private ResourceOwner<RawResource> {
   USING_GARBAGE_COLLECTED_MIXIN(DocumentThreadableLoader);
 
  public:
@@ -203,32 +204,6 @@ class CORE_EXPORT DocumentThreadableLoader final : public ThreadableLoader,
   void LoadRequest(ResourceRequest&, ResourceLoaderOptions);
   bool IsAllowedRedirect(network::mojom::FetchRequestMode, const KURL&) const;
 
-  // TODO(hiroshige): After crbug.com/633696 is fixed,
-  // - Remove RawResourceClientStateChecker logic,
-  // - Make DocumentThreadableLoader to be a ResourceOwner and remove this
-  //   re-implementation of ResourceOwner, and
-  // - Consider re-applying RawResourceClientStateChecker in a more
-  //   general fashion (crbug.com/640291).
-  RawResource* GetResource() const { return resource_.Get(); }
-  void ClearResource() { SetResource(nullptr); }
-  void SetResource(RawResource* new_resource) {
-    if (new_resource == resource_)
-      return;
-
-    if (RawResource* old_resource = resource_.Release()) {
-      checker_.WillRemoveClient();
-      old_resource->RemoveClient(this);
-    }
-
-    if (new_resource) {
-      resource_ = new_resource;
-      checker_.WillAddClient();
-      resource_->AddClient(this);
-    }
-  }
-  Member<RawResource> resource_;
-  // End of ResourceOwner re-implementation, see above.
-
   SecurityOrigin* GetSecurityOrigin() const;
 
   // Returns null if the loader is not associated with Document.

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

@@ -31,7 +31,6 @@
 #include "platform/loader/fetch/MemoryCache.h"
 #include "platform/loader/fetch/ResourceClientWalker.h"
 #include "platform/loader/fetch/ResourceFetcher.h"
-#include "platform/loader/fetch/ResourceLoader.h"
 #include "platform/network/http_names.h"
 #include "platform/scheduler/child/web_scheduler.h"
 #include "public/platform/Platform.h"
@@ -296,11 +295,6 @@ void RawResource::NotifyFinished() {
   Resource::NotifyFinished();
 }
 
-void RawResource::SetDefersLoading(bool defers) {
-  if (Loader())
-    Loader()->SetDefersLoading(defers);
-}
-
 static bool ShouldIgnoreHeaderForCacheReuse(AtomicString header_name) {
   // FIXME: This list of headers that don't affect cache policy almost certainly
   // isn't complete.

third_party/WebKit/Source/platform/loader/fetch/RawResource.h

@@ -66,12 +66,6 @@ class PLATFORM_EXPORT RawResource final : public Resource {
     return CreateForTest(KURL(url), type);
   }
 
-  // FIXME: AssociatedURLLoader shouldn't be a DocumentThreadableLoader and
-  // therefore shouldn't use RawResource. However, it is, and it needs to be
-  // able to defer loading. This can be fixed by splitting CORS preflighting out
-  // of DocumentThreadableLoader.
-  void SetDefersLoading(bool);
-
   // Resource implementation
   bool CanReuse(const FetchParameters&) const override;
   bool WillFollowRedirect(const ResourceRequest&,