[webcrypto] Make it safe to delete WebCryptoResult from any thread.

Source/modules/crypto/CryptoResultImpl.cpp

Index: Source/modules/crypto/CryptoResultImpl.cpp
diff --git a/Source/modules/crypto/CryptoResultImpl.cpp b/Source/modules/crypto/CryptoResultImpl.cpp
index 11015d4abdde9d77e4354399cacb02eefa7581d0..af10ba9516aa792293b6749de9434784855ae66a 100644
--- a/Source/modules/crypto/CryptoResultImpl.cpp
+++ b/Source/modules/crypto/CryptoResultImpl.cpp
@@ -33,6 +33,7 @@
 
 #include "bindings/v8/NewScriptState.h"
 #include "bindings/v8/ScriptPromiseResolverWithContext.h"
+#include "core/dom/ContextLifecycleObserver.h"
 #include "core/dom/ExecutionContext.h"
 #include "modules/crypto/Key.h"
 #include "modules/crypto/KeyPair.h"
@@ -44,119 +45,140 @@
 
 namespace WebCore {
 
-CryptoResultImpl::~CryptoResultImpl()
-{
-}
+// The PromiseState class contains all the state which is tied to an
+// ExecutionContext. Whereas CryptoResultImpl can be deleted from any thread,
+// PromiseState is not thread safe and must only be accessed and deleted from
+// the blink thread.
+//
+// This is achieved by making CryptoResultImpl hold a WeakPtr to the PromiseState.
+// The PromiseState deletes itself after being notified of completion.
+// Additionally the PromiseState deletes itself when the ExecutionContext is
+// destroyed (necessary to avoid leaks when dealing with WebWorker threads,
+// which may die before the operation is completed).
+class CryptoResultImpl::PromiseState FINAL : public ContextLifecycleObserver {
+public:
+    static WeakPtr<PromiseState> create()
+    {
+        ExecutionContext* context = callingExecutionContext(v8::Isolate::GetCurrent());

[abarth-chromium, 2014/04/25 23:26:14]

Can we pass in the isolate?

[eroman, 2014/04/26 00:23:44]

Done (I think).

+        PromiseState* promiseState = new PromiseState(context);
+        return promiseState->m_weakFactory.createWeakPtr();
+    }
 
-PassRefPtr<CryptoResultImpl> CryptoResultImpl::create()
-{
-    return adoptRef(new CryptoResultImpl(callingExecutionContext(v8::Isolate::GetCurrent())));
-}
+    // Override from ContextLifecycleObserver
+    virtual void contextDestroyed() OVERRIDE
+    {
+        ContextLifecycleObserver::contextDestroyed();
+        delete this;
+    }
 
-void CryptoResultImpl::completeWithError(const blink::WebString& errorDetails)
-{
-    ASSERT(!m_finished);
+    ScriptPromise promise()
+    {
+        return m_promiseResolver->promise();
+    }
 
-    if (canCompletePromise()) {
+    void completeWithError(const blink::WebString& errorDetails)
+    {
         if (!errorDetails.isEmpty()) {
             // FIXME: Include the line number which started the crypto operation.
             executionContext()->addConsoleMessage(JSMessageSource, ErrorMessageLevel, errorDetails);
         }
         m_promiseResolver->reject(V8NullType());
+        delete this;
     }
-}
-
-void CryptoResultImpl::completeWithError()
-{
-    completeWithError(blink::WebString());
-}
 
-void CryptoResultImpl::completeWithBuffer(const blink::WebArrayBuffer& buffer)
-{
-    ASSERT(!m_finished);
+    void completeWithError()
+    {
+        completeWithError(blink::WebString());
+    }
 
-    if (canCompletePromise()) {
+    void completeWithBuffer(const blink::WebArrayBuffer& buffer)
+    {
         m_promiseResolver->resolve(PassRefPtr<ArrayBuffer>(buffer));
+        delete this;
     }
 
-    finish();
-}
-
-void CryptoResultImpl::completeWithBoolean(bool b)
-{
-    ASSERT(!m_finished);
-
-    if (canCompletePromise()) {
+    void completeWithBoolean(bool b)
+    {
         m_promiseResolver->resolve(b);
+        delete this;
     }
 
-    finish();
-}
+    void completeWithKey(const blink::WebCryptoKey& key)
+    {
+        m_promiseResolver->resolve(Key::create(key));
+        delete this;
+    }
 
-void CryptoResultImpl::completeWithKey(const blink::WebCryptoKey& key)
-{
-    ASSERT(!m_finished);
+    void completeWithKeyPair(const blink::WebCryptoKey& publicKey, const blink::WebCryptoKey& privateKey)
+    {
+        m_promiseResolver->resolve(KeyPair::create(publicKey, privateKey));
+        delete this;
+    }
 
-    if (canCompletePromise()) {
-        m_promiseResolver->resolve(Key::create(key));
+private:
+    explicit PromiseState(ExecutionContext* context)
+        : ContextLifecycleObserver(context)
+        , m_weakFactory(this)
+        , m_promiseResolver(ScriptPromiseResolverWithContext::create(NewScriptState::current(toIsolate(context))))
+    {
     }
 
-    finish();
-}
+    WeakPtrFactory<PromiseState> m_weakFactory;
+    RefPtr<ScriptPromiseResolverWithContext> m_promiseResolver;
+};
 
-void CryptoResultImpl::completeWithKeyPair(const blink::WebCryptoKey& publicKey, const blink::WebCryptoKey& privateKey)
+CryptoResultImpl::~CryptoResultImpl()
 {
-    ASSERT(!m_finished);
+}
 
-    if (canCompletePromise()) {
-        m_promiseResolver->resolve(KeyPair::create(publicKey, privateKey));
-    }
+PassRefPtr<CryptoResultImpl> CryptoResultImpl::create()
+{
+    return adoptRef(new CryptoResultImpl);
+}
 
-    finish();
+void CryptoResultImpl::completeWithError(const blink::WebString& errorDetails)
+{
+    if (m_promiseState)
+        m_promiseState->completeWithError(errorDetails);
 }
 
-CryptoResultImpl::CryptoResultImpl(ExecutionContext* context)
-    : ContextLifecycleObserver(context)
-    , m_promiseResolver(ScriptPromiseResolverWithContext::create(NewScriptState::current(toIsolate(context))))
-#if !ASSERT_DISABLED
-    , m_owningThread(currentThread())
-    , m_finished(false)
-#endif
+void CryptoResultImpl::completeWithError()
 {
+    completeWithError(blink::WebString());
 }
 
-void CryptoResultImpl::finish()
+void CryptoResultImpl::completeWithBuffer(const blink::WebArrayBuffer& buffer)
 {
-#if !ASSERT_DISABLED
-    m_finished = true;
-#endif
-    clearPromiseResolver();
+    if (m_promiseState)
+        m_promiseState->completeWithBuffer(buffer);
 }
 
-void CryptoResultImpl::clearPromiseResolver()
+void CryptoResultImpl::completeWithBoolean(bool b)
 {
-    m_promiseResolver.clear();
+    if (m_promiseState)
+        m_promiseState->completeWithBoolean(b);
 }
 
-void CryptoResultImpl::CheckValidThread() const
+void CryptoResultImpl::completeWithKey(const blink::WebCryptoKey& key)
 {
-    ASSERT(m_owningThread == currentThread());
+    if (m_promiseState)
+        m_promiseState->completeWithKey(key);
 }
 
-void CryptoResultImpl::contextDestroyed()
+void CryptoResultImpl::completeWithKeyPair(const blink::WebCryptoKey& publicKey, const blink::WebCryptoKey& privateKey)
 {
-    ContextLifecycleObserver::contextDestroyed();
+    if (m_promiseState)
+        m_promiseState->completeWithKeyPair(publicKey, privateKey);
+}
 
-    // Abandon the promise without completing it when the context goes away.
-    clearPromiseResolver();
-    ASSERT(!canCompletePromise());
+CryptoResultImpl::CryptoResultImpl()
+    : m_promiseState(PromiseState::create())
+{
 }
 
-bool CryptoResultImpl::canCompletePromise() const
+ScriptPromise CryptoResultImpl::promise()
 {
-    CheckValidThread();
-    ExecutionContext* context = executionContext();
-    return context && !context->activeDOMObjectsAreSuspended() && !context->activeDOMObjectsAreStopped();
+    return m_promiseState->promise();
 }
 
 } // namespace WebCore