Web MIDI: Improve device change monitornig on Windows

media/midi/midi_manager_win.cc

 // Copyright 2013 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 "media/midi/midi_manager_win.h"
 
 #include <windows.h>
-#include <dbt.h>
 #include <ks.h>
 #include <ksmedia.h>
 #include <mmreg.h>
 // Prevent unnecessary functions from being included from <mmsystem.h>
 #define MMNODRV
 #define MMNOSOUND
 #define MMNOWAVE
 #define MMNOAUX
 #define MMNOMIXER
 #define MMNOTIMER
@@ skipping 10 matching lines @@
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/containers/hash_tables.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/system_monitor/system_monitor.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_checker.h"
 #include "base/timer/timer.h"
 #include "base/win/message_window.h"
 #include "device/usb/usb_ids.h"
 #include "media/midi/midi_manager.h"
 #include "media/midi/midi_message_queue.h"
 #include "media/midi/midi_message_util.h"
 #include "media/midi/midi_port_info.h"
 
 namespace media {
 namespace {
 
-const wchar_t kWindowClassName[] = L"ChromeMidiDeviceMonitorWindow";
-const int kOnDeviceArrivalDelayMilliSec = 500;
 static const size_t kBufferLength = 32 * 1024;
 
 // We assume that nullpter represents an invalid MIDI handle.
 const HMIDIIN kInvalidMidiInHandle = nullptr;
 const HMIDIOUT kInvalidMidiOutHandle = nullptr;
 
-// Note that |RegisterDeviceNotificationW| returns nullptr as an invalid handle.
-const HDEVNOTIFY kInvalidDeviceNotifyHandle = nullptr;
-
 std::string GetInErrorMessage(MMRESULT result) {
   wchar_t text[MAXERRORLENGTH];
   MMRESULT get_result = midiInGetErrorText(result, text, arraysize(text));
   if (get_result != MMSYSERR_NOERROR) {
     DLOG(ERROR) << "Failed to get error message."
                 << " original error: " << result
                 << " midiInGetErrorText error: " << get_result;
     return std::string();
   }
   return base::WideToUTF8(text);
@@ skipping 210 matching lines @@
       return 0;
     return EXTRACT_USBAUDIO_PID(&caps.ProductGuid);
   }
   static uint16 ExtractUsbProductIdIfExists(const MIDIOUTCAPS2W& caps) {
     if (!IS_COMPATIBLE_USBAUDIO_PID(&caps.ProductGuid))
       return 0;
     return EXTRACT_USBAUDIO_PID(&caps.ProductGuid);
   }
 };
 
 std::string GetManufacturerName(const MidiDeviceInfo& info) {

[Takashi Toyoshima, 2015/04/06 18:02:38]

This fix is split to another CL.

[yukawa, 2015/04/06 18:41:41]

Acknowledged.

-  if (info.is_usb_device)
-    return device::UsbIds::GetVendorName(info.usb_vendor_id);
+  if (info.is_usb_device) {
+    const char* name = device::UsbIds::GetVendorName(info.usb_vendor_id);
+    return std::string(name ? name : "");
+  }
 
   switch (info.manufacturer_id) {
     case MM_MICROSOFT:
       return "Microsoft Corporation";
     default:
       // TODO(toyoshim): Support other manufacture IDs.  crbug.com/472341.
       return "";
   }
 }
 
 using PortNumberCache = base::hash_map<
     MidiDeviceInfo,
     std::priority_queue<uint32, std::vector<uint32>, std::greater<uint32>>,
     MidiDeviceInfo::Hasher>;
 
-class DeviceMonitorWindow final {
- public:
-  explicit DeviceMonitorWindow(base::Closure on_device_arrival)
-      : on_device_arrival_(on_device_arrival),
-        dev_notify_handle_(kInvalidDeviceNotifyHandle) {
-    window_.reset(new base::win::MessageWindow);
-    if (!window_->CreateNamed(base::Bind(&DeviceMonitorWindow::HandleMessage,
-                                         base::Unretained(this)),
-                              base::string16(kWindowClassName))) {
-      LOG(ERROR) << "Failed to create message window: " << kWindowClassName;
-      window_.reset();
-    }
-    DEV_BROADCAST_DEVICEINTERFACE kBloadcastRequest = {
-        sizeof(DEV_BROADCAST_DEVICEINTERFACE), DBT_DEVTYP_DEVICEINTERFACE,
-    };
-    dev_notify_handle_ = RegisterDeviceNotificationW(
-        window_->hwnd(), &kBloadcastRequest,
-        DEVICE_NOTIFY_WINDOW_HANDLE | DEVICE_NOTIFY_ALL_INTERFACE_CLASSES);
-  }
-
-  ~DeviceMonitorWindow() {
-    if (dev_notify_handle_ != kInvalidDeviceNotifyHandle) {
-      BOOL result = UnregisterDeviceNotification(dev_notify_handle_);
-      if (!result) {
-        const DWORD last_error = GetLastError();
-        DLOG(ERROR) << "UnregisterDeviceNotification failed. error: "
-                    << last_error;
-      }
-      dev_notify_handle_ = kInvalidDeviceNotifyHandle;
-    }
-  }
-
- private:
-  bool HandleMessage(UINT message,
-                     WPARAM wparam,
-                     LPARAM lparam,
-                     LRESULT* result) {
-    if (message == WM_DEVICECHANGE) {
-      switch (wparam) {
-        case DBT_DEVICEARRIVAL: {
-          device_arrival_timer_.Stop();
-          device_arrival_timer_.Start(
-              FROM_HERE,
-              base::TimeDelta::FromMilliseconds(kOnDeviceArrivalDelayMilliSec),
-              on_device_arrival_);
-          break;
-        }
-      }
-    }
-    return false;
-  }
-
-  base::Closure on_device_arrival_;
-  base::OneShotTimer<DeviceMonitorWindow> device_arrival_timer_;
-  base::ThreadChecker thread_checker_;
-  scoped_ptr<base::win::MessageWindow> window_;
-  HDEVNOTIFY dev_notify_handle_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeviceMonitorWindow);
-};
-
 struct MidiInputDeviceState final : base::RefCounted<MidiInputDeviceState> {
   explicit MidiInputDeviceState(const MidiDeviceInfo& device_info)
       : device_info(device_info),
         midi_handle(kInvalidMidiInHandle),
         port_index(0),
         port_age(0),
         start_time_initialized(false) {}
 
   const MidiDeviceInfo device_info;
   HMIDIIN midi_handle;
@@ skipping 34 matching lines @@
   uint64 port_age;
   // True if the device is already closed and |midi_handle| is considered to be
   // invalid.
   // TODO(toyoshim): Use std::atomic<bool> when it is allowed in Chromium
   // project.
   volatile bool closed;
 };
 
 // The core logic of MIDI device handling for Windows. Basically this class is
 // shared among following 4 threads:
