Introduce KeepAliveWhilePending to ScriptPromiseResolverWithContext.

Source/bindings/v8/ScriptPromiseResolverWithContext.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "bindings/v8/ScriptPromiseResolverWithContext.h"
 
 #include "bindings/v8/V8RecursionScope.h"
 
 namespace WebCore {
 
+namespace {
+const char hiddenPropertyName[] = "blink::keepObjectWhilePending";
+} // namespace
+
 ScriptPromiseResolverWithContext::ScriptPromiseResolverWithContext(ScriptState* scriptState)
     : ActiveDOMObject(scriptState->executionContext())
     , m_state(Pending)
     , m_scriptState(scriptState)
     , m_timer(this, &ScriptPromiseResolverWithContext::onTimerFired)
     , m_resolver(ScriptPromiseResolver::create(m_scriptState.get()))
 {
 }
 
+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();
 }
 
 void ScriptPromiseResolverWithContext::resume()
 {
     if (m_state == Resolving || m_state == Rejecting)
         m_timer.startOneShot(0, FROM_HERE);
 }
@@ skipping 25 matching lines @@
         } else {
             ASSERT(m_state == Rejecting);
             m_resolver->reject(m_value.newLocal(m_scriptState->isolate()));
         }
     }
     clear();
 }
 
 void ScriptPromiseResolverWithContext::clear()
 {
+    unregisterKeptObject();
     ResolutionState state = m_state;
     m_state = ResolvedOrRejected;
     m_resolver.clear();
     m_value.clear();
     if (state == Resolving || state == Rejecting) {
         // |ref| was called in |resolveOrReject|.
         deref();
     }
     // |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