AssociatedURLLoader shouldn't derive from WebURLLoader

content/renderer/fetchers/resource_fetcher_impl.cc

Index: content/renderer/fetchers/resource_fetcher_impl.cc
diff --git a/content/renderer/fetchers/resource_fetcher_impl.cc b/content/renderer/fetchers/resource_fetcher_impl.cc
index a1a760fdc8eea30fd98334da65588a75f8fd4f70..23e2196f20a7b04e4f24950fe417e38d49bd67e3 100644
--- a/content/renderer/fetchers/resource_fetcher_impl.cc
+++ b/content/renderer/fetchers/resource_fetcher_impl.cc
@@ -4,15 +4,20 @@
 
 #include "content/renderer/fetchers/resource_fetcher_impl.h"
 
+#include <stdint.h>
+
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "third_party/WebKit/public/platform/Platform.h"
 #include "third_party/WebKit/public/platform/WebHTTPBody.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
+#include "third_party/WebKit/public/platform/WebURLError.h"
 #include "third_party/WebKit/public/platform/WebURLLoader.h"
-#include "third_party/WebKit/public/platform/WebURLRequest.h"
+#include "third_party/WebKit/public/platform/WebURLLoaderClient.h"
+#include "third_party/WebKit/public/platform/WebURLResponse.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebKit.h"
@@ -25,12 +30,116 @@ ResourceFetcher* ResourceFetcher::Create(const GURL& url) {
   return new ResourceFetcherImpl(url);
 }
 
+class ResourceFetcherImpl::ClientImpl : public blink::WebURLLoaderClient {
+ public:
+  ClientImpl(ResourceFetcherImpl* parent, const Callback& callback)
+      : parent_(parent),
+        completed_(false),
+        status_(LOADING),
+        callback_(callback) {}
+
+  ~ClientImpl() override {}
+
+  virtual void Cancel() { OnLoadCompleteInternal(LOAD_FAILED); }
+
+  bool completed() const { return completed_; }
+
+ private:
+  enum LoadStatus {
+    LOADING,
+    LOAD_FAILED,
+    LOAD_SUCCEEDED,
+  };
+
+  void OnLoadCompleteInternal(LoadStatus status) {
+    DCHECK(!completed_);
+    DCHECK(status_ == LOADING);
+
+    completed_ = true;
+    status_ = status;
+
+    parent_->OnLoadComplete();
+
+    if (callback_.is_null())
+      return;
+
+    // Take a reference to the callback as running the callback may lead to our
+    // destruction.
+    Callback callback = callback_;
+    callback.Run(status_ == LOAD_FAILED ? blink::WebURLResponse() : response_,
+                 status_ == LOAD_FAILED ? std::string() : data_);
+  }
+
+  // WebURLLoaderClient methods:
+  void didReceiveResponse(blink::WebURLLoader* loader,
+                          const blink::WebURLResponse& response) override {
+    DCHECK(!completed_);
+    response_ = response;
+  }
+  void didReceiveCachedMetadata(blink::WebURLLoader* loader,
+                                const char* data,
+                                int data_length) override {
+    DCHECK(!completed_);
+    DCHECK(data_length > 0);
+  }
+  void didReceiveData(blink::WebURLLoader* loader,
+                      const char* data,
+                      int data_length,
+                      int encoded_data_length,
+                      int encoded_body_length) override {
+    // The AssociatedURLLoader will continue after a load failure.

[kinuko, 2016/10/14 03:15:20]

Do we still need these behaviors in this class?

[tyoshino (SeeGerritForStatus), 2016/10/14 06:12:06]

Good catch! Replaced this and one in didFinishLoading with DCHECK().

+    // For example, for an Access Control error.
+    if (completed_)
+      return;
+    DCHECK(data_length > 0);
+
+    data_.append(data, data_length);
+  }
+  void didFinishLoading(blink::WebURLLoader* loader,
+                        double finishTime,
+                        int64_t total_encoded_data_length) override {
+    // The AssociatedURLLoader will continue after a load failure.
+    // For example, for an Access Control error.
+    if (completed_)
+      return;
+    OnLoadCompleteInternal(LOAD_SUCCEEDED);
+  }
+  void didFail(blink::WebURLLoader* loader,
+               const blink::WebURLError& error) override {
+    OnLoadCompleteInternal(LOAD_FAILED);
+  }
+
+ private:
+  ResourceFetcherImpl* parent_;
+
+  // Set to true once the request is complete.
+  bool completed_;
+
+  // Buffer to hold the content from the server.
+  std::string data_;
+
+  // A copy of the original resource response.
+  blink::WebURLResponse response_;
+
+  LoadStatus status_;
+
+  // Callback when we're done.
+  Callback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(ClientImpl);
+};
+
 ResourceFetcherImpl::ResourceFetcherImpl(const GURL& url)
     : request_(url) {
 }
 
 ResourceFetcherImpl::~ResourceFetcherImpl() {
-  if (!completed() && loader_)
+  if (!loader_)
+    return;
+
+  DCHECK(client_);
+
+  if (!client_->completed())
     loader_->cancel();
 }
 
