Make CryptoResultImpl not to use WeakPtr.

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 18f45f92f91164f107de07d2b54f144feaee3bcf..0faa7082cf4818541fcaef2724c398ff3f12a1b0 100644
--- a/Source/modules/crypto/CryptoResultImpl.cpp
+++ b/Source/modules/crypto/CryptoResultImpl.cpp
@@ -50,26 +50,26 @@ namespace blink {
 
 class CryptoResultImpl::WeakResolver : public ScriptPromiseResolver {
 public:
-    static WeakPtr<ScriptPromiseResolver> create(ScriptState* scriptState, CryptoResultImpl* result)
+    static PassRefPtr<ScriptPromiseResolver> create(ScriptState* scriptState, CryptoResultImpl* result)
     {
         RefPtr<WeakResolver> p = adoptRef(new WeakResolver(scriptState, result));
         p->suspendIfNeeded();
         p->keepAliveWhilePending();
-        return p->m_weakPtrFactory.createWeakPtr();
+        return p.release();
     }
 
-    virtual ~WeakResolver()
+    virtual void stop() override
     {
         m_result->cancel();
+        m_result->clearResolver();
+        ScriptPromiseResolver::stop();
     }
 
 private:
     WeakResolver(ScriptState* scriptState, CryptoResultImpl* result)
         : ScriptPromiseResolver(scriptState)
-        , m_weakPtrFactory(this)
         , m_result(result) { }
-    WeakPtrFactory<ScriptPromiseResolver> m_weakPtrFactory;
-    RefPtr<CryptoResultImpl> m_result;
+    CryptoResultImpl* m_result;

[yhirano, 2014/12/11 02:27:59]

I'm not sure this change is correct, because I think |m_result| can be
destructed before this object is destroyed without RefPtr.
Perhaps we should clear m_result explicitly when calling resolve / reject,
right?

[tasak, 2014/12/11 09:44:39]

I see.
Now CryptoResultImpl::clearResolver() invokes clearCryptoResult() to clear
m_result.

 };
 
 ExceptionCode webCryptoErrorToExceptionCode(WebCryptoErrorType errorType)
@@ -104,6 +104,11 @@ CryptoResultImpl::~CryptoResultImpl()
 {
 }
 
+void CryptoResultImpl::clearResolver()
+{
+    m_resolver = nullptr;
+}
+
 PassRefPtr<CryptoResultImpl> CryptoResultImpl::create(ScriptState* scriptState)
 {
     return adoptRef(new CryptoResultImpl(scriptState));
@@ -113,12 +118,14 @@ void CryptoResultImpl::completeWithError(WebCryptoErrorType errorType, const Web
 {
     if (m_resolver)
         m_resolver->reject(DOMException::create(webCryptoErrorToExceptionCode(errorType), errorDetails));
+    m_resolver = nullptr;
 }
 
 void CryptoResultImpl::completeWithBuffer(const void* bytes, unsigned bytesSize)
 {
     if (m_resolver)
         m_resolver->resolve(DOMArrayBuffer::create(bytes, bytesSize));
+    m_resolver = nullptr;
 }
 
 void CryptoResultImpl::completeWithJson(const char* utf8Data, unsigned length)
@@ -139,18 +146,21 @@ void CryptoResultImpl::completeWithJson(const char* utf8Data, unsigned length)
             resolver->resolve(jsonDictionary);
         }
     }
+    m_resolver = nullptr;
 }
 
 void CryptoResultImpl::completeWithBoolean(bool b)
 {
     if (m_resolver)
         m_resolver->resolve(b);
+    m_resolver = nullptr;
 }
 
 void CryptoResultImpl::completeWithKey(const WebCryptoKey& key)
 {
     if (m_resolver)
         m_resolver->resolve(CryptoKey::create(key));
+    m_resolver = nullptr;
 }
 
 void CryptoResultImpl::completeWithKeyPair(const WebCryptoKey& publicKey, const WebCryptoKey& privateKey)
@@ -169,6 +179,7 @@ void CryptoResultImpl::completeWithKeyPair(const WebCryptoKey& publicKey, const
 
         m_resolver->resolve(keyPair.v8Value());
     }
+    m_resolver = nullptr;
 }
 
 bool CryptoResultImpl::cancelled() const
@@ -186,7 +197,7 @@ CryptoResultImpl::CryptoResultImpl(ScriptState* scriptState)
 {
     // Creating the WeakResolver may return nullptr if active dom objects have

[eroman, 2014/12/09 19:15:29]

This comment is no longer correct, and I believe the line below can now result
in a use-after-free.

The constructor of ScriptPromiseResolver may set
  m_state = ResolvedOrRejected
When active DOM obects are already stopped.

If that happens, then the call to keepAliveWhilePending() will not add a
reference. So m_resolver will be set to the value of the WeakResolver, however
as soon as the temporary PassRefPtr is destructed, the underlying object's
refcount will reach zero and be freed.

[tasak, 2014/12/11 09:44:39]

I see.
I checked whether ActiveDOMObjectStopped here. If stopped, m_result = nullptr.

     // been stopped. And in the process set m_cancelled to 1.
-    m_resolver = WeakResolver::create(scriptState, this);
+    m_resolver = WeakResolver::create(scriptState, this).get();
 }
 
 ScriptPromise CryptoResultImpl::promise()