Make CryptoResultImpl not to use WeakPtr.

Source/modules/crypto/CryptoResultImpl.h

Index: Source/modules/crypto/CryptoResultImpl.h
diff --git a/Source/modules/crypto/CryptoResultImpl.h b/Source/modules/crypto/CryptoResultImpl.h
index 3ea352cb662d7b5bb8d5a35704e30b9bfeaeb49d..af105e3fe071ccda7419a21967890108730f6c44 100644
--- a/Source/modules/crypto/CryptoResultImpl.h
+++ b/Source/modules/crypto/CryptoResultImpl.h
@@ -72,12 +72,18 @@ public:
     ScriptPromise promise();
 
 private:
-    class WeakResolver;
+    class Resolver;
     explicit CryptoResultImpl(ScriptState*);
 
+    void clearResolver();
     void cancel();
 
-    WeakPtr<ScriptPromiseResolver> m_resolver;
+    // Since ScriptPromiseResolver destroys itself, we should not use RefPtr

[haraken, 2014/12/12 08:49:31]

I still don't fully understand why we cannot make it a
RefPtr<ScriptPromiseResolver>. As far as I understand, ScriptPromiseResolver
doesn't destroy itself but just deref()'s itself when the ScriptPromiseResolver
is cleared. ScriptPromiseResolver is destroyed when the last reference is gone.

So I think we can just make this a RefPtr<ScriptPromiseResolver> and clear the
RefPtr in clearResolver(). It seems safer than having a raw pointer to the
ScriptPromiseResolver.

[eroman, 2014/12/12 23:32:54]

I had some concerns on this explained in earlier comments:

CryptoResultImpl may be deleted from a different thread. ScriptPromiseResolver
is not RefCountedThreadSafe. Therefore it is misleading to have a RefPtr<> since
it should not be releasing a reference during the destructor

[haraken, 2014/12/15 01:34:59]

Thanks, makes sense.

tasak@: Let's update a comment accordingly.

BTW, it still looks strange that CryptoResultImpl and CryptoResultImpl::Resolver
are referencing to each other using a raw pointer. Should we make
CryptoResultImpl::Resolver::m_result a RefPtr<CryptoResultImpl>? (In the
previous code, CryptoResultImpl::Resolver::m_result was a
RefPtr<CryptoResultImpl>. We can just keep it.)

+    // here. So to avoid using stale pointer, m_resolver should be cleared

[haraken, 2014/12/12 08:49:31]

a stale pointer

+    // after resolving.

[haraken, 2014/12/12 08:49:31]

after resolving => when getting resolved

[tasak, 2014/12/15 05:26:46]

Updated the comment.

+    // FIXME: ScriptPromiseResolver should not be exported.
+    // Instead, use ScriptPromise.
+    ScriptPromiseResolver* m_resolver;

[haraken, 2014/12/12 08:49:31]

Shall we change ScriptPromiseResolver* to CryptoResultImpl::Resolver* ?

(But I agree that a right way to fix this problem is to change the
ScriptPromiseResolver* to a ScriptPromise. We should try this in a follow-up
CL.)

     volatile int m_cancelled;
 };