-//  1. Device Monitor Thread
+//  1. Chrome IO Thread
 //  2. OS Multimedia Thread
-//  3. Misc. Task Thread
+//  3. Task Thread
 //  4. Sender Thread
 //
-// Device Monitor Thread:
-//  This is a standalone UI thread that is dedicated for a message-only window
-//  which will receive WM_DEVICECHANGE Win32 message whose
-//  wParam == DBT_DEVICEARRIVAL. This event will be used as the trigger to open
-//  all the new MIDI devices. Note that in the current implementation we will
-//  try to open all the existing devices in practice. This is OK because trying
-//  to reopen a MIDI device that is already opened would simply fail, and there
-//  is no unwilling side effect. Note also that this thread isn't responsible
-//  for updating the device database. It will be handled by MIM_OPEN/MOM_OPEN
-//  events dispatched to OS Multimedia Thread.
+// Chrome IO Thread:
+//  MidiManager runs on Chrome IO thread. Device change notification is
+//  delivered to the thread through the thread-safe SystemMonitor service.

[yukawa, 2015/04/06 18:41:40]

I'm afraid that "thread-safe" here is ambiguous.  Although
|base::SystemMonitor::AddDevicesChangedObserver| can be called from any thread
except or the notification callback, the document comment on
|base::SystemMonitor::RemoveDevicesChangedObserver| is extremely misleading. 
You MUST call |base::SystemMonitor::RemoveDevicesChangedObserver| from the same
thread which called |base::SystemMonitor::AddDevicesChangedObserver|.  See
comment on |ObserverListThreadSafe::RemoveObserver|.  Otherwise, you might see
wired use-after-free crashes.  http://crbug.com/404767 is an unfortunate
example.

The same comment to "thread-safe" in the CL description.

[Takashi Toyoshima, 2015/04/06 19:51:47]

Aha. "thread-safe" is always a vague word.
I just mean that the callback can be registered from any thread and the callback
is always invoked on the same thread here. I'd remove "thread-safe" word here.

