Fix crash when ExecutionContext is torn down before a crypto operation has completed.

Source/modules/crypto/CryptoResultImpl.cpp

 /*
  * Copyright (C) 2013 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 70 matching lines @@
 
 } // namespace
 
 // 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
+// Additionally the PromiseState is deleted 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 {
+class CryptoResultImpl::PromiseState FINAL {
 public:
     static WeakPtr<PromiseState> create(ExecutionContext* context)
     {
         PromiseState* promiseState = new PromiseState(context);
         return promiseState->m_weakFactory.createWeakPtr();
     }
 
-    // Override from ContextLifecycleObserver
-    virtual void contextDestroyed() OVERRIDE
+    void contextDestroyed()
     {
-        ContextLifecycleObserver::contextDestroyed();
         delete this;
     }
 
     ScriptPromise promise()
     {
         return m_promiseResolver->promise();
     }
 
     void completeWithError(blink::WebCryptoErrorType errorType, const blink::WebString& errorDetails)
     {
@@ skipping 19 matching lines @@
         delete this;
     }
 
     void completeWithKeyPair(const blink::WebCryptoKey& publicKey, const blink::WebCryptoKey& privateKey)
     {
         m_promiseResolver->resolve(KeyPair::create(publicKey, privateKey));
         delete this;
     }
 
 private:
+    // This subclass of ScriptPromiseResolverWithContext is to be notified
+    // when the context was destroyed.
+    class PromiseResolver FINAL : public ScriptPromiseResolverWithContext {
+    public:
+        static PassRefPtr<PromiseResolver> create(ScriptState* scriptState, PromiseState* promiseState)
+        {
+            RefPtr<PromiseResolver> resolver = adoptRef(new PromiseResolver(scriptState, promiseState));
+            resolver->suspendIfNeeded();
+            return resolver.release();
+        }
+
+        virtual void contextDestroyed() OVERRIDE

[haraken, 2014/05/06 01:44:02]

How about adding the functionality to ScriptPromiseResolverWithContext, instead
of adding a new subclass?

I think the goal of ScriptPromiseResolverWithContext is to take care of this
kind of complexity. yhirano@ would know better.

[eroman, 2014/05/06 22:59:22]

Sure what do you have in mind? Perhaps pass in a delegate pointer that
ScriptPromiseResolverWithContext calls on context destruction?

+        {
+            ScriptPromiseResolverWithContext::contextDestroyed();
+            m_promiseState->contextDestroyed();
+        }
+
+    private:
+        explicit PromiseResolver(ScriptState* scriptState, PromiseState* promiseState)
+            : ScriptPromiseResolverWithContext(scriptState)
+            , m_promiseState(promiseState)
+        {
+        }
+
+        PromiseState* m_promiseState;
+    };
+
     explicit PromiseState(ExecutionContext* context)
-        : ContextLifecycleObserver(context)
-        , m_weakFactory(this)
-        , m_promiseResolver(ScriptPromiseResolverWithContext::create(ScriptState::current(toIsolate(context))))
+        : m_weakFactory(this)
+        , m_promiseResolver(PromiseResolver::create(ScriptState::current(toIsolate(context)), this))
     {
     }
 
     WeakPtrFactory<PromiseState> m_weakFactory;
-    RefPtr<ScriptPromiseResolverWithContext> m_promiseResolver;
+    RefPtr<PromiseResolver> m_promiseResolver;
 };
 
 CryptoResultImpl::~CryptoResultImpl()
 {
 }
 
 PassRefPtr<CryptoResultImpl> CryptoResultImpl::create()
 {
     return adoptRef(new CryptoResultImpl(callingExecutionContext(v8::Isolate::GetCurrent())));
 }
@@ skipping 32 matching lines @@
     : m_promiseState(PromiseState::create(context))
 {
 }
 
 ScriptPromise CryptoResultImpl::promise()
 {
     return m_promiseState->promise();
 }
 
 } // namespace WebCore