Pass isolate to ScriptPromise and ScriptString constructors

Source/bindings/v8/ScriptValue.h

Index: Source/bindings/v8/ScriptValue.h
diff --git a/Source/bindings/v8/ScriptValue.h b/Source/bindings/v8/ScriptValue.h
index dc12b8b94cd865179b11fb149472084e9609f6ba..505fd8db7b803c54ea3c50eb6f1db001f8e6e208 100644
--- a/Source/bindings/v8/ScriptValue.h
+++ b/Source/bindings/v8/ScriptValue.h
@@ -57,19 +57,26 @@ typedef Vector<RefPtr<WTF::ArrayBuffer>, 1> ArrayBufferArray;
 
 class ScriptValue {
 public:
-    ScriptValue() { }
+    ScriptValue()
+        : m_isolate(0)

[haraken, 2013/09/18 00:25:12]

Is there any reason why we can't call Isolate::GetCurrent() here and ensure that
m_isolate is not 0 ?

[do-not-use, 2013/09/18 06:35:15]

My understanding was that we wanted to avoid calls to Isolate::GetCurrent() as
much as possible. I expect there are many cases where we construct an empty
StringValue and don't need an isolate at all.

[haraken, 2013/09/18 07:02:15]

Understood.

+    { }
+
     virtual ~ScriptValue();
 
     ScriptValue(v8::Handle<v8::Value> value, v8::Isolate* isolate)
-        : m_value(value.IsEmpty() ? 0 : SharedPersistent<v8::Value>::create(value, isolate))
+        : m_isolate(isolate)
+        , m_value(value.IsEmpty() ? 0 : SharedPersistent<v8::Value>::create(value, isolate))
     {
     }
 
     ScriptValue(const ScriptValue& value)
-        : m_value(value.m_value)
+        : m_isolate(value.m_isolate)
+        , m_value(value.m_value)
     {
     }
 
+    v8::Isolate* isolate() const { return m_isolate; }

[haraken, 2013/09/18 00:25:12]

Even if we cannot ensure that m_isolate is not 0 for some reason, we want to
ensure that isolate() doesn't return 0. Hence we want to write:

  v8::Isolate* isolate() const { if(!m_isolate) m_isolate =
Isolate::GetCurrent(); return m_isolate; }

[do-not-use, 2013/09/18 06:35:15]

Yes, this sounds like a good idea.

[do-not-use, 2013/09/18 07:18:46]

Done.

+
     static ScriptValue createNull()
     {
         v8::Isolate* isolate = v8::Isolate::GetCurrent();
@@ -83,8 +90,10 @@ public:
 
     ScriptValue& operator=(const ScriptValue& value)
     {
-        if (this != &value)
+        if (this != &value) {
             m_value = value.m_value;
+            m_isolate = value.m_isolate;
+        }
         return *this;
     }
 
@@ -155,21 +164,21 @@ public:
     void clear()
     {
         m_value = 0;
+        m_isolate = 0;

[haraken, 2013/09/18 00:25:12]

Do we need to clear an isolate? In other words, is it possible that we reuse the
same ScriptPromise in different isolates?

[do-not-use, 2013/09/18 06:35:15]

I honestly have no idea. I chose the "safe" approach and reset everything.

[haraken, 2013/09/18 07:02:15]

I believe you can remove m_isolate=0. V8 heaps are completely separated among
isolates, so it's not possible to share one ScriptValue among isolates.

[do-not-use, 2013/09/18 07:18:46]

Done.

     }
 
     v8::Handle<v8::Value> v8Value() const
     {
-        return m_value.get() ? m_value->newLocal(v8::Isolate::GetCurrent()) : v8::Handle<v8::Value>();
+        return m_value.get() ? m_value->newLocal(m_isolate) : v8::Handle<v8::Value>();
     }
 
-    bool getString(ScriptState* scriptState, String& result) const { return getString(result, scriptState->isolate()); }
-    bool getString(String& result) const { return getString(result, v8::Isolate::GetCurrent()); }
-    bool getString(String& result, v8::Isolate*) const;
-    String toString(ScriptState*) const;
+    bool getString(String& result) const;
+    String toString() const;
 
     PassRefPtr<JSONValue> toJSONValue(ScriptState*) const;
 
 private:
+    v8::Isolate* m_isolate;
     RefPtr<SharedPersistent<v8::Value> > m_value;
 };