Invoke WorkerScriptLoader's m_finishedCallback callback safely.

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 29 matching lines @@
 #include "wtf/RefPtr.h"
 
 namespace blink {
 
 WorkerScriptLoader::WorkerScriptLoader()
     : m_responseCallback(nullptr)
     , m_finishedCallback(nullptr)
     , m_failed(false)
     , m_identifier(0)
     , m_appCacheID(0)
-    , m_finishing(false)
     , m_requestContext(WebURLRequest::RequestContextWorker)
 {
 }
 
 WorkerScriptLoader::~WorkerScriptLoader()
 {
 }
 
 void WorkerScriptLoader::loadSynchronously(ExecutionContext& executionContext, const KURL& url, CrossOriginRequestPolicy crossOriginRequestPolicy)
 {
@@ skipping 27 matching lines @@
     if (!request)
         return;
 
     ThreadableLoaderOptions options;
     options.crossOriginRequestPolicy = crossOriginRequestPolicy;
 
     ResourceLoaderOptions resourceLoaderOptions;
     resourceLoaderOptions.allowCredentials = AllowStoredCredentials;
 
     m_threadableLoader = ThreadableLoader::create(executionContext, this, *request, options, resourceLoaderOptions);
+    if (m_failed)
+        notifyFinished();
+    // Do nothing here since notifyFinished() could delete |this|.
 }
 
 const KURL& WorkerScriptLoader::responseURL() const
 {
     ASSERT(!failed());
     return m_responseURL;
 }
 
 PassOwnPtr<ResourceRequest> WorkerScriptLoader::createResourceRequest()
 {
@@ skipping 79 matching lines @@
         m_threadableLoader->cancel();
 }
 
 String WorkerScriptLoader::script()
 {
     return m_script.toString();
 }
 
 void WorkerScriptLoader::notifyFinished()
 {
-    if (!m_finishedCallback || m_finishing)
+    ASSERT(m_failed);
+    if (!m_finishedCallback)
         return;
 
-    m_finishing = true;
-    if (m_finishedCallback)
-        (*m_finishedCallback)();
+    // notifyError() would be called before ThreadableLoader::create() returns

[kinuko, 2015/07/01 08:11:15]

nit: would be -> could be ?

[Takashi Toyoshima, 2015/07/02 05:55:03]

Done.

+    // e.g., from didFail(). Since m_finishedCallback potentially delete this

[kinuko, 2015/07/01 08:11:15]

could we also clearly note that null m_threadableLoader means that this is
called before ThreadableLoader::create() returns?

e.g.

"notifyError() could be called before ThreadableLoader::create() returns, e.g.
from didFail(), and in that case m_threadableLoader is not yet set (i.e. still
null)."

Also doing these checks in notifyError() might be clearer (so that we only call
notifyFinished once in the case too)?

[Takashi Toyoshima, 2015/07/02 05:55:03]

Done.

+    // object, the callback invocation should be postponed until the create()
+    // call returns.
+    if (!m_threadableLoader)
+        return;
+
+    OwnPtr<Closure> callback = m_finishedCallback.release();
+    (*callback)();
 }
 
 void WorkerScriptLoader::processContentSecurityPolicy(const ResourceResponse& response)
 {
     // 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, and
     // m_contentSecurityPolicy should be left as nullptr to inherit the policy).
     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