ScriptPromise should understand the ScriptState from which the ScriptPromise is generated

Source/bindings/v8/ScriptPromiseResolver.h

Index: Source/bindings/v8/ScriptPromiseResolver.h
diff --git a/Source/bindings/v8/ScriptPromiseResolver.h b/Source/bindings/v8/ScriptPromiseResolver.h
index a54ace56ec096e10c9391b1101112c4af05f7bcc..e101d61167d08f2fbef4021fd57ef90a6adac657 100644
--- a/Source/bindings/v8/ScriptPromiseResolver.h
+++ b/Source/bindings/v8/ScriptPromiseResolver.h
@@ -68,8 +68,7 @@ class ExecutionContext;
 class ScriptPromiseResolver : public RefCounted<ScriptPromiseResolver> {
     WTF_MAKE_NONCOPYABLE(ScriptPromiseResolver);
 public:
-    static PassRefPtr<ScriptPromiseResolver> create(ExecutionContext*);
-    static PassRefPtr<ScriptPromiseResolver> create(v8::Isolate*);
+    static PassRefPtr<ScriptPromiseResolver> create(ScriptState*);
 
     // A ScriptPromiseResolver should be resolved / rejected before
     // its destruction.
@@ -85,7 +84,7 @@ public:
     template<typename T>
     void resolve(const T& value, ExecutionContext* executionContext)
     {
-        ASSERT(m_isolate->InContext());
+        ASSERT(m_scriptState->isolate()->InContext());
         // You should use ScriptPromiseResolverWithContext when you want
         // to resolve a Promise in a non-original V8 context.
         return resolve(value);
@@ -93,7 +92,7 @@ public:
     template<typename T>
     void reject(const T& value, ExecutionContext* executionContext)
     {
-        ASSERT(m_isolate->InContext());
+        ASSERT(m_scriptState->isolate()->InContext());
         // You should use ScriptPromiseResolverWithContext when you want
         // to reject a Promise in a non-original V8 context.
         return reject(value);
@@ -101,27 +100,27 @@ public:
     template<typename T>
     void resolve(const T& value)
     {
-        ASSERT(m_isolate->InContext());
+        ASSERT(m_scriptState->isolate()->InContext());
         resolve(toV8Value(value));
     }
     template<typename T>
     void reject(const T& value)
     {
-        ASSERT(m_isolate->InContext());
+        ASSERT(m_scriptState->isolate()->InContext());
         reject(toV8Value(value));
     }
     template<typename T> void resolve(const T& value, v8::Handle<v8::Object> creationContext)
     {
-        ASSERT(m_isolate->InContext());
+        ASSERT(m_scriptState->isolate()->InContext());
         resolve(toV8Value(value, creationContext));
     }
     template<typename T> void reject(const T& value, v8::Handle<v8::Object> creationContext)
     {
-        ASSERT(m_isolate->InContext());
+        ASSERT(m_scriptState->isolate()->InContext());
         reject(toV8Value(value, creationContext));
     }
 
-    v8::Isolate* isolate() const { return m_isolate; }
+    v8::Isolate* isolate() const { return m_scriptState->isolate(); }
 
     void resolve(v8::Handle<v8::Value>);
     void reject(v8::Handle<v8::Value>);
@@ -141,19 +140,18 @@ private:
     template<typename T>
     v8::Handle<v8::Value> toV8Value(const T& value, v8::Handle<v8::Object> creationContext)
     {
-        return ToV8Value<ScriptPromiseResolver, v8::Handle<v8::Object> >::toV8Value(value, creationContext, m_isolate);
+        return ToV8Value<ScriptPromiseResolver, v8::Handle<v8::Object> >::toV8Value(value, creationContext, m_scriptState->isolate());
     }
 
     template<typename T>
     v8::Handle<v8::Value> toV8Value(const T& value)
     {
-        return ToV8Value<ScriptPromiseResolver, v8::Isolate*>::toV8Value(value, m_isolate, m_isolate);
+        return ToV8Value<ScriptPromiseResolver, v8::Isolate*>::toV8Value(value, m_scriptState->isolate(), m_scriptState->isolate());

[yhirano, 2014/05/09 06:24:57]

This can be ToV8Value<ScriptPromiseResolver, ScriptState*> .

[haraken, 2014/05/09 07:37:25]

Actually I don't fully understand this template. If I change it to
ToV8Value<ScriptPromiseResolver, ScriptState*>, I hit the compile error.

In the first place, why are we passing m_isolate to both second and the third
parameter?

[yhirano, 2014/05/09 07:58:37]

The second parameter is used for getting the creation context and its type is
the second template parameter of ToV8Value. The third parameter type is always
v8::Isolate*. Please compare with the above toV8Value.

The second parameter is given to T::getCreationContext where T is the first
template parameter of ToV8Value. The current code gets the isolate from the
scriptState in the second toV8Value, gets the scriptState from the given isolate
in the second getCreationContext and get the V8 context from it.
By changing both the second toV8Value and the second getCreationContext, you can
save unnecessary ScriptState -> isolate -> Scriptstate conversion.

[haraken, 2014/05/09 08:42:00]

Thanks for the details, done.

     }
 
-    ScriptPromiseResolver(ExecutionContext*);
-    ScriptPromiseResolver(v8::Isolate*);
+    explicit ScriptPromiseResolver(ScriptState*);
 
-    v8::Isolate* m_isolate;
+    RefPtr<ScriptState> m_scriptState;
     // Used when scriptPromiseOnV8Promise is disabled.
     ScriptPromise m_promise;
     // Used when scriptPromiseOnV8Promise is enabled.