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..bb42432688d9137cb8bb93121f84591f9fd68f48 100644
--- a/Source/bindings/v8/ScriptPromiseResolverWithContext.cpp
+++ b/Source/bindings/v8/ScriptPromiseResolverWithContext.cpp
@@ -9,13 +9,28 @@
 
 namespace WebCore {
 
-ScriptPromiseResolverWithContext::ScriptPromiseResolverWithContext(ScriptState* scriptState)
+ScriptPromiseResolverWithContext::ScriptPromiseResolverWithContext(ScriptState* scriptState, Mode mode)
     : ActiveDOMObject(scriptState->executionContext())
     , m_state(Pending)
     , m_scriptState(scriptState)
+    , m_mode(mode)
     , m_timer(this, &ScriptPromiseResolverWithContext::onTimerFired)
     , m_resolver(ScriptPromiseResolver::create(m_scriptState.get()))
 {
+    if (mode == KeepAliveWhilePending) {
+        // Keep |this| while the promise is Pending.
+        // deref() will be called in clear().
+        ref();

[eroman, 2014/06/16 20:05:50]

When I run this patch in debug mode I am hitting this assertion:

../../third_party/WebKit/Source/wtf/RefCounted.h:59
59              ASSERT(!m_adoptionIsRequired);

I think either the ref() needs to be moved out of the constructor and into the
create() method, or maybe you can call relaxAdoptionRequirement() to turn off
that assertion.

[yhirano, 2014/06/17 01:45:44]

Thanks for pointing it out.
I would like to call relaxAdoptionRequirement here, because this class can be
subclassed.

+    }
+}
+
+ScriptPromiseResolverWithContext::~ScriptPromiseResolverWithContext()
+{
+    if (m_state != ResolvedOrRejected) {
+        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()
@@ -31,13 +46,13 @@ void ScriptPromiseResolverWithContext::resume()
 
 void ScriptPromiseResolverWithContext::stop()
 {
+    ScriptState::Scope scope(m_scriptState.get());
     m_timer.stop();
     clear();
 }
 
 void ScriptPromiseResolverWithContext::onTimerFired(Timer<ScriptPromiseResolverWithContext>*)
 {
-    RefPtr<ScriptPromiseResolverWithContext> protect(this);
     ScriptState::Scope scope(m_scriptState.get());
     resolveOrRejectImmediately();
 }
@@ -63,15 +78,20 @@ void ScriptPromiseResolverWithContext::resolveOrRejectImmediately()
 
 void ScriptPromiseResolverWithContext::clear()
 {
+    if (m_state == ResolvedOrRejected)
+        return;
     ResolutionState state = m_state;
     m_state = ResolvedOrRejected;
     m_resolver.clear();
     m_value.clear();
+    if (m_mode == KeepAliveWhilePending) {
+        // |ref| was called in the constructor.
+        deref();

[eroman, 2014/06/16 19:36:35]

Is it possible for the state to be Pending here?

If so, then this line may "delete this" and it would be strange to execute the
following line.

[yhirano, 2014/06/17 01:45:44]

I added a comment. Is it clear now?

+    }
     if (state == Resolving || state == Rejecting) {
         // |ref| was called in |resolveOrReject|.
         deref();
     }
-    // |this| may be deleted here.
 }
 
 } // namespace WebCore