Make WebMIDI use blink Promise.

Source/modules/webmidi/MIDIAccess.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 13 matching lines @@
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "modules/webmidi/MIDIAccess.h"
 
+#include "bindings/v8/DOMRequestState.h"
+#include "bindings/v8/DOMWrapperWorld.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 "modules/webmidi/NavigatorWebMIDI.h"
 
 namespace WebCore {
 
 DEFINE_GC_INFO(MIDIAccess);
 
-PassRefPtrWillBeRawPtr<MIDIAccess> MIDIAccess::create(ExecutionContext* context, MIDIAccessPromise* promise)
+PassRefPtrWillBeRawPtr<MIDIAccess> MIDIAccess::create(const MIDIOptions& options, ExecutionContext* context)
 {
-    RefPtrWillBeRawPtr<MIDIAccess> midiAccess(adoptRefCountedWillBeRefCountedGarbageCollected(new MIDIAccess(context, promise)));
+    RefPtrWillBeRawPtr<MIDIAccess> midiAccess(adoptRefCountedWillBeRefCountedGarbageCollected(new MIDIAccess(options, context)));
     midiAccess->suspendIfNeeded();
-    midiAccess->startRequest();
     return midiAccess.release();
 }
 
 MIDIAccess::~MIDIAccess()
 {
-    stop();
+    ASSERT(!m_requesting);
 }
 
-MIDIAccess::MIDIAccess(ExecutionContext* context, MIDIAccessPromise* promise)
+MIDIAccess::MIDIAccess(const MIDIOptions& options, ExecutionContext* context)
     : ActiveDOMObject(context)
-    , m_promise(promise)
+    , m_options(options)
     , m_hasAccess(false)
     , m_sysExEnabled(false)
     , m_requesting(false)
 {
     ScriptWrappable::init(this);
     m_accessor = MIDIAccessor::create(this);
 }
 
 void MIDIAccess::setSysExEnabled(bool enable)
 {
-    m_requesting = false;
+    RefPtr<MIDIAccess> protect(this);
     m_sysExEnabled = enable;
     if (enable)
         m_accessor->startSession();
     else
-        permissionDenied();
+        reject(DOMError::create("SecurityError"));
 }
 
 void MIDIAccess::didAddInputPort(const String& id, const String& manufacturer, const String& name, const String& version)
 {
     ASSERT(isMainThread());
 
     m_inputs.append(MIDIInput::create(this, id, manufacturer, name, version));
 }
 
 void MIDIAccess::didAddOutputPort(const String& id, const String& manufacturer, const String& name, const String& version)
 {
     ASSERT(isMainThread());
 
     unsigned portIndex = m_outputs.size();
     m_outputs.append(MIDIOutput::create(this, portIndex, id, manufacturer, name, version));
 }
 
 void MIDIAccess::didStartSession(bool success)
 {
+    RefPtr<MIDIAccess> protect(this);
     ASSERT(isMainThread());
-
-    m_hasAccess = success;
+    if (!m_requesting)
+        return;
     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()) {
         // 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.
@@ skipping 23 matching lines @@
             double documentStartTime = document->loader()->timing()->referenceMonotonicTime();
             timeStamp = documentStartTime + 0.001 * timeStampInMilliseconds;
         }
 
         m_accessor->sendMIDIData(portIndex, data, length, timeStamp);
     }
 }
 
 void MIDIAccess::stop()
 {
+    RefPtr<MIDIAccess> protect(this);
     m_hasAccess = false;
     if (!m_requesting)
         return;
-    m_requesting = false;
     Document* document = toDocument(executionContext());
     ASSERT(document);
     MIDIController* controller = MIDIController::from(document->page());
     ASSERT(controller);
     controller->cancelSysExPermissionRequest(this);
+
+    m_accessor.clear();
+    reject(DOMError::create("AbortError"));
 }
 
-void MIDIAccess::startRequest()
+void MIDIAccess::permissionDenied()
 {
-    if (!m_promise->options()->sysex) {
+    RefPtr<MIDIAccess> protect(this);
+    ASSERT(isMainThread());
+    reject(DOMError::create("SecurityError"));
+}
+
+ScriptPromise MIDIAccess::startRequest()
+{
+    ASSERT(!m_requesting);
+    RefPtr<MIDIAccess> protect(this);

[haraken, 2014/02/21 11:11:40]

Why do you need protection? Is this "just in case"?

It's true that |this| object can be destructed in resolve()/reject(), but
resolve()/reject() are executed at the very end of this method. Since we're not
touching |this| object after resolve()/reject(), I don't think we need the
protection. (In any event, if we're touching |this| object after
resolve()/reject(), we're doing something wrong.)

[yhirano, 2014/02/21 13:52:35]

I think they were introduced by your suggestion.
https://codereview.chromium.org/77773003/diff/230001/Source/modules/webmidi/M...

[yhirano, 2014/02/26 09:18:08]

I deleted many of protect(this).

+    ScriptPromise promise = ScriptPromise::createPending();
+    m_resolver = ScriptPromiseResolver::create(promise, executionContext());
+    m_world = RefPtr<DOMWrapperWorld>(DOMWrapperWorld::current(toIsolate(executionContext())));
+    m_requesting = true;
+    setPendingActivity(this);

[haraken, 2014/02/21 11:11:40]

m_requesting and setPendingActivity should be synchronized. Can you add a helper
method that updates the two variables?

[yhirano, 2014/02/21 13:52:35]

Done.

+    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;
+    if (controller)
         controller->requestSysExPermission(this);
-    } else {
-        permissionDenied();
-    }
+    else
+        reject(DOMError::create("SecurityError"));
+    return promise;
 }
 
-void MIDIAccess::permissionDenied()
+void MIDIAccess::resolve()
 {
-    ASSERT(isMainThread());
+    RefPtr<MIDIAccess> protect(this);
+    if (!m_requesting)
+        return;
+    ASSERT(m_world.get());
+    DOMRequestState state(executionContext(), m_world);
+    DOMRequestState::Scope scope(state);

[haraken, 2014/02/21 11:11:40]

I don't understand why you need DOMRequestState here.

DOMRequestState holds a world. MIDIAccess also holds a world. This implies that
something is mis-designed...

[yhirano, 2014/02/21 13:52:35]

I introduced MIDIAccessResolver.

+    m_world.clear();
+    m_hasAccess = true;

[haraken, 2014/02/21 11:11:40]

What is m_hasAccess for?

[yhirano, 2014/02/21 13:52:35]

m_hasAccess is used in didReceiveMIDIData and sendMIDIData.
This class blocks input / output operations when it is false.

+    m_requesting = false;
+    unsetPendingActivity(this);
+    // FIXME: Care about suspend / stop.

[haraken, 2014/02/21 11:11:40]

I think you need to fix this in this CL.

[yhirano, 2014/02/21 13:52:35]

Done.

+    m_resolver->resolve(this, executionContext());
+}
 
+void MIDIAccess::reject(PassRefPtr<DOMError> error)
+{
+    RefPtr<MIDIAccess> protect(this);
+    if (!m_requesting)
+        return;
+    ASSERT(m_world.get());
+    DOMRequestState state(executionContext(), m_world);
+    DOMRequestState::Scope scope(state);
+    m_world.clear();
     m_hasAccess = false;
-    m_promise->reject(DOMError::create("SecurityError"));
+    m_requesting = false;
+    unsetPendingActivity(this);
+    // FIXME: Care about suspend / stop.
+    m_resolver->reject(error, executionContext());
 }
 
 void MIDIAccess::trace(Visitor* visitor)
 {
     visitor->trace(m_inputs);
     visitor->trace(m_outputs);
-    visitor->trace(m_promise);
 }
 
 } // namespace WebCore