+//  OnDevicesChanged() callback is invoked to update the MIDI device list.
+//  Note that in the current implementation we will try to open all the
+//  existing devices in practice. This is OK because trying to reopen a MIDI
+//  device that is already opened would simply fail, and there is no unwilling
+//  side effect.
 //
 // OS Multimedia Thread:
 //  This thread is maintained by the OS as a part of MIDI runtime, and
 //  responsible for receiving all the system initiated events such as device
-//  open and close. For performance reasons, most of potentially blocking
-//  operations will be dispatched into Misc. Task Thread.
+//  close, and receiving data. For performance reasons, most of potentially
+//  blocking operations will be dispatched into Task Thread.
 //
-// Misc. Task Thread:
+// Task Thread:
 //  This thread will be used to call back following methods of MidiManager.
 //  - MidiManager::CompleteInitialization
 //  - MidiManager::AddInputPort
 //  - MidiManager::AddOutputPort
 //  - MidiManager::SetInputPortState
 //  - MidiManager::SetOutputPortState
 //  - MidiManager::ReceiveMidiData
 //
 // Sender Thread:
 //  This thread will be used to call Win32 APIs to send MIDI message at the
-//  specified time. We don't want to call MIDI send APIs on Misc. Task Thread
+//  specified time. We don't want to call MIDI send APIs on Task Thread
 //  because those APIs could be performed synchronously, hence they could block
 //  the caller thread for a while. See the comment in
 //  SendLongMidiMessageInternal for details. Currently we expect that the
 //  blocking time would be less than 1 second.
-class MidiServiceWinImpl : public MidiServiceWin {
+class MidiServiceWinImpl : public MidiServiceWin,
+                           public base::SystemMonitor::DevicesChangedObserver {
  public:
   MidiServiceWinImpl()
       : delegate_(nullptr),
-        monitor_thread_("Windows MIDI device monitor thread"),
         sender_thread_("Windows MIDI sender thread"),
         task_thread_("Windows MIDI task thread"),
         destructor_started(false) {}
 
   ~MidiServiceWinImpl() final {
+    base::SystemMonitor::Get()->RemoveDevicesChangedObserver(this);

[yukawa, 2015/04/06 18:41:40]

We might want to make sure that |RemoveDevicesChangedObserver| is called from
the same thread which called AddObserver.

[Takashi Toyoshima, 2015/04/06 19:51:47]

ok, ThreadChecker is our friend.

     destructor_started = true;
-    if (monitor_thread_.IsRunning()) {
-      monitor_thread_.message_loop()->PostTask(
-          FROM_HERE,
-          base::Bind(&MidiServiceWinImpl::StopDeviceMonitorOnMonitorThreadSync,
-                     base::Unretained(this)));
-    }
     {
       std::vector<HMIDIIN> input_devices;
       {
         base::AutoLock auto_lock(input_ports_lock_);
         for (auto it : input_device_map_)
           input_devices.push_back(it.first);
       }
       {
         for (const auto handle : input_devices) {
           MMRESULT result = midiInClose(handle);
@@ skipping 22 matching lines @@
             result = midiOutReset(handle);
             DLOG_IF(ERROR, result != MMSYSERR_NOERROR)
                 << "midiOutReset failed: " << GetOutErrorMessage(result);
             result = midiOutClose(handle);
           }
           DLOG_IF(ERROR, result != MMSYSERR_NOERROR)
               << "midiOutClose failed: " << GetOutErrorMessage(result);
         }
       }
     }
-    monitor_thread_.Stop();
     sender_thread_.Stop();
     task_thread_.Stop();
   }
 
   // MidiServiceWin overrides:
   void InitializeAsync(MidiServiceWinDelegate* delegate) final {
     delegate_ = delegate;
-    sender_thread_.StartWithOptions(
-        base::Thread::Options(base::MessageLoop::TYPE_IO, 0));
-    task_thread_.StartWithOptions(
-        base::Thread::Options(base::MessageLoop::TYPE_IO, 0));
-    monitor_thread_.StartWithOptions(
-        base::Thread::Options(base::MessageLoop::TYPE_UI, 0));
-    monitor_thread_.message_loop()->PostTask(
-        FROM_HERE, base::Bind(&MidiServiceWinImpl::InitializeOnMonitorThread,
-                              base::Unretained(this)));
+    sender_thread_.Start();

[Takashi Toyoshima, 2015/04/06 18:02:38]

TYPE_IO has a misleading name, but it isn't for blocking call, but for async
non-blocking calls. Let's use default type.

[yukawa, 2015/04/06 18:41:40]

Acknowledged.

+    task_thread_.Start();
+
+    // Start monitoring device changes. This should start before the

[yukawa, 2015/04/06 18:41:40]

Can you elaborate a bit more on why we can avoid race.

[Takashi Toyoshima, 2015/04/06 19:51:47]

Done.

+    // following open operation to avoid a race.
+    base::SystemMonitor::Get()->AddDevicesChangedObserver(this);
+
+    UpdateDeviceList();
+
+    task_thread_.message_loop()->PostTask(
+        FROM_HERE,
+        base::Bind(&MidiServiceWinImpl::CompleteInitializationOnTaskThread,
+                   base::Unretained(this), MIDI_OK));
   }
 
   void SendMidiDataAsync(uint32 port_number,
                          const std::vector<uint8>& data,
                          base::TimeTicks time) final {
     if (destructor_started) {
       LOG(ERROR) << "ThreadSafeSendData failed because MidiServiceWinImpl is "
                     "being destructed.  port: " << port_number;
       return;
     }
@@ skipping 17 matching lines @@
                                 state->port_age, data, time),
           time - now);
     } else {
       sender_thread_.message_loop()->PostTask(
           FROM_HERE, base::Bind(&MidiServiceWinImpl::SendOnSenderThread,
                                 base::Unretained(this), port_number,
                                 state->port_age, data, time));
     }
   }
 
+  // base::SystemMonitor::DevicesChangedObserver overrides:
+  void OnDevicesChanged(base::SystemMonitor::DeviceType device_type) final {

[yukawa, 2015/04/06 18:41:40]

Can we predict (and have an assertion) that which thread calls this method?

[Takashi Toyoshima, 2015/04/06 19:51:47]

Done.

+    switch (device_type) {
+      case base::SystemMonitor::DEVTYPE_AUDIO_CAPTURE:
+      case base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE:
+        // Add case of other unrelated device types here.
+        return;
+      case base::SystemMonitor::DEVTYPE_UNKNOWN:
+        // Interested in MIDI devices. Try updating the device list.
+        UpdateDeviceList();

[yukawa, 2015/04/06 18:41:40]

|UpdateDeviceList()| is supposed to be a blocking call.  What will happen in
other observers of SystemMonitor if |OnDevicesChanged| doesn't return
immediately? 

[Takashi Toyoshima, 2015/04/06 19:51:47]

OnDeviceChanged is invoked via PostTask on the registered thread. In our case,
it's on the Chrome IO Thread. So, it isn't critical, but also isn't good. As far
as I tried, all operations are not black listed on the thread.
Anyway, let's post it to task thread.

[yukawa, 2015/04/06 20:18:48]

The question which thread should be used for opening MIDI deviecs is difficult
in practice because we also rely on the task thread to dispatch the MIDI Receive
event. So in theory overloading tasks to the task thread might increase the risk
of jitter in MIDI receive events. It depends on the event rate of
DBT_DEVNODES_CHANGES and worst case blocking time of enumerating all the MIDI
devices and opening all of them in real world though.
I'm OK to start with the design of patch set #3, but we need to understand stats
in real world to choose the best design.

+        break;
+        // No default here to capture new DeviceType by compile time.
+    }
+  }
+
  private:
   scoped_refptr<MidiInputDeviceState> GetInputDeviceFromHandle(
       HMIDIIN midi_handle) {
     base::AutoLock auto_lock(input_ports_lock_);
     const auto it = input_device_map_.find(midi_handle);
     return (it != input_device_map_.end() ? it->second : nullptr);
   }
 
   scoped_refptr<MidiOutputDeviceState> GetOutputDeviceFromHandle(
       HMIDIOUT midi_handle) {
     base::AutoLock auto_lock(output_ports_lock_);
     const auto it = output_device_map_.find(midi_handle);
     return (it != output_device_map_.end() ? it->second : nullptr);
   }
 
   scoped_refptr<MidiOutputDeviceState> GetOutputDeviceFromPort(
       uint32 port_number) {
     base::AutoLock auto_lock(output_ports_lock_);
     if (output_ports_.size() <= port_number)
       return nullptr;
     return output_ports_[port_number];
   }
 
+  void UpdateDeviceList() {

[Takashi Toyoshima, 2015/04/06 18:02:38]

Name is changed because we do not want to open the device here in the future.

[yukawa, 2015/04/06 18:41:40]

Acknowledged.

+    const UINT num_in_devices = midiInGetNumDevs();
+    for (UINT device_id = 0; device_id < num_in_devices; ++device_id) {
+      // Here we use |CALLBACK_FUNCTION| to subscribe MIM_DATA, MIM_LONGDATA,
+      // MIM_OPEN, and MIM_CLOSE events.
+      // - MIM_DATA: This is the only way to get a short MIDI message with
+      //     timestamp information.
+      // - MIM_LONGDATA: This is the only way to get a long MIDI message with
+      //     timestamp information.
+      // - MIM_OPEN: This event is sent the input device is opened. Note that

[Takashi Toyoshima, 2015/04/06 18:02:38]

Note is added.

[yukawa, 2015/04/06 18:41:40]

Acknowledged.

+      //     this message is called on the caller thread.
+      // - MIM_CLOSE: This event is sent when 1) midiInClose() is called, or 2)
+      //     the MIDI device becomes unavailable for some reasons, e.g., the
+      //     cable is disconnected. As for the former case, HMIDIOUT will be
+      //     invalidated soon after the callback is finished. As for the later
+      //     case, however, HMIDIOUT continues to be valid until midiInClose()
+      //     is called.
+      HMIDIIN midi_handle = kInvalidMidiInHandle;
+      const MMRESULT result = midiInOpen(
+          &midi_handle, device_id,
+          reinterpret_cast<DWORD_PTR>(&OnMidiInEventOnMainlyMultimediaThread),
+          reinterpret_cast<DWORD_PTR>(this), CALLBACK_FUNCTION);
+      DLOG_IF(ERROR, result != MMSYSERR_NOERROR && result != MMSYSERR_ALLOCATED)
+          << "Failed to open output device. "
+          << " id: " << device_id << " message: " << GetInErrorMessage(result);
+    }
+
+    const UINT num_out_devices = midiOutGetNumDevs();
+    for (UINT device_id = 0; device_id < num_out_devices; ++device_id) {
+      // Here we use |CALLBACK_FUNCTION| to subscribe MOM_DONE, MOM_OPEN, and
+      // MOM_CLOSE events.
+      // - MOM_DONE: SendLongMidiMessageInternal() relies on this event to clean
+      //     up the backing store where a long MIDI message is stored.
+      // - MOM_OPEN: This event is sent the output device is opened. Note that
+      //     this message is called on the caller thread.
+      // - MOM_CLOSE: This event is sent when 1) midiOutClose() is called, or 2)
+      //     the MIDI device becomes unavailable for some reasons, e.g., the
+      //     cable is disconnected. As for the former case, HMIDIOUT will be
+      //     invalidated soon after the callback is finished. As for the later
+      //     case, however, HMIDIOUT continues to be valid until midiOutClose()
+      //     is called.
+      HMIDIOUT midi_handle = kInvalidMidiOutHandle;
+      const MMRESULT result = midiOutOpen(
+          &midi_handle, device_id,
+          reinterpret_cast<DWORD_PTR>(&OnMidiOutEventOnMainlyMultimediaThread),
+          reinterpret_cast<DWORD_PTR>(this), CALLBACK_FUNCTION);
+      DLOG_IF(ERROR, result != MMSYSERR_NOERROR && result != MMSYSERR_ALLOCATED)

[Takashi Toyoshima, 2015/04/06 18:02:38]

Minor cleanup to use DLOG_IF as the first loop for in devices does.

[yukawa, 2015/04/06 18:41:40]

Acknowledged.

+          << "Failed to open output device. "
+          << " id: " << device_id << " message: " << GetOutErrorMessage(result);
+    }
+  }
+
   /////////////////////////////////////////////////////////////////////////////
   // Callbacks on the OS multimedia thread.
   /////////////////////////////////////////////////////////////////////////////
 
-  static void CALLBACK OnMidiInEventOnMultimediaThread(HMIDIIN midi_in_handle,
-                                                       UINT message,
-                                                       DWORD_PTR instance,
-                                                       DWORD_PTR param1,
-                                                       DWORD_PTR param2) {
+  static void CALLBACK
+  OnMidiInEventOnMainlyMultimediaThread(HMIDIIN midi_in_handle,

[Takashi Toyoshima, 2015/04/06 18:02:38]

Name is changed because MIM_OPEN is not called on multimedia thread, but on the
caller thread.

[yukawa, 2015/04/06 18:41:41]

Acknowledged.

+                                        UINT message,
+                                        DWORD_PTR instance,
+                                        DWORD_PTR param1,
+                                        DWORD_PTR param2) {
     MidiServiceWinImpl* self = reinterpret_cast<MidiServiceWinImpl*>(instance);
     if (!self)
       return;
     switch (message) {
       case MIM_OPEN:
-        self->OnMidiInOpenOnMultimediaThread(midi_in_handle);
+        self->OnMidiInOpen(midi_in_handle);
         break;
       case MIM_DATA:
         self->OnMidiInDataOnMultimediaThread(midi_in_handle, param1, param2);
         break;
       case MIM_LONGDATA:
         self->OnMidiInLongDataOnMultimediaThread(midi_in_handle, param1,
                                                  param2);
         break;
       case MIM_CLOSE:
         self->OnMidiInCloseOnMultimediaThread(midi_in_handle);
         break;
     }
   }
 
-  void OnMidiInOpenOnMultimediaThread(HMIDIIN midi_in_handle) {
+  void OnMidiInOpen(HMIDIIN midi_in_handle) {
     UINT device_id = 0;
     MMRESULT result = midiInGetID(midi_in_handle, &device_id);
     if (result != MMSYSERR_NOERROR) {
       DLOG(ERROR) << "midiInGetID failed: " << GetInErrorMessage(result);
       return;
     }
     MIDIINCAPS2W caps = {};
     result = midiInGetDevCaps(device_id, reinterpret_cast<LPMIDIINCAPSW>(&caps),
                               sizeof(caps));
     if (result != MMSYSERR_NOERROR) {
@@ skipping 138 matching lines @@
       unused_input_ports_[device_info].push(port_number);
     }
     task_thread_.message_loop()->PostTask(
         FROM_HERE,
         base::Bind(&MidiServiceWinImpl::SetInputPortStateOnTaskThread,
                    base::Unretained(this), port_number,
                    MIDI_PORT_DISCONNECTED));
   }
 
   static void CALLBACK
-  OnMidiOutEventOnMultimediaThread(HMIDIOUT midi_out_handle,
-                                   UINT message,
-                                   DWORD_PTR instance,
-                                   DWORD_PTR param1,
-                                   DWORD_PTR param2) {
+  OnMidiOutEventOnMainlyMultimediaThread(HMIDIOUT midi_out_handle,
+                                         UINT message,
+                                         DWORD_PTR instance,
+                                         DWORD_PTR param1,
+                                         DWORD_PTR param2) {
     MidiServiceWinImpl* self = reinterpret_cast<MidiServiceWinImpl*>(instance);
     if (!self)
       return;
     switch (message) {
       case MOM_OPEN:
-        self->OnMidiOutOpenOnMultimediaThread(midi_out_handle, param1, param2);
+        self->OnMidiOutOpen(midi_out_handle, param1, param2);
         break;
       case MOM_DONE:
         self->OnMidiOutDoneOnMultimediaThread(midi_out_handle, param1);
         break;
       case MOM_CLOSE:
         self->OnMidiOutCloseOnMultimediaThread(midi_out_handle);
         break;
     }
   }
 
-  void OnMidiOutOpenOnMultimediaThread(HMIDIOUT midi_out_handle,
-                                       DWORD_PTR param1,
-                                       DWORD_PTR param2) {
+  void OnMidiOutOpen(HMIDIOUT midi_out_handle,
+                     DWORD_PTR param1,
+                     DWORD_PTR param2) {
     UINT device_id = 0;
     MMRESULT result = midiOutGetID(midi_out_handle, &device_id);
     if (result != MMSYSERR_NOERROR) {
       DLOG(ERROR) << "midiOutGetID failed: " << GetOutErrorMessage(result);
       return;
     }
     MIDIOUTCAPS2W caps = {};
     result = midiOutGetDevCaps(
         device_id, reinterpret_cast<LPMIDIOUTCAPSW>(&caps), sizeof(caps));
     if (result != MMSYSERR_NOERROR) {
@@ skipping 77 matching lines @@
       state->closed = true;
     }
     task_thread_.message_loop()->PostTask(
         FROM_HERE,
         base::Bind(&MidiServiceWinImpl::SetOutputPortStateOnTaskThread,
                    base::Unretained(this), port_number,
                    MIDI_PORT_DISCONNECTED));
   }
 
   /////////////////////////////////////////////////////////////////////////////
-  // Callbacks on the monitor thread.
-  /////////////////////////////////////////////////////////////////////////////
-
-  void AssertOnMonitorThread() {
-    DCHECK_EQ(monitor_thread_.thread_id(), base::PlatformThread::CurrentId());
-  }
-
-  void InitializeOnMonitorThread() {
-    AssertOnMonitorThread();
-    // Synchronously open all the existing MIDI devices.
-    TryOpenAllDevicesOnMonitorThreadSync();
-    // Since |TryOpenAllDevicesOnMonitorThreadSync()| is a blocking call, it is
-    // guaranteed that all the initial MIDI ports are already opened here, and
-    // corresponding tasks to call |AddInputPortOnTaskThread()| and
-    // |AddOutputPortOnTaskThread()| are already queued in the Misc. Task
-    // Thread.  This means that the following task to call
-    // |CompleteInitializationOnTaskThread()| are always called after all the
-    // pending tasks to call |AddInputPortOnTaskThread()| and
-    // |AddOutputPortOnTaskThread()| are completed.
-    task_thread_.message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&MidiServiceWinImpl::CompleteInitializationOnTaskThread,
-                   base::Unretained(this), MIDI_OK));
-    // Start monitoring based on notifications generated by
-    // RegisterDeviceNotificationW API.
-    device_monitor_window_.reset(new DeviceMonitorWindow(
-        base::Bind(&MidiServiceWinImpl::TryOpenAllDevicesOnMonitorThreadSync,
-                   base::Unretained(this))));
-  }
-
-  void StopDeviceMonitorOnMonitorThreadSync() {
-    device_monitor_window_.reset();
-  }
-
-  // Note that this is a blocking method.
-  void TryOpenAllDevicesOnMonitorThreadSync() {
-    AssertOnMonitorThread();
-    const UINT num_in_devices = midiInGetNumDevs();
-    for (UINT device_id = 0; device_id < num_in_devices; ++device_id) {
-      // Here we use |CALLBACK_FUNCTION| to subscribe MIM_DATA, MIM_LONGDATA,
-      // MIM_OPEN, and MIM_CLOSE events.
-      // - MIM_DATA: This is the only way to get a short MIDI message with
-      //     timestamp information.
-      // - MIM_LONGDATA: This is the only way to get a long MIDI message with
-      //     timestamp information.
-      // - MIM_OPEN: This event is sent the input device is opened.
-      // - MIM_CLOSE: This event is sent when 1) midiInClose() is called, or 2)
-      //     the MIDI device becomes unavailable for some reasons, e.g., the
-      //     cable is disconnected. As for the former case, HMIDIOUT will be
-      //     invalidated soon after the callback is finished. As for the later
-      //     case, however, HMIDIOUT continues to be valid until midiInClose()
-      //     is called.
-      HMIDIIN midi_handle = kInvalidMidiInHandle;
-      const MMRESULT result = midiInOpen(
-          &midi_handle, device_id,
-          reinterpret_cast<DWORD_PTR>(&OnMidiInEventOnMultimediaThread),
-          reinterpret_cast<DWORD_PTR>(this), CALLBACK_FUNCTION);
-      DLOG_IF(ERROR, result != MMSYSERR_NOERROR && result != MMSYSERR_ALLOCATED)
-          << "Failed to open output device. "
-          << " id: " << device_id << " message: " << GetInErrorMessage(result);
-    }
-
-    const UINT num_out_devices = midiOutGetNumDevs();
-    for (UINT device_id = 0; device_id < num_out_devices; ++device_id) {
-      // Here we use |CALLBACK_FUNCTION| to subscribe MOM_DONE, MOM_OPEN, and
-      // MOM_CLOSE events.
-      // - MOM_DONE: SendLongMidiMessageInternal() relies on this event to clean
-      //     up the backing store where a long MIDI message is stored.
-      // - MOM_OPEN: This event is sent the output device is opened.
-      // - MOM_CLOSE: This event is sent when 1) midiOutClose() is called, or 2)
-      //     the MIDI device becomes unavailable for some reasons, e.g., the
-      //     cable is disconnected. As for the former case, HMIDIOUT will be
-      //     invalidated soon after the callback is finished. As for the later
-      //     case, however, HMIDIOUT continues to be valid until midiOutClose()
-      //     is called.
-      HMIDIOUT midi_handle = kInvalidMidiOutHandle;
-      const MMRESULT result = midiOutOpen(
-          &midi_handle, device_id,
-          reinterpret_cast<DWORD_PTR>(&OnMidiOutEventOnMultimediaThread),
-          reinterpret_cast<DWORD_PTR>(this), CALLBACK_FUNCTION);
-      if (result != MMSYSERR_NOERROR && result != MMSYSERR_ALLOCATED) {
-        DLOG(ERROR) << "Failed to open output device. "
-                    << " id: " << device_id
-                    << " message: " << GetOutErrorMessage(result);
-      }
-    }
-  }
-
-  /////////////////////////////////////////////////////////////////////////////
   // Callbacks on the sender thread.
   /////////////////////////////////////////////////////////////////////////////
 
   void AssertOnSenderThread() {
     DCHECK_EQ(sender_thread_.thread_id(), base::PlatformThread::CurrentId());
   }
 
   void SendOnSenderThread(uint32 port_number,
                           uint64 port_age,
                           const std::vector<uint8>& data,
@@ skipping 43 matching lines @@
   }
 
   /////////////////////////////////////////////////////////////////////////////
   // Callbacks on the task thread.
   /////////////////////////////////////////////////////////////////////////////
 
   void AssertOnTaskThread() {
     DCHECK_EQ(task_thread_.thread_id(), base::PlatformThread::CurrentId());
   }
 
+

[yukawa, 2015/04/06 18:41:41]

Is this blank line intentional?

[Takashi Toyoshima, 2015/04/06 19:51:47]

oops, that's a mistake.

   void StartInputDeviceOnTaskThread(HMIDIIN midi_in_handle) {
     AssertOnTaskThread();
     auto state = GetInputDeviceFromHandle(midi_in_handle);
     if (!state)
       return;
     MMRESULT result =
         midiInPrepareHeader(state->midi_handle, state->midi_header.get(),
                             sizeof(*state->midi_header));
     if (result != MMSYSERR_NOERROR) {
       DLOG(ERROR) << "Failed to initialize input buffer: "
@@ skipping 49 matching lines @@
     delegate_->OnSetOutputPortState(port_index, state);
   }
 
   /////////////////////////////////////////////////////////////////////////////
   // Fields:
   /////////////////////////////////////////////////////////////////////////////
 
   // Does not take ownership.
   MidiServiceWinDelegate* delegate_;
 
-  base::Thread monitor_thread_;
   base::Thread sender_thread_;
   base::Thread task_thread_;
 
-  scoped_ptr<DeviceMonitorWindow> device_monitor_window_;
-
   base::Lock input_ports_lock_;
   base::hash_map<HMIDIIN, scoped_refptr<MidiInputDeviceState>>
       input_device_map_;                // GUARDED_BY(input_ports_lock_)
   PortNumberCache unused_input_ports_;  // GUARDED_BY(input_ports_lock_)
   std::vector<scoped_refptr<MidiInputDeviceState>>
       input_ports_;                       // GUARDED_BY(input_ports_lock_)
   std::vector<uint64> input_ports_ages_;  // GUARDED_BY(input_ports_lock_)
 
   base::Lock output_ports_lock_;
   base::hash_map<HMIDIOUT, scoped_refptr<MidiOutputDeviceState>>
       output_device_map_;                // GUARDED_BY(output_ports_lock_)
   PortNumberCache unused_output_ports_;  // GUARDED_BY(output_ports_lock_)
   std::vector<scoped_refptr<MidiOutputDeviceState>>
       output_ports_;                       // GUARDED_BY(output_ports_lock_)
   std::vector<uint64> output_ports_ages_;  // GUARDED_BY(output_ports_lock_)
 
   // True if one thread reached MidiServiceWinImpl::~MidiServiceWinImpl(). Note
   // that MidiServiceWinImpl::~MidiServiceWinImpl() is blocked until
-  // |monitor_thread_|, |sender_thread_|, and |task_thread_| are stopped.
+  // |sender_thread_|, and |task_thread_| are stopped.
   // This flag can be used as the signal that when background tasks must be
   // interrupted.
   // TODO(toyoshim): Use std::atomic<bool> when it is allowed.
   volatile bool destructor_started;
 
   DISALLOW_COPY_AND_ASSIGN(MidiServiceWinImpl);
 };
 
 }  // namespace
 
@@ skipping 21 matching lines @@
   // TOOD(toyoshim): This calculation should be done when the date is actually
   // sent.
   client->AccumulateMidiBytesSent(data.size());
 }
 
 MidiManager* MidiManager::Create() {
   return new MidiManagerWin();
 }
 
 }  // namespace media