Make WebMIDI use blink Promise.

Source/modules/webmidi/MIDIAccess.cpp

Index: Source/modules/webmidi/MIDIAccess.cpp
diff --git a/Source/modules/webmidi/MIDIAccess.cpp b/Source/modules/webmidi/MIDIAccess.cpp
index 32d6a11870ff9383c0060f9d20132d6a3048f4f4..a15878825c44e01c5d7c99b0babf60a2ce50a7ae 100644
--- a/Source/modules/webmidi/MIDIAccess.cpp
+++ b/Source/modules/webmidi/MIDIAccess.cpp
@@ -31,49 +31,74 @@
 #include "config.h"
 #include "modules/webmidi/MIDIAccess.h"
 
+#include "bindings/v8/MIDIAccessResolver.h"
+#include "bindings/v8/ScriptPromise.h"
+#include "bindings/v8/ScriptPromiseResolver.h"
+#include "bindings/v8/V8Binding.h"
 #include "core/dom/DOMError.h"
 #include "core/dom/Document.h"
 #include "core/loader/DocumentLoadTiming.h"
 #include "core/loader/DocumentLoader.h"
-#include "modules/webmidi/MIDIAccessPromise.h"
 #include "modules/webmidi/MIDIConnectionEvent.h"
 #include "modules/webmidi/MIDIController.h"
+#include "modules/webmidi/MIDIOptions.h"
 #include "modules/webmidi/MIDIPort.h"
+#include "platform/AsyncMethodRunner.h"
 
 namespace WebCore {
 
-PassRefPtrWillBeRawPtr<MIDIAccess> MIDIAccess::create(ExecutionContext* context, MIDIAccessPromise* promise)
+class MIDIAccess::PostAction : public ScriptFunction {
+public:
+    static PassOwnPtr<MIDIAccess::PostAction> create(v8::Isolate* isolate, MIDIAccess* owner, State state) { return adoptPtr(new PostAction(isolate, owner, state)); }
+
+private:
+    PostAction(v8::Isolate* isolate, MIDIAccess* owner, State state): ScriptFunction(isolate), m_owner(owner), m_state(state) { }
+    virtual ScriptValue call(ScriptValue value) OVERRIDE
+    {
+        m_owner->doPostAction(m_state);
+        return value;
+    }
+
+    // MIDIAccess must be accessible when |call| function is called.
+    MIDIAccess* m_owner;

[abarth-chromium, 2014/03/20 00:43:46]

This raw pointer is scary...

[yhirano, 2014/03/20 02:40:51]

Done.

+    State m_state;
+};
+
+ScriptPromise MIDIAccess::request(const MIDIOptions& options, ExecutionContext* context)
 {
-    RefPtrWillBeRawPtr<MIDIAccess> midiAccess(adoptRefWillBeRefCountedGarbageCollected(new MIDIAccess(context, promise)));
+    RefPtrWillBeRawPtr<MIDIAccess> midiAccess(adoptRefWillBeRefCountedGarbageCollected(new MIDIAccess(options, context)));
     midiAccess->suspendIfNeeded();
-    midiAccess->startRequest();
-    return midiAccess.release();
+    return midiAccess->startRequest();
 }
 
 MIDIAccess::~MIDIAccess()
 {
-    stop();
 }
 
-MIDIAccess::MIDIAccess(ExecutionContext* context, MIDIAccessPromise* promise)
+MIDIAccess::MIDIAccess(const MIDIOptions& options, ExecutionContext* context)
     : ActiveDOMObject(context)
-    , m_promise(promise)
-    , m_hasAccess(false)
+    , m_state(Requesting)
+    , m_options(options)
     , m_sysExEnabled(false)
-    , m_requesting(false)
+    , m_asyncResolveRunner(this, &MIDIAccess::resolveNow)
+    , m_asyncRejectRunner(this, &MIDIAccess::rejectNow)
 {
     ScriptWrappable::init(this);
     m_accessor = MIDIAccessor::create(this);
+    // Create a wrapper to expose this object to the V8 GC so that hasPendingActivity takes effect.
+    toV8NoInline(this, executionContext());

[abarth-chromium, 2014/03/20 00:43:46]

This seems like a bit of a hack.  We don't use this pattern elsewhere...  How
does other code solve this problem?

[yhirano, 2014/03/20 02:40:51]

Another CL[1] being reviewed is using this pattern.
Note that I will generalize this "Async initialization of Promise resolution
target DOM objects" pattern and hide this hack in the common base class in the
followup CL.

Another solution is retaining objects in a global variable explicitly.
modules/quota does this. The QuotaDispatcher singleton object has pending
callbacks[2].

In the latter pattern, we have to pass the initialization context ownership
around pending operations if we have multiple initialization steps.

The former pattern is more natural to me, because the initializing context holds
all.

[1] https://codereview.chromium.org/104363002/
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/quot...

+    // Now this object is retained because m_state equals to Requesting.
+    // startRequest should be called at once.
 }
 
 void MIDIAccess::setSysExEnabled(bool enable)
 {
-    m_requesting = false;
     m_sysExEnabled = enable;
-    if (enable)
+    if (enable) {
         m_accessor->startSession();
-    else
-        permissionDenied();
+    } else {
+        reject(DOMError::create("SecurityError"));
+    }
 }
 
 void MIDIAccess::didAddInputPort(const String& id, const String& manufacturer, const String& name, const String& version)
@@ -94,19 +119,17 @@ void MIDIAccess::didAddOutputPort(const String& id, const String& manufacturer,
 void MIDIAccess::didStartSession(bool success)
 {
     ASSERT(isMainThread());
-
-    m_hasAccess = success;
     if (success)
-        m_promise->fulfill();
+        resolve();
     else
-        m_promise->reject(DOMError::create("InvalidStateError"));
+        reject(DOMError::create("InvalidStateError"));
 }
 
 void MIDIAccess::didReceiveMIDIData(unsigned portIndex, const unsigned char* data, size_t length, double timeStamp)
 {
     ASSERT(isMainThread());
 
-    if (m_hasAccess && portIndex < m_inputs.size()) {
+    if (m_state == Resolved && portIndex < m_inputs.size()) {
         // Convert from time in seconds which is based on the time coordinate system of monotonicallyIncreasingTime()
         // into time in milliseconds (a DOMHighResTimeStamp) according to the same time coordinate system as performance.now().
         // This is how timestamps are defined in the Web MIDI spec.
@@ -121,7 +144,7 @@ void MIDIAccess::didReceiveMIDIData(unsigned portIndex, const unsigned char* dat
 
 void MIDIAccess::sendMIDIData(unsigned portIndex, const unsigned char* data, size_t length, double timeStampInMilliseconds)
 {
-    if (m_hasAccess && portIndex < m_outputs.size() && data && length > 0) {
+    if (m_state == Resolved && portIndex < m_outputs.size() && data && length > 0) {
         // Convert from a time in milliseconds (a DOMHighResTimeStamp) according to the same time coordinate system as performance.now()
         // into a time in seconds which is based on the time coordinate system of monotonicallyIncreasingTime().
         double timeStamp;
@@ -141,49 +164,106 @@ void MIDIAccess::sendMIDIData(unsigned portIndex, const unsigned char* data, siz
     }
 }
 
+void MIDIAccess::suspend()
+{
+    m_asyncResolveRunner.suspend();
+    m_asyncRejectRunner.suspend();
+}
+
+void MIDIAccess::resume()
+{
+    m_asyncResolveRunner.resume();
+    m_asyncRejectRunner.resume();
+}
+
 void MIDIAccess::stop()
 {
-    m_hasAccess = false;
-    if (!m_requesting)
+    if (m_state == Stopped)
         return;
-    m_requesting = false;
-    Document* document = toDocument(executionContext());
-    ASSERT(document);
-    MIDIController* controller = MIDIController::from(document->page());
-    ASSERT(controller);
-    controller->cancelSysExPermissionRequest(this);
+    m_error.clear();
+    m_accessor.clear();
+    m_asyncResolveRunner.stop();
+    m_asyncRejectRunner.stop();
+    if (m_state == Requesting) {
+        Document* document = toDocument(executionContext());
+        ASSERT(document);
+        MIDIController* controller = MIDIController::from(document->page());
+        ASSERT(controller);
+        controller->cancelSysExPermissionRequest(this);
+    }
+    m_state = Stopped;

[abarth-chromium, 2014/03/20 00:43:46]

How does this clear out the m_owner pointer in PostAction?  At this point |this|
can be GCed...

[yhirano, 2014/03/20 02:40:51]

Done.

+}
+
+bool MIDIAccess::hasPendingActivity() const
+{
+    return m_state == Requesting;
+}
+
+void MIDIAccess::permissionDenied()
+{
+    ASSERT(isMainThread());
+    reject(DOMError::create("SecurityError"));
 }
 
-void MIDIAccess::startRequest()
+ScriptPromise MIDIAccess::startRequest()
 {
-    if (!m_promise->options()->sysex) {
+    ASSERT(frame());
+    ScriptPromise promise = ScriptPromise::createPending();
+    promise.then(PostAction::create(toIsolate(executionContext()), this, Resolved),
+        PostAction::create(toIsolate(executionContext()), this, Stopped));
+
+    m_resolver = MIDIAccessResolver::create(ScriptPromiseResolver::create(promise, executionContext()), toIsolate(executionContext()));
+    if (!m_options.sysex) {
         m_accessor->startSession();
-        return;
+        return promise;
     }
     Document* document = toDocument(executionContext());
     ASSERT(document);
     MIDIController* controller = MIDIController::from(document->page());
     if (controller) {
-        m_requesting = true;
         controller->requestSysExPermission(this);
     } else {
-        permissionDenied();
+        reject(DOMError::create("SecurityError"));
     }
+    return promise;
 }
 
-void MIDIAccess::permissionDenied()
+void MIDIAccess::resolve()
 {
-    ASSERT(isMainThread());
+    m_asyncResolveRunner.runAsync();
+}
 
-    m_hasAccess = false;
-    m_promise->reject(DOMError::create("SecurityError"));
+void MIDIAccess::reject(PassRefPtr<DOMError> error)
+{
+    m_error = error;
+    m_asyncRejectRunner.runAsync();
+}
+
+void MIDIAccess::resolveNow()
+{
+    m_resolver->resolve(this, executionContext());
+}
+
+void MIDIAccess::rejectNow()
+{
+    m_resolver->reject(m_error.release().get(), executionContext());
+}
+
+void MIDIAccess::doPostAction(State state)
+{
+    ASSERT(m_state == Requesting);
+    ASSERT(state == Resolved || state == Stopped);
+    m_error.clear();
+    if (state == Stopped) {
+        m_accessor.clear();
+    }
+    m_state = state;
 }
 
 void MIDIAccess::trace(Visitor* visitor)
 {
     visitor->trace(m_inputs);
     visitor->trace(m_outputs);
-    visitor->trace(m_promise);
 }
 
 } // namespace WebCore