ScriptPromise should understand the ScriptState from which the ScriptPromise is generated

Source/bindings/v8/ScriptPromiseResolver.h

 /*
  * 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 50 matching lines @@
 // To prevent memory leaks, you should release the reference manually
 // by calling resolve or reject.
 // Destroying the object will also release the reference.
 // Note that ScriptPromiseResolver::promise returns an empty value when the
 // resolver is already resolved or rejected. If you want to resolve a resolver
 // immediately and return the associated promise, you should get the promise
 // before resolving.
 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.
     // A ScriptPromiseResolver can be destructed safely without
     // entering a v8 context.
     ~ScriptPromiseResolver();
 
     // Returns the underlying Promise.
     // Note that the underlying Promise is cleared when |resolve| or |reject|
     // is called.
     ScriptPromise promise();
 
     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);
     }
     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);
     }
     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>);
 
     // Used by ToV8Value<ScriptPromiseResolver, v8::Handle<v8::Object> >.
     static v8::Handle<v8::Object> getCreationContext(v8::Handle<v8::Object> creationContext)
     {
         return creationContext;
     }
     // Used by ToV8Value<ScriptPromiseResolver, v8::Isolate*>.
     static v8::Handle<v8::Object> getCreationContext(v8::Isolate* isolate)
     {
         return ScriptState::current(isolate)->context()->Global();
     }
 
 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.
     ScriptValue m_resolver;
 };
 
 } // namespace WebCore
 
 
 #endif // ScriptPromiseResolver_h