@@ -69,58 +178,26 @@ void ResourceFetcherImpl::SetHeader(const std::string& header,
   }
 }
 
-void ResourceFetcherImpl::SetSkipServiceWorker(
-    blink::WebURLRequest::SkipServiceWorker skip_service_worker) {
-  DCHECK(!request_.isNull());
-  DCHECK(!loader_);
-
-  request_.setSkipServiceWorker(skip_service_worker);
-}
-
-void ResourceFetcherImpl::SetCachePolicy(blink::WebCachePolicy policy) {
-  DCHECK(!request_.isNull());
-  DCHECK(!loader_);
-
-  request_.setCachePolicy(policy);
-}
-
-void ResourceFetcherImpl::SetLoaderOptions(
-    const blink::WebURLLoaderOptions& options) {
-  DCHECK(!request_.isNull());
-  DCHECK(!loader_);
-
-  options_ = options;
-}
-
 void ResourceFetcherImpl::Start(
     blink::WebFrame* frame,
     blink::WebURLRequest::RequestContext request_context,
     blink::WebURLRequest::FrameType frame_type,
-    LoaderType loader_type,
     const Callback& callback) {
   DCHECK(!loader_);
+  DCHECK(!client_);
   DCHECK(!request_.isNull());
-  DCHECK(callback_.is_null());
-  DCHECK(!completed());
   if (!request_.httpBody().isNull())
     DCHECK_NE("GET", request_.httpMethod().utf8()) << "GETs can't have bodies.";
 
-  callback_ = callback;
-
   request_.setRequestContext(request_context);
   request_.setFrameType(frame_type);
   request_.setFirstPartyForCookies(frame->document().firstPartyForCookies());
   frame->dispatchWillSendRequest(request_);
 
-  switch (loader_type) {
-    case PLATFORM_LOADER:
-      loader_.reset(blink::Platform::current()->createURLLoader());
-      break;
-    case FRAME_ASSOCIATED_LOADER:
-      loader_.reset(frame->createAssociatedURLLoader(options_));
-      break;
-  }
-  loader_->loadAsynchronously(request_, this);
+  client_.reset(new ClientImpl(this, callback));
+
+  loader_.reset(blink::Platform::current()->createURLLoader());
+  loader_->loadAsynchronously(request_, client_.get());
 
   // No need to hold on to the request; reset it now.
   request_ = blink::WebURLRequest();
@@ -128,26 +205,19 @@ void ResourceFetcherImpl::Start(
 
 void ResourceFetcherImpl::SetTimeout(const base::TimeDelta& timeout) {
   DCHECK(loader_);
-  DCHECK(!completed());
+  DCHECK(client_);
+  DCHECK(!client_->completed());
 
   timeout_timer_.Start(FROM_HERE, timeout, this, &ResourceFetcherImpl::Cancel);
 }
 
 void ResourceFetcherImpl::OnLoadComplete() {
   timeout_timer_.Stop();
-  if (callback_.is_null())
-    return;
-
-  // Take a reference to the callback as running the callback may lead to our
-  // destruction.
-  Callback callback = callback_;
-  callback.Run(status() == LOAD_FAILED ? blink::WebURLResponse() : response(),
-               status() == LOAD_FAILED ? std::string() : data());
 }
 
 void ResourceFetcherImpl::Cancel() {
   loader_->cancel();
-  WebURLLoaderClientImpl::Cancel();
+  client_->Cancel();
 }
 
 }  // namespace content