Remove WorkerScriptLoaderClient and inheritances

Source/core/workers/WorkerScriptLoader.cpp

 /*
  * Copyright (C) 2009 Apple Inc. All Rights Reserved.
  * Copyright (C) 2009, 2011 Google Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 14 matching lines @@
  *
  */
 
 #include "config.h"
 #include "core/workers/WorkerScriptLoader.h"
 
 #include "core/dom/ExecutionContext.h"
 #include "core/html/parser/TextResourceDecoder.h"
 #include "core/loader/WorkerThreadableLoader.h"
 #include "core/workers/WorkerGlobalScope.h"
-#include "core/workers/WorkerScriptLoaderClient.h"
 #include "platform/network/ContentSecurityPolicyResponseHeaders.h"
 #include "platform/network/ResourceResponse.h"
 #include "public/platform/WebURLRequest.h"
 
 #include "wtf/OwnPtr.h"
 #include "wtf/RefPtr.h"
 
 namespace blink {
 
 WorkerScriptLoader::WorkerScriptLoader()
-    : m_client(nullptr)
+    : m_responseCallback(nullptr)
+    , m_finishedCallback(nullptr)
     , m_failed(false)
     , m_identifier(0)
+    , m_appCacheID(0)
     , m_finishing(false)
     , m_requestContext(WebURLRequest::RequestContextWorker)
 {
 }
 
-WorkerScriptLoader::~WorkerScriptLoader()
-{
-}
+WorkerScriptLoader::~WorkerScriptLoader() = default;

[kinuko, 2015/06/24 07:27:54]

nit: it's now allowed and ok, but in this case it doesn't look too useful to use
this

[Takashi Toyoshima, 2015/06/24 08:53:45]

ok, revert this part.

 
 void WorkerScriptLoader::loadSynchronously(ExecutionContext& executionContext, const KURL& url, CrossOriginRequestPolicy crossOriginRequestPolicy)
 {
     m_url = url;
 
     OwnPtr<ResourceRequest> request(createResourceRequest());
     if (!request)
         return;
 
     ASSERT_WITH_SECURITY_IMPLICATION(executionContext.isWorkerGlobalScope());
 
     ThreadableLoaderOptions options;
     options.crossOriginRequestPolicy = crossOriginRequestPolicy;
     // FIXME: Should we add EnforceScriptSrcDirective here?
     options.contentSecurityPolicyEnforcement = DoNotEnforceContentSecurityPolicy;
 
     ResourceLoaderOptions resourceLoaderOptions;
     resourceLoaderOptions.allowCredentials = AllowStoredCredentials;
 
     WorkerThreadableLoader::loadResourceSynchronously(toWorkerGlobalScope(executionContext), *request, *this, options, resourceLoaderOptions);
 }
 
-void WorkerScriptLoader::loadAsynchronously(ExecutionContext& executionContext, const KURL& url, CrossOriginRequestPolicy crossOriginRequestPolicy, WorkerScriptLoaderClient* client)
+void WorkerScriptLoader::loadAsynchronously(ExecutionContext& executionContext, const KURL& url, CrossOriginRequestPolicy crossOriginRequestPolicy, PassOwnPtr<Closure> responseCallback, PassOwnPtr<Closure> finishedCallback)
 {
-    ASSERT(client);
-    m_client = client;
+    ASSERT(responseCallback || finishedCallback);
+    m_responseCallback = responseCallback;
+    m_finishedCallback = finishedCallback;
     m_url = url;
 
     OwnPtr<ResourceRequest> request(createResourceRequest());
     if (!request)
         return;
 
     ThreadableLoaderOptions options;
     options.crossOriginRequestPolicy = crossOriginRequestPolicy;
 
     ResourceLoaderOptions resourceLoaderOptions;
     resourceLoaderOptions.allowCredentials = AllowStoredCredentials;
 
-    // During create, callbacks may happen which remove the last reference to this object.
-    RefPtr<WorkerScriptLoader> protect(this);
     m_threadableLoader = ThreadableLoader::create(executionContext, this, *request, options, resourceLoaderOptions);
 }
 
 const KURL& WorkerScriptLoader::responseURL() const
 {
     ASSERT(!failed());
     return m_responseURL;
 }
 
 PassOwnPtr<ResourceRequest> WorkerScriptLoader::createResourceRequest()
 {
     OwnPtr<ResourceRequest> request = adoptPtr(new ResourceRequest(m_url));
     request->setHTTPMethod("GET");
     request->setRequestContext(m_requestContext);
     return request.release();
 }
 
 void WorkerScriptLoader::didReceiveResponse(unsigned long identifier, const ResourceResponse& response, PassOwnPtr<WebDataConsumerHandle> handle)
 {
     ASSERT_UNUSED(handle, !handle);
     if (response.httpStatusCode() / 100 != 2 && response.httpStatusCode()) {
         m_failed = true;
         return;
     }
+    m_identifier = identifier;
     m_responseURL = response.url();
     m_responseEncoding = response.textEncodingName();
+    m_appCacheID = response.appCacheID();
     processContentSecurityPolicy(response);
-    if (m_client)
-        m_client->didReceiveResponse(identifier, response);
+    if (m_responseCallback)
+        (*m_responseCallback)();
 }
 
 void WorkerScriptLoader::didReceiveData(const char* data, unsigned len)
 {
     if (m_failed)
         return;
 
     if (!m_decoder) {
         if (!m_responseEncoding.isEmpty())
             m_decoder = TextResourceDecoder::create("text/javascript", m_responseEncoding);
@@ skipping 16 matching lines @@
 void WorkerScriptLoader::didFinishLoading(unsigned long identifier, double)
 {
     if (m_failed) {
         notifyError();
         return;
     }
 
     if (m_decoder)
         m_script.append(m_decoder->flush());
 
-    m_identifier = identifier;
-    notifyFinished();
+    if (m_finishedCallback)
+        (*m_finishedCallback)();
 }
 
 void WorkerScriptLoader::didFail(const ResourceError&)
 {
     notifyError();
 }
 
 void WorkerScriptLoader::didFailRedirectCheck()
 {
     notifyError();
@@ skipping 11 matching lines @@
         m_threadableLoader->cancel();
 }
 
 String WorkerScriptLoader::script()
 {
     return m_script.toString();
 }
 
 void WorkerScriptLoader::notifyFinished()
 {
-    if (!m_client || m_finishing)
+    if (!m_finishedCallback || m_finishing)
         return;
 
     m_finishing = true;
-    m_client->notifyFinished();
+    if (m_finishedCallback)
+        (*m_finishedCallback)();
 }
 
 void WorkerScriptLoader::processContentSecurityPolicy(const ResourceResponse& response)
 {
+    // Should not create an empty ContentSecurityPolicy object for other cases

[Takashi Toyoshima, 2015/06/23 12:00:46]

Kinuko: this comment describes the point I talked offline today.

[Mike West, 2015/06/24 07:18:33]

I'd rephrase this comment a bit. Something like: "Per
http://www.w3.org/TR/CSP2/#processing-model-workers, if the Worker's URL is not
a GUID, then it grabs its CSP from the response headers directly. Otherwise, the
Worker inherits the policy from the parent document (which is implemented in
WorkerMessagingProxy)."

[kinuko, 2015/06/24 07:27:54]

Ok, I found the relevant part:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[Takashi Toyoshima, 2015/06/24 08:53:45]

Thanks, Mike.
I'll replace this comment with your suggestion.
But, I still want to keep following part since it's a little tricky.
"m_contentSecurityPolicy should be left as nullptr to inherit the policy"

+    // so that WorkerMessagingProxy can pick up the right instance in order to
+    // inherit the parent document's ContentSecurityPolicy.
     if (!response.url().protocolIs("blob") && !response.url().protocolIs("file") && !response.url().protocolIs("filesystem")) {
         m_contentSecurityPolicy = ContentSecurityPolicy::create();
         m_contentSecurityPolicy->setOverrideURLForSelf(response.url());
         m_contentSecurityPolicy->didReceiveHeaders(ContentSecurityPolicyResponseHeaders(response));
     }
 }
 
 } // namespace blink