Web MIDI: do not keep virtual synth open

media/midi/midi_manager_win.cc

Index: media/midi/midi_manager_win.cc
diff --git a/media/midi/midi_manager_win.cc b/media/midi/midi_manager_win.cc
index 5a589cdabe8c32614c4ebbd2ec04baa631c799c6..ca1734987bd3d5b2848ac4e1e147a5df8613fe7a 100644
--- a/media/midi/midi_manager_win.cc
+++ b/media/midi/midi_manager_win.cc
@@ -828,8 +828,7 @@ class MidiServiceWinImpl : public MidiServiceWin,
         make_scoped_refptr(new MidiOutputDeviceState(MidiDeviceInfo(caps)));
     state->midi_handle = midi_out_handle;
     const auto& state_device_info = state->device_info;
-    if (IsUnsupportedDevice(state_device_info))
-      return;
+    bool unsupported = IsUnsupportedDevice(state_device_info);

[Takashi Toyoshima, 2016/11/10 08:12:26]

Note: I tried to close handle here so to just return. But, closing handle inside
this callback seems to fail always. According to MSDN documents, calling MMAPI
inside midi callbacks are not allowed officially, and it may cause an internal
deadlock. I'm planning to rework legacy win32 backend impl later as a side work
of mojo-fication.

     bool add_new_port = false;
     uint32_t port_number = 0;
     {
@@ -860,16 +859,18 @@ class MidiServiceWinImpl : public MidiServiceWin,
           GetManufacturerName(state_device_info),
           base::WideToUTF8(state_device_info.product_name),
           MmversionToString(state_device_info.driver_version),
-          PortState::OPENED);
+          unsupported ? PortState::DISCONNECTED : PortState::OPENED);
       task_thread_.task_runner()->PostTask(
-          FROM_HERE, base::Bind(&MidiServiceWinImpl::AddOutputPortOnTaskThread,
-                                base::Unretained(this), port_info));
+          FROM_HERE,
+          base::Bind(&MidiServiceWinImpl::AddOutputPortOnTaskThread,
+                     base::Unretained(this), port_number, port_info));
     } else {
       task_thread_.task_runner()->PostTask(
           FROM_HERE,
-          base::Bind(&MidiServiceWinImpl::SetOutputPortStateOnTaskThread,
-                     base::Unretained(this), port_number,
-                     PortState::CONNECTED));
+          base::Bind(
+              &MidiServiceWinImpl::SetOutputPortStateOnTaskThread,
+              base::Unretained(this), port_number,
+              unsupported ? PortState::DISCONNECTED : PortState::CONNECTED));
     }
   }
 
@@ -1074,8 +1075,22 @@ class MidiServiceWinImpl : public MidiServiceWin,
     delegate_->OnAddInputPort(info);
   }
 
-  void AddOutputPortOnTaskThread(MidiPortInfo info) {
+  void AddOutputPortOnTaskThread(uint32_t port_index, MidiPortInfo info) {
     AssertOnTaskThread();
+    if (info.state == PortState::DISCONNECTED) {

[yukawa, 2016/11/10 08:13:13]

Fine as long as this is a short-term fix.  Ideally I think we should have an
explicit PortState UNSUPPORTED rather than reusing DISCONNECTED though.

+      // Close immediately if the device is unsupported one. Device information
+      // should be updated inside MOM_CLOSE callback.
+      HMIDIOUT midi_handle = kInvalidMidiOutHandle;
+      {
+        base::AutoLock auto_lock(output_ports_lock_);
+        scoped_refptr<MidiOutputDeviceState> device_state =
+            output_ports_[port_index];
+        if (device_state)
+          midi_handle = device_state->midi_handle;
+      }
+      if (midi_handle != kInvalidMidiOutHandle)
+        midiOutClose(midi_handle);
+    }
     delegate_->OnAddOutputPort(info);
   }
 
@@ -1086,6 +1101,20 @@ class MidiServiceWinImpl : public MidiServiceWin,
 
   void SetOutputPortStateOnTaskThread(uint32_t port_index, PortState state) {
     AssertOnTaskThread();
+    if (state == PortState::DISCONNECTED) {

[yukawa, 2016/11/10 08:13:13]

Ditto.

+      // Close immediately if the device is unsupported one and still opened.
+      // Device information should be updated inside MOM_CLOSE callback.
+      HMIDIOUT midi_handle = kInvalidMidiOutHandle;
+      {
+        base::AutoLock auto_lock(output_ports_lock_);
+        scoped_refptr<MidiOutputDeviceState> device_state =
+            output_ports_[port_index];
+        if (device_state && device_state->midi_handle != kInvalidMidiOutHandle)
+          midi_handle = device_state->midi_handle;
+      }
+      if (midi_handle != kInvalidMidiOutHandle)
+        midiOutClose(midi_handle);
+    }
     delegate_->OnSetOutputPortState(port_index, state);
   }