Remove WorkerScriptLoaderClient and inheritances

Source/core/workers/WorkerScriptLoader.cpp

Index: Source/core/workers/WorkerScriptLoader.cpp
diff --git a/Source/core/workers/WorkerScriptLoader.cpp b/Source/core/workers/WorkerScriptLoader.cpp
index 7b30539fe2394cef581fe5adb8c8152f918f0853..50b86de5017304943038d5dad29b8ccd556f4473 100644
--- a/Source/core/workers/WorkerScriptLoader.cpp
+++ b/Source/core/workers/WorkerScriptLoader.cpp
@@ -32,7 +32,6 @@
 #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"
@@ -43,17 +42,17 @@
 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)
 {
@@ -76,10 +75,11 @@ void WorkerScriptLoader::loadSynchronously(ExecutionContext& executionContext, c
     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());
@@ -92,8 +92,6 @@ void WorkerScriptLoader::loadAsynchronously(ExecutionContext& executionContext,
     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);
 }
 
@@ -118,11 +116,13 @@ void WorkerScriptLoader::didReceiveResponse(unsigned long identifier, const Reso
         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)
@@ -159,8 +159,8 @@ void WorkerScriptLoader::didFinishLoading(unsigned long identifier, double)
     if (m_decoder)
         m_script.append(m_decoder->flush());
 
-    m_identifier = identifier;
-    notifyFinished();
+    if (m_finishedCallback)
+        (*m_finishedCallback)();
 }
 
 void WorkerScriptLoader::didFail(const ResourceError&)
@@ -192,15 +192,19 @@ String WorkerScriptLoader::script()
 
 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());