Web MIDI: MIDIPort::open/close should run asynchronously

Current implementation dispatch MIDIConnectionEvent synchronously,
but this should be done asynchronously according to the spec.

BUG=735058

Change-Id: If49d588e36e092aa6fc40625fd88483c9ae1719d
Reviewed-on: https://chromium-review.googlesource.com/545577
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486745}

third_party/WebKit/LayoutTests/webmidi/implicit-open-expected.txt

@@ -16,8 +16,6 @@ PASS eventport.connection is "open"
 PASS handler is called with port [object MIDIInput].
 PASS eventport.id is "MockInputID-0"
 PASS eventport.connection is "open"
-- check final state.
-PASS port.connection is "open"
 - reset input device
 Check state transition for addeventlistener on closed state.
 - check initial state.
@@ -30,8 +28,6 @@ PASS eventport.connection is "open"
 PASS handler is called with port [object MIDIInput].
 PASS eventport.id is "MockInputID-0"
 PASS eventport.connection is "open"
-- check final state.
-PASS port.connection is "open"
 Check state transition for send on closed state.
 - check initial state.
 PASS port.connection is "closed"
@@ -43,8 +39,6 @@ PASS eventport.connection is "open"
 PASS handler is called with port [object MIDIOutput].
 PASS eventport.id is "MockOutputID-0"
 PASS eventport.connection is "open"
-- check final state.
-PASS port.connection is "open"
 PASS successfullyParsed is true
 
 TEST COMPLETE

third_party/WebKit/LayoutTests/webmidi/implicit-open.html

@@ -16,8 +16,7 @@ function resetInputDevice(port) {
     // Reset event handlers so that close() does not cause check failures.
     port.onstatechange = null;
     access.onstatechange = null;
-    port.close();
-    return Promise.resolve();
+    return port.close();
 }
 
 function successAccessCallback(a) {

third_party/WebKit/LayoutTests/webmidi/state-check-utils.js

@@ -13,30 +13,31 @@ function checkStateTransition(options) {
         shouldBeEqualToString("eventport.id", options.port.id);
         shouldBeEqualToString("eventport.connection", options.finalconnection);
     };
-    port.onstatechange = function(e) {
-        debug("- check port handler.");
-        checkHandler(e);
-    };
-    access.onstatechange = function(e) {
-        debug("- check access handler.");
-        checkHandler(e);
-    };
-    if (options.method == "send") {
-        port.send([]);
-    }
+    const portPromise = new Promise(resolve => {
+        port.onstatechange = e => {
+            debug("- check port handler.");
+            checkHandler(e);
+            resolve();
+        };
+    });
+    const accessPromise = new Promise(resolve => {
+        access.onstatechange = e => {
+            debug("- check access handler.");
+            checkHandler(e);
+            resolve();
+        };
+    });
     if (options.method == "setonmidimessage") {
         port.onmidimessage = function() {};
+        return Promise.all([portPromise, accessPromise]);
     }
     if (options.method == "addeventlistener") {
         port.addEventListener("midimessage", function() {});
+        return Promise.all([portPromise, accessPromise]);
     }
-    if (options.method == "send" || options.method == "setonmidimessage" ||
-        options.method == "addeventlistener") {
-        // Following tests expect an implicit open finishes synchronously.
-        // But it will be asynchronous in the future.
-        debug("- check final state.");
-        shouldBeEqualToString("port.connection", options.finalconnection);
-        return Promise.resolve();
+    if (options.method == "send") {
+        port.send([]);
+        return Promise.all([portPromise, accessPromise]);
     }
     // |method| is expected to be "open" or "close".
     return port[options.method]().then(function(p) {

third_party/WebKit/Source/modules/webmidi/MIDIOutput.cpp

@@ -240,8 +240,14 @@ void MIDIOutput::send(NotShared<DOMUint8Array> array,
 
   if (MessageValidator::Validate(array.View(), exception_state,
                                  midiAccess()->sysexEnabled())) {
-    midiAccess()->SendMIDIData(port_index_, array.View()->Data(),
-                               array.View()->length(), timestamp);
+    if (IsOpening()) {
+      pending_data_.push_back(std::make_pair(Vector<uint8_t>(), timestamp));
+      pending_data_.back().first.Append(array.View()->Data(),
+                                        array.View()->length());
+    } else {
+      midiAccess()->SendMIDIData(port_index_, array.View()->Data(),
+                                 array.View()->length(), timestamp);
+    }
   }
 }
 
@@ -280,6 +286,17 @@ void MIDIOutput::send(Vector<unsigned> unsigned_data,
   send(unsigned_data, 0.0, exception_state);
 }
 
+void MIDIOutput::DidOpen(bool opened) {
+  while (!pending_data_.empty()) {
+    if (opened) {
+      auto& front = pending_data_.front();
+      midiAccess()->SendMIDIData(port_index_, front.first.data(),
+                                 front.first.size(), front.second);
+    }
+    pending_data_.TakeFirst();
+  }
+}
+
 DEFINE_TRACE(MIDIOutput) {
   MIDIPort::Trace(visitor);
 }

third_party/WebKit/Source/modules/webmidi/MIDIOutput.h

@@ -31,6 +31,7 @@
 #ifndef MIDIOutput_h
 #define MIDIOutput_h
 
+#include <utility>
 #include "core/typed_arrays/ArrayBufferViewHelpers.h"
 #include "core/typed_arrays/DOMTypedArray.h"
 #include "modules/webmidi/MIDIPort.h"
@@ -71,7 +72,10 @@ class MIDIOutput final : public MIDIPort {
              const String& version,
              midi::mojom::PortState);
 
+  void DidOpen(bool opened) override;
+
   unsigned port_index_;
+  Deque<std::pair<Vector<uint8_t>, double>> pending_data_;
 };
 
 }  // namespace blink

third_party/WebKit/Source/modules/webmidi/MIDIPort.cpp

@@ -33,6 +33,7 @@
 #include "bindings/core/v8/ScriptPromise.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/Document.h"
+#include "core/dom/TaskRunnerHelper.h"
 #include "core/frame/UseCounter.h"
 #include "modules/webmidi/MIDIAccess.h"
 #include "modules/webmidi/MIDIConnectionEvent.h"
@@ -98,17 +99,37 @@ String MIDIPort::type() const {
 }
 
 ScriptPromise MIDIPort::open(ScriptState* script_state) {
-  open();
-  return Accept(script_state);
+  if (connection_ == kConnectionStateOpen)
+    return Accept(script_state);
+
+  ScriptPromiseResolver* resolver = ScriptPromiseResolver::Create(script_state);
+  TaskRunnerHelper::Get(TaskType::kMiscPlatformAPI, GetExecutionContext())
+      ->PostTask(BLINK_FROM_HERE,
+                 WTF::Bind(&MIDIPort::OpenAsynchronously, WrapPersistent(this),
+                           WrapPersistent(resolver)));
+  running_open_count_++;
+  return resolver->Promise();
+}
+
+void MIDIPort::open() {
+  if (connection_ == kConnectionStateOpen || running_open_count_)
+    return;
+  TaskRunnerHelper::Get(TaskType::kMiscPlatformAPI, GetExecutionContext())
+      ->PostTask(BLINK_FROM_HERE, WTF::Bind(&MIDIPort::OpenAsynchronously,
+                                            WrapPersistent(this), nullptr));
+  running_open_count_++;
 }
 
 ScriptPromise MIDIPort::close(ScriptState* script_state) {
-  if (connection_ != kConnectionStateClosed) {
-    // TODO(toyoshim): Do clear() operation on MIDIOutput.
-    // TODO(toyoshim): Add blink API to perform a real close operation.
-    SetStates(state_, kConnectionStateClosed);
-  }
-  return Accept(script_state);
+  if (connection_ == kConnectionStateClosed)
+    return Accept(script_state);
+
+  ScriptPromiseResolver* resolver = ScriptPromiseResolver::Create(script_state);
+  TaskRunnerHelper::Get(TaskType::kMiscPlatformAPI, GetExecutionContext())
+      ->PostTask(BLINK_FROM_HERE,
+                 WTF::Bind(&MIDIPort::CloseAsynchronously, WrapPersistent(this),
+                           WrapPersistent(resolver)));
+  return resolver->Promise();
 }
 
 void MIDIPort::SetState(PortState state) {
@@ -173,9 +194,13 @@ DEFINE_TRACE_WRAPPERS(MIDIPort) {
   EventTargetWithInlineData::TraceWrappers(visitor);
 }
 
-void MIDIPort::open() {
+void MIDIPort::OpenAsynchronously(ScriptPromiseResolver* resolver) {
   UseCounter::Count(*ToDocument(GetExecutionContext()),
                     WebFeature::kMIDIPortOpen);
+  DCHECK_NE(0u, running_open_count_);
+  running_open_count_--;
+
+  DidOpen(state_ == PortState::CONNECTED);
   switch (state_) {
     case PortState::DISCONNECTED:
       SetStates(state_, kConnectionStatePending);
@@ -189,6 +214,16 @@ void MIDIPort::open() {
       NOTREACHED();
       break;
   }
+  if (resolver)
+    resolver->Resolve(this);
+}
+
+void MIDIPort::CloseAsynchronously(ScriptPromiseResolver* resolver) {
+  DCHECK(resolver);
+  // TODO(toyoshim): Do clear() operation on MIDIOutput.
+  // TODO(toyoshim): Add blink API to perform a real close operation.
+  SetStates(state_, kConnectionStateClosed);
+  resolver->Resolve(this);
 }
 
 ScriptPromise MIDIPort::Accept(ScriptState* script_state) {

third_party/WebKit/Source/modules/webmidi/MIDIPort.h

@@ -32,6 +32,7 @@
 #define MIDIPort_h
 
 #include "bindings/core/v8/ScriptPromise.h"
+#include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/ContextLifecycleObserver.h"
 #include "core/dom/ExceptionCode.h"
 #include "media/midi/midi_service.mojom-blink.h"
@@ -105,9 +106,14 @@ class MIDIPort : public EventTargetWithInlineData,
            midi::mojom::PortState);
 
   void open();
+  bool IsOpening() { return running_open_count_; }
   MIDIAccess* midiAccess() const { return access_; }
 
  private:
+  void OpenAsynchronously(ScriptPromiseResolver*);
+  virtual void DidOpen(bool opened) {}
+  void CloseAsynchronously(ScriptPromiseResolver*);
+
   ScriptPromise Accept(ScriptState*);
   ScriptPromise Reject(ScriptState*, ExceptionCode, const String& message);
 
@@ -121,6 +127,7 @@ class MIDIPort : public EventTargetWithInlineData,
   TraceWrapperMember<MIDIAccess> access_;
   midi::mojom::PortState state_;
   ConnectionState connection_;
+  unsigned running_open_count_ = 0;
 };
 
 }  // namespace blink