Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2129)

Unified Diff: Source/modules/crypto/CryptoResultImpl.cpp

Issue 253563002: [webcrypto] Make it safe to delete WebCryptoResult from any thread. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Rebase Created 6 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/modules/crypto/CryptoResultImpl.h ('k') | Source/platform/CryptoResult.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/modules/crypto/CryptoResultImpl.cpp
diff --git a/Source/modules/crypto/CryptoResultImpl.cpp b/Source/modules/crypto/CryptoResultImpl.cpp
index 11015d4abdde9d77e4354399cacb02eefa7581d0..794632fe13309fb4369420d39ff1b6e9c38f1439 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,139 @@
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)
+ {
+ 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(callingExecutionContext(v8::Isolate::GetCurrent())));
+}
- 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(ExecutionContext* context)
+ : m_promiseState(PromiseState::create(context))
+{
}
-bool CryptoResultImpl::canCompletePromise() const
+ScriptPromise CryptoResultImpl::promise()
{
- CheckValidThread();
- ExecutionContext* context = executionContext();
- return context && !context->activeDOMObjectsAreSuspended() && !context->activeDOMObjectsAreStopped();
+ return m_promiseState->promise();
}
} // namespace WebCore
« no previous file with comments | « Source/modules/crypto/CryptoResultImpl.h ('k') | Source/platform/CryptoResult.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698