Invoke WorkerScriptLoader's m_finishedCallback callback safely.

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 15dee1bffecaf174eb8588d8fefcd8e87308edb9..20876b175ad5dbbffd21e3274841b36a7053780c 100644
--- a/Source/core/workers/WorkerScriptLoader.cpp
+++ b/Source/core/workers/WorkerScriptLoader.cpp
@@ -47,7 +47,6 @@ WorkerScriptLoader::WorkerScriptLoader()
     , m_failed(false)
     , m_identifier(0)
     , m_appCacheID(0)
-    , m_finishing(false)
     , m_requestContext(WebURLRequest::RequestContextWorker)
 {
 }
@@ -95,6 +94,9 @@ void WorkerScriptLoader::loadAsynchronously(ExecutionContext& executionContext,
     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
@@ -194,12 +196,19 @@ String WorkerScriptLoader::script()
 
 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)