Introduce KeepAliveWhilePending to ScriptPromiseResolverWithContext.

Source/bindings/v8/ScriptPromiseResolverWithContext.cpp

Index: Source/bindings/v8/ScriptPromiseResolverWithContext.cpp
diff --git a/Source/bindings/v8/ScriptPromiseResolverWithContext.cpp b/Source/bindings/v8/ScriptPromiseResolverWithContext.cpp
index 1a39cac07223f4c777f9d4df80e891041609d0fa..bc4ba6d7da416643bf6a74480b35d8dd22e56db2 100644
--- a/Source/bindings/v8/ScriptPromiseResolverWithContext.cpp
+++ b/Source/bindings/v8/ScriptPromiseResolverWithContext.cpp
@@ -9,6 +9,10 @@
 
 namespace WebCore {
 
+namespace {
+const char hiddenPropertyName[] = "blink::keepObjectWhilePending";
+} // namespace
+
 ScriptPromiseResolverWithContext::ScriptPromiseResolverWithContext(ScriptState* scriptState)
     : ActiveDOMObject(scriptState->executionContext())
     , m_state(Pending)
@@ -18,6 +22,16 @@ ScriptPromiseResolverWithContext::ScriptPromiseResolverWithContext(ScriptState*
 {
 }
 
+ScriptPromiseResolverWithContext::~ScriptPromiseResolverWithContext()
+{
+    if (m_state != ResolvedOrRejected) {
+        unregisterKeptObject();
+        ScriptState::Scope scope(m_scriptState.get());
+        reject(v8::Exception::Error(v8::String::NewFromUtf8(m_scriptState->isolate(),
+            "ScriptPromiseResolverWithContext is destructed without resolve / reject")));
+    }
+}
+
 void ScriptPromiseResolverWithContext::suspend()
 {
     m_timer.stop();
@@ -63,6 +77,7 @@ void ScriptPromiseResolverWithContext::resolveOrRejectImmediately()
 
 void ScriptPromiseResolverWithContext::clear()
 {
+    unregisterKeptObject();
     ResolutionState state = m_state;
     m_state = ResolvedOrRejected;
     m_resolver.clear();
@@ -74,4 +89,31 @@ void ScriptPromiseResolverWithContext::clear()
     // |this| may be deleted here.
 }
 
+void ScriptPromiseResolverWithContext::keepObjectWhilePendingInternal(v8::Handle<v8::Value> value)
+{
+    v8::HandleScope handleScope(m_scriptState->isolate());
+    if (!m_resolver)
+        return;
+    ScriptPromise promise = this->promise();
+    if (promise.isEmpty())
+        return;
+    v8::Local<v8::Promise> v8Promise = promise.v8Value().As<v8::Promise>();
+    v8::Local<v8::String> name = v8::String::NewFromUtf8(m_scriptState->isolate(), hiddenPropertyName);
+    ASSERT(v8Promise->GetHiddenValue(name).IsEmpty());
+    v8Promise->SetHiddenValue(name, value);

[haraken, 2014/06/13 05:03:51]

Consider using helper methods in V8HiddenValue.h instead of directly using
Get/Set/DeleteHiddenValue. (We're planning to replace the implementation of
V8HiddenValue so that it uses private names, which is faster than
Get/Set/DeleteHiddenValue.)

[yhirano, 2014/06/13 11:15:22]

Done.

+}
+
+void ScriptPromiseResolverWithContext::unregisterKeptObject()
+{
+    v8::HandleScope handleScope(m_scriptState->isolate());
+    if (!m_resolver)
+        return;
+    ScriptPromise promise = this->promise();
+    if (promise.isEmpty())
+        return;
+    v8::Local<v8::Promise> v8Promise = promise.v8Value().As<v8::Promise>();
+    v8::Local<v8::String> name = v8::String::NewFromUtf8(m_scriptState->isolate(), hiddenPropertyName);
+    v8Promise->DeleteHiddenValue(name);

[yhirano, 2014/06/12 11:02:51]

Rafael, I heard that deleting a property is generally bad for performance (for
example, your comments at [1]). Is the rule also applied to hidden properties?

1. https://codereview.chromium.org/250883002/#msg8

[rafaelw, 2014/06/13 02:36:17]

Haraken is probably more authoritative on this than I, but it looks to me like
hidden properties aren't stored in the object's map, but in a referenced hash
map
(https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/objects.cc&...).
I wouldn't expect deleting a hidden property to slow the object down


On 2014/06/12 11:02:51, yhirano wrote:

[haraken, 2014/06/13 03:36:53]

Agreed, I don't think deleting a hidden property affects performance.

However, an object that has hidden properties is heavier (in terms of memory)
than an object that doesn't have hidden properties. Also hidden property access
is much slower than normal property access.

I'm not sure if these performance & memory issues become a real problem here
though.

[yhirano, 2014/06/13 04:19:30]

Then is it better to have another boolean member variable showing whether
keepObjectWhilePending is called, for performance?

[yhirano, 2014/06/13 04:19:30]

Thanks!

[haraken, 2014/06/13 05:03:51]

Only if this code is really performance-sensitive (and I don't think so).

+}
+
 } // namespace WebCore