Add a helper for implementing Promise-valued properties.

Source/bindings/v8/ScriptPromisePropertyBase.cpp

Index: Source/bindings/v8/ScriptPromisePropertyBase.cpp
diff --git a/Source/bindings/v8/ScriptPromisePropertyBase.cpp b/Source/bindings/v8/ScriptPromisePropertyBase.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..c2c5d41cbacbe00297484d011b8128255d6696db
--- /dev/null
+++ b/Source/bindings/v8/ScriptPromisePropertyBase.cpp
@@ -0,0 +1,233 @@
+// 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/ScriptPromisePropertyBase.h"
+
+#include "bindings/v8/V8Binding.h"
+#include "bindings/v8/V8HiddenValue.h"
+#include "core/dom/ExecutionContext.h"
+#include "wtf/text/StringBuilder.h"
+
+namespace WebCore {
+
+class ScriptPromisePropertyBase::PendingAction {
+public:
+    PendingAction(v8::Isolate* isolate, v8::Handle<v8::Promise::Resolver> resolver, v8::Handle<v8::Value> value, State state)
+        : m_resolver(isolate, resolver)
+        , m_value(isolate, value)
+        , m_state(state) { }
+    ~PendingAction() { }
+
+    void execute(v8::Isolate*);
+
+protected:
+    ScopedPersistent<v8::Promise::Resolver> m_resolver;
+    ScopedPersistent<v8::Value> m_value;
+    State m_state;
+};
+
+void ScriptPromisePropertyBase::PendingAction::execute(v8::Isolate* isolate)
+{
+    v8::HandleScope handleScope(isolate);
+    ScriptState::Scope scope(ScriptState::from(m_resolver.newLocal(isolate)->CreationContext()));

[haraken, 2014/07/01 06:29:13]

It would be nicer to explicitly pass in the ScriptState that created the
PendingAction, cache the ScriptState on PendingAction, and use the ScriptState
here.

Then you won't need the HandleScope above.

+    v8::Handle<v8::Promise::Resolver> resolver = m_resolver.newLocal(isolate);
+    m_resolver.clear();
+    v8::Handle<v8::Value> value = m_value.newLocal(isolate);
+    m_value.clear();

[haraken, 2014/07/01 06:29:13]

Just to confirm: Even if we don't clear m_value and m_resolver, it won't cause
leaks, right?

[dominicc (has gone to gerrit), 2014/07/02 04:02:16]

Those are strong, so while the ScriptPromiseProperty has pending actions it will
leak if the ScriptPromiseProperty is leaked.

Clearing them here doesn't make much sense because the whole PendingAction
should be deleted soon anyway...

+
+    switch (m_state) {
+    case Pending:
+        ASSERT_NOT_REACHED();
+        break;
+    case Resolved:
+        resolver->Resolve(value);
+        break;
+    case Rejected:
+        resolver->Reject(value);
+        break;
+    }
+}
+
+static v8::Handle<v8::String> makeName(v8::Isolate* isolate, const char* name, const char* suffix)
+{
+    StringBuilder builder;
+    builder.append(name);
+    builder.append(suffix);
+    return v8String(isolate, builder.toString());
+}
+
+ScriptPromisePropertyBase::ScriptPromisePropertyBase(ExecutionContext* executionContext, const char* name)
+    : ActiveDOMObject(executionContext)
+    , m_isolate(toIsolate(executionContext))
+    , m_state(Pending)
+    , m_inResume(false)
+{
+    v8::HandleScope handleScope(m_isolate);
+    m_resolverName.set(m_isolate, makeName(m_isolate, name, "/Resolver"));
+    m_promiseName.set(m_isolate, makeName(m_isolate, name, "/Promise"));
+}
+
+ScriptPromisePropertyBase::~ScriptPromisePropertyBase()
+{
+    v8::HandleScope handleScope(m_isolate);
+    v8::Handle<v8::Object> wrapper = m_mainWorldWrapper.newLocal(m_isolate);
+    m_mainWorldWrapper.clear();

[haraken, 2014/07/01 06:29:13]

You don't need to call this, since it's destructed anyway.

+    if (!wrapper.IsEmpty()) {
+        v8::Handle<v8::String> resolverName = m_resolverName.newLocal(m_isolate);
+        wrapper->DeleteHiddenValue(resolverName);
+        v8::Handle<v8::String> promiseName = m_promiseName.newLocal(m_isolate);
+        wrapper->DeleteHiddenValue(promiseName);
+    }
+}
+
+static void clearHandle(const v8::WeakCallbackData<v8::Object, ScopedPersistent<v8::Object> >& data)
+{
+    data.GetParameter()->clear();
+}
+
+ScriptPromise ScriptPromisePropertyBase::promise(DOMWrapperWorld& world)
+{
+    ASSERT(executionContext() && !executionContext()->activeDOMObjectsAreStopped());

[haraken, 2014/07/01 06:29:12]

How is this guaranteed?

[dominicc (has gone to gerrit), 2014/07/02 04:02:16]

It should be guaranteed by the caller.

Looking at likely users--FontFaceSet, ServiceWorkerContainer--they are already
lifetime-observing (ActiveDOMObject or ContextLifecycleObserver, could become
ActiveDOMObject).

Maybe it is better if I take the ActiveDOMObject and pending stuff out of this
class? Probably the user has to do it anyway. It would make
ScriptPromiseProperty much simpler. WDYT?

+
+    if (!world.isMainWorld()) {
+        // FIXME: Support isolated worlds.
+        return ScriptPromise();
+    }
+
+    v8::HandleScope handleScope(m_isolate);
+    ASSERT(m_isolate == toIsolate(executionContext()));

[haraken, 2014/07/01 06:29:13]

You can remove this, since it must hold :)

+    v8::Handle<v8::Context> context = toV8Context(executionContext(), world);
+    if (context.IsEmpty())
+        return ScriptPromise();
+    ScriptState* scriptState = ScriptState::from(context);
+    ScriptState::Scope scope(scriptState);
+
+    v8::Handle<v8::String> resolverName = m_resolverName.newLocal(m_isolate);
+    v8::Handle<v8::String> promiseName = m_promiseName.newLocal(m_isolate);
+
+    v8::Handle<v8::Object> wrapper = m_mainWorldWrapper.newLocal(m_isolate);
+    if (wrapper.IsEmpty()) {
+        wrapper = holder(context->Global(), m_isolate);
+        ASSERT(!wrapper.IsEmpty());
+        ASSERT(V8HiddenValue::getHiddenValue(m_isolate, wrapper, resolverName).IsEmpty());
+        ASSERT(V8HiddenValue::getHiddenValue(m_isolate, wrapper, promiseName).IsEmpty());
+        m_mainWorldWrapper.set(m_isolate, wrapper);
+        m_mainWorldWrapper.setWeak(&m_mainWorldWrapper, &clearHandle);
+    }
+    ASSERT(wrapper->CreationContext() == context);
+
+    v8::Handle<v8::Promise> promise = V8HiddenValue::getHiddenValue(m_isolate, wrapper, promiseName).As<v8::Promise>();
+    if (!promise.IsEmpty()) {
+        // Return cached Promise
+        return ScriptPromise(scriptState, promise);
+    }
+
+    // Create and cache the Promise
+    v8::Handle<v8::Promise::Resolver> resolver = v8::Promise::Resolver::New(m_isolate);
+    promise = resolver->GetPromise();
+    V8HiddenValue::setHiddenValue(m_isolate, wrapper, promiseName, promise);
+    V8HiddenValue::setHiddenValue(m_isolate, promise, promiseName, wrapper);

[haraken, 2014/07/01 06:29:12]

Add comments (probably in a header file) about how hidden values are organized.

+
+    switch (m_state) {
+    case Pending:
+        // Cache the resolver too
+        V8HiddenValue::setHiddenValue(m_isolate, wrapper, resolverName, resolver);
+        break;
+    case Resolved:
+    case Rejected:
+        resolveOrReject(resolver);
+        break;
+    }
+
+    return ScriptPromise(scriptState, promise);
+}
+
+void ScriptPromisePropertyBase::settle(State targetState)
+{
+    ASSERT(executionContext() && !executionContext()->activeDOMObjectsAreStopped());
+    ASSERT(m_state == Pending);
+    ASSERT(targetState == Resolved || targetState == Rejected);
+
+    m_state = targetState;
+
+    v8::HandleScope handleScope(m_isolate);
+    v8::Handle<v8::Object> wrapper = m_mainWorldWrapper.newLocal(m_isolate);
+    if (wrapper.IsEmpty())
+        return; // wrapper has died or was never populated
+    ScriptState::Scope scope(ScriptState::from(wrapper->CreationContext()));
+
+    v8::Handle<v8::String> resolverName = m_resolverName.newLocal(m_isolate);
+    v8::Handle<v8::Promise::Resolver> resolver = V8HiddenValue::getHiddenValue(m_isolate, wrapper, resolverName).As<v8::Promise::Resolver>();
+
+    V8HiddenValue::deleteHiddenValue(m_isolate, wrapper, resolverName);
+    resolveOrReject(resolver);
+}
+
+void ScriptPromisePropertyBase::resolveOrReject(v8::Handle<v8::Promise::Resolver> resolver)
+{
+    ASSERT(m_state == Resolved || m_state == Rejected);
+
+    v8::Handle<v8::Value> value;
+    switch (m_state) {
+    case Pending:
+        ASSERT_NOT_REACHED();
+        break;
+    case Resolved:
+        value = resolvedValue(resolver->CreationContext()->Global(), m_isolate);
+        break;
+    case Rejected:
+        value = rejectedValue(resolver->CreationContext()->Global(), m_isolate);
+        break;
+    }
+
+    if (executionContext()->activeDOMObjectsAreSuspended()) {
+        enqueuePendingAction(resolver, value, m_state);
+        return;
+    }
+
+    switch (m_state) {
+    case Pending:
+        ASSERT_NOT_REACHED();
+        break;
+    case Resolved:
+        resolver->Resolve(value);
+        break;
+    case Rejected:
+        resolver->Reject(value);
+        break;
+    }
+}
+
+void ScriptPromisePropertyBase::enqueuePendingAction(v8::Handle<v8::Promise::Resolver> resolver, v8::Handle<v8::Value> value, State state)
+{
+    if (m_pending.isEmpty())
+        setPendingActivity(this);

[haraken, 2014/07/01 06:29:13]

setPendingActivity() is deprecated, and we don't want to increase # of call
sites.

Can we make ScriptPromisePropertyBase override
ActiveDOMObject::hasPendingActivity? Probably the hasPendingActivity can return
true if m_pending.size() > 0.

+    m_pending.append(adoptPtr(new PendingAction(m_isolate, resolver, value, state)));
+}
+
+void ScriptPromisePropertyBase::resume()
+{
+    ASSERT_WITH_SECURITY_IMPLICATION(!m_inResume);

[haraken, 2014/07/01 06:29:13]

Is there any reason you want to use _WITH_SECURITY_IMPLICATION? Otherwise, let's
just use ASSERT or RELEASE_ASSERT.

+    if (!hasPendingActivity())
+        return;
+    ASSERT(m_pending.size());
+    m_inResume = true;
+    size_t i = 0;
+    while (i < m_pending.size() && executionContext() && !executionContext()->activeDOMObjectsAreSuspended() && !executionContext()->activeDOMObjectsAreStopped())

[haraken, 2014/07/01 06:29:13]

Add a comment and mention that the following execute() can stop/suspend the
execution context.

BTW, how can we guarantee that resume() is not reentered? In other words, I
wonder how it's guaranteed that ASSERT(!m_inResume) holds.

+        m_pending[i++]->execute(m_isolate);
+    if (m_pending.size())
+        m_pending.remove(0, i);

[haraken, 2014/07/01 06:29:13]

Probably you can use a linked list for m_pending and use takeFirst().

+    if (m_pending.isEmpty())
+        unsetPendingActivity(this);
+}
+
+void ScriptPromisePropertyBase::stop()
+{
+    if (m_pending.size())
+        unsetPendingActivity(this);
+    m_pending.clear();
+}
+
+} // namespace WebCore