Web MIDI backend for Windows 10

media/midi/midi_manager_winrt.cc

Index: media/midi/midi_manager_winrt.cc
diff --git a/media/midi/midi_manager_winrt.cc b/media/midi/midi_manager_winrt.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d2a8dc973b5f46fae34c68b8821025d0822aace2
--- /dev/null
+++ b/media/midi/midi_manager_winrt.cc
@@ -0,0 +1,780 @@
+// Copyright 2016 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_winrt.h"
+
+#include <robuffer.h>
+#include <windows.devices.enumeration.h>
+#include <windows.devices.midi.h>
+#include <wrl/event.h>
+
+#include <iomanip>
+
+#include "base/bind.h"
+#include "base/containers/hash_tables.h"

[Shao-Chuan Lee, 2016/08/24 09:40:30]

Deprecated (https://crbug.com/576864). base::hash_map is just an alias of
std::unordered_map now, will replace all usages.

[Takashi Toyoshima, 2016/08/25 06:09:45]

Can you add TODO for record?

[Shao-Chuan Lee, 2016/08/25 07:06:42]

Will replace in next patch set.

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Done.

+#include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_checker.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/timer/timer.h"
+#include "base/win/scoped_comptr.h"
+#include "base/win/windows_version.h"
+#include "media/midi/midi_scheduler.h"
+
+namespace media {
+namespace midi {
+namespace {
+
+namespace WRL = Microsoft::WRL;
+
+using namespace ABI::Windows::Devices::Enumeration;
+using namespace ABI::Windows::Devices::Midi;
+using namespace ABI::Windows::Foundation;
+using namespace ABI::Windows::Storage::Streams;
+
+using base::win::ScopedComPtr;
+
+// Helpers for printing HRESULTs.

[Takashi Toyoshima, 2016/08/25 06:09:44]

Oh, we do not have this kind of utilities under base/? That's bad.

In this simple code, we just dump the HRESULT as a hex code, right?
If it's enough, we can just cast to integer to redirect it to LOG().

Otherwise, we may want to have more graceful messages by using HRESULT_CODE()
and FormatMessage.

Or, base::SystemErrorCodeToString() in base/logging is the one we want to use
here?

[Shao-Chuan Lee, 2016/08/25 07:06:42]

base::SystemErrorCodeToString() is using error codes from Win32 APIs, here we
are using COM-style HRESULTs with WinRT. I'll look into using FormatMessage() or
_com_error (https://msdn.microsoft.com/en-us/library/0ye3k36s.aspx) for
retrieving error messages. Printing error codes in 32-bit hex since it's common
in documents.

[Shao-Chuan Lee, 2016/08/29 08:27:52]

Now using _com_error(hr).ErrorMessage(), printing something like:

"CoInitialize has not been called. (0x800401F0)"

+struct PrintHr {
+  PrintHr(HRESULT hr) : hr_(hr) {}
+  HRESULT hr_;
+};
+
+std::ostream& operator<<(std::ostream& os, const PrintHr& phr) {
+  std::ios_base::fmtflags ff = os.flags();
+  os << "HRESULT 0x" << std::hex << std::uppercase << std::setfill('0')
+     << std::setw(8) << phr.hr_;
+  os.flags(ff);
+  return os;
+}
+
+// Factory functions that activate and create WinRT components. The caller takes
+// ownership of the returning ComPtr.
+template <typename InterfaceType, base::char16 const* runtime_class_id>
+ScopedComPtr<InterfaceType> WrlStaticsFactory() {
+  ScopedComPtr<InterfaceType> com_ptr;
+
+  HRESULT hr = GetActivationFactory(
+      WRL::Wrappers::HStringReference(runtime_class_id).Get(),
+      com_ptr.Receive());
+  if (FAILED(hr)) {
+    VLOG(1) << "GetActivationFactory failed: " << PrintHr(hr);
+    com_ptr = nullptr;
+  }
+
+  return com_ptr;
+}
+
+ScopedComPtr<IBufferFactory> GetBufferFactory() {

[Takashi Toyoshima, 2016/08/25 06:09:44]

Since now we have only one caller for GetBufferFactory() and
GetDeviceInformationStatics(), I'm not sure it's useful to have these method.

[Shao-Chuan Lee, 2016/08/25 07:06:42]

Will replace usages with WrlStaticsFactory().

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Done.

+  return WrlStaticsFactory<IBufferFactory,
+                           RuntimeClass_Windows_Storage_Streams_Buffer>();
+}
+
+ScopedComPtr<IDeviceInformationStatics> GetDeviceInformationStatics() {
+  return WrlStaticsFactory<
+      IDeviceInformationStatics,
+      RuntimeClass_Windows_Devices_Enumeration_DeviceInformation>();
+}
+
+template <typename T, HRESULT (T::*method)(HSTRING*)>
+std::string GetStringFromObjectMethod(T* obj) {
+  HSTRING result;
+  HRESULT hr = (obj->*method)(&result);
+  if (FAILED(hr)) {
+    VLOG(1) << "GetStringFromObjectMethod failed: " << PrintHr(hr);
+    return std::string();
+  }
+
+  // Note: empty HSTRINGs are represent as nullptr, and instantiating
+  // std::string with nullptr (in base::WideToUTF8) is undefined behavior.
+  const base::char16* buffer = WindowsGetStringRawBuffer(result, nullptr);
+  if (buffer)
+    return base::WideToUTF8(buffer);
+  return std::string();
+}
+
+template <typename T>
+std::string GetIdString(T* obj) {
+  return GetStringFromObjectMethod<T, &T::get_Id>(obj);
+}
+
+template <typename T>
+std::string GetDeviceIdString(T* obj) {
+  return GetStringFromObjectMethod<T, &T::get_DeviceId>(obj);
+}
+
+std::string GetNameString(IDeviceInformation* info) {
+  return GetStringFromObjectMethod<IDeviceInformation,
+                                   &IDeviceInformation::get_Name>(info);
+}
+
+HRESULT GetPointerToBufferData(IBuffer* buffer, uint8_t** out) {

[Takashi Toyoshima, 2016/08/25 06:09:45]

Why do you change this interface?
HRESULT isn't used effectively outside this function. So just returning nullptr
for error cases looks reasonable as you did before.

[Shao-Chuan Lee, 2016/08/25 07:06:42]

Since this is used in WRL callback which should return HRESULTs, I'd like to
return the HRESULT here if it fails (line 556).

Actually I tried returning errors in WRL callbacks but there seems no
difference, so maybe this is unnecessary.

[Takashi Toyoshima, 2016/08/29 09:48:12]

Aha, I understand the reason of this change.

Since you already handle errors properly, it's probably ok to return S_OK from
WRL callbacks. Otherwise, callers may do something unpredictable to recover?

+  ScopedComPtr<Windows::Storage::Streams::IBufferByteAccess> buffer_byte_access;
+
+  HRESULT hr = buffer_byte_access.QueryFrom(buffer);
+  if (FAILED(hr)) {
+    VLOG(1) << "QueryInterface failed: " << PrintHr(hr);
+    return hr;
+  }
+
+  // Lifetime of the pointing buffer is controlled by the buffer object.
+  hr = buffer_byte_access->Buffer(out);
+  if (FAILED(hr)) {
+    VLOG(1) << "Buffer failed: " << PrintHr(hr);
+    return hr;
+  }
+
+  return S_OK;
+}
+
+template <typename InterfaceType>
+struct MidiPort {
+  MidiPort() = default;
+
+  uint32_t index;
+  ScopedComPtr<InterfaceType> handle;
+  base::TimeTicks start_time;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MidiPort);
+};
+
+}  // namespace
+
+template <typename InterfaceType,
+          typename RuntimeType,
+          typename StaticsInterfaceType,
+          base::char16 const* runtime_class_id>
+class MidiManagerWinrt::MidiPortManager {
+ public:
+  // MidiPortManager instances should be constructed on the COM thread.
+  MidiPortManager(MidiManagerWinrt* midi_manager)
+      : midi_manager_(midi_manager),
+        task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
+
+  bool StartWatcher() {
+    DCHECK(thread_checker_.CalledOnValidThread());
+
+    HRESULT hr;
+
+    midi_port_statics_ =
+        WrlStaticsFactory<StaticsInterfaceType, runtime_class_id>();
+    if (!midi_port_statics_)
+      return false;
+
+    HSTRING device_selector = nullptr;
+    hr = midi_port_statics_->GetDeviceSelector(&device_selector);
+    if (FAILED(hr)) {
+      VLOG(1) << "GetDeviceSelector failed: " << PrintHr(hr);
+      return false;
+    }
+
+    auto dev_info_statics = GetDeviceInformationStatics();
+    if (!dev_info_statics)
+      return false;
+
+    hr = dev_info_statics->CreateWatcherAqsFilter(device_selector,
+                                                  watcher_.Receive());
+    if (FAILED(hr)) {
+      VLOG(1) << "CreateWatcherAqsFilter failed: " << PrintHr(hr);
+      return false;
+    }
+
+    // Register callbacks to WinRT that post state-modifying jobs back to COM
+    // thread. |weak_ptr| and |task_runner| are captured by lambda callbacks for
+    // posting jobs. Note that WinRT callback arguments should not be passed
+    // outside the callback since the pointers may be unavailable afterwards.
+    base::WeakPtr<MidiPortManager> weak_ptr = GetWeakPtrFromFactory();
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner = task_runner_;
+
+    hr = watcher_->add_Added(
+        WRL::Callback<ITypedEventHandler<DeviceWatcher*, DeviceInformation*>>(
+            [weak_ptr, task_runner](IDeviceWatcher* watcher,
+                                    IDeviceInformation* info) {
+              std::string dev_id = GetIdString(info),
+                          dev_name = GetNameString(info);
+
+              task_runner->PostTask(
+                  FROM_HERE, base::Bind(&MidiPortManager::OnAdded, weak_ptr,
+                                        dev_id, dev_name));
+
+              return S_OK;
+            })
+            .Get(),
+        &token_Added_);
+    if (FAILED(hr)) {
+      VLOG(1) << "add_Added failed: " << PrintHr(hr);
+      return false;
+    }
+
+    hr = watcher_->add_EnumerationCompleted(
+        WRL::Callback<ITypedEventHandler<DeviceWatcher*, IInspectable*>>(
+            [weak_ptr, task_runner](IDeviceWatcher* watcher,
+                                    IInspectable* insp) {
+              task_runner->PostTask(
+                  FROM_HERE,
+                  base::Bind(&MidiPortManager::OnEnumerationCompleted,
+                             weak_ptr));
+
+              return S_OK;
+            })
+            .Get(),
+        &token_EnumerationCompleted_);
+    if (FAILED(hr)) {
+      VLOG(1) << "add_EnumerationCompleted failed: " << PrintHr(hr);
+      return false;
+    }
+
+    hr = watcher_->add_Removed(
+        WRL::Callback<
+            ITypedEventHandler<DeviceWatcher*, DeviceInformationUpdate*>>(
+            [weak_ptr, task_runner](IDeviceWatcher* watcher,
+                                    IDeviceInformationUpdate* update) {
+              std::string dev_id = GetIdString(update);
+
+              task_runner->PostTask(
+                  FROM_HERE,
+                  base::Bind(&MidiPortManager::OnRemoved, weak_ptr, dev_id));
+
+              return S_OK;
+            })
+            .Get(),
+        &token_Removed_);
+    if (FAILED(hr)) {
+      VLOG(1) << "add_Removed failed: " << PrintHr(hr);
+      return false;
+    }
+
+    hr = watcher_->add_Stopped(
+        WRL::Callback<ITypedEventHandler<DeviceWatcher*, IInspectable*>>(
+            [](IDeviceWatcher* watcher, IInspectable* insp) {
+              // Placeholder, does nothing for now.
+              return S_OK;
+            })
+            .Get(),
+        &token_Stopped_);
+    if (FAILED(hr)) {
+      VLOG(1) << "add_Stopped failed: " << PrintHr(hr);
+      return false;
+    }
+
+    hr = watcher_->add_Updated(
+        WRL::Callback<
+            ITypedEventHandler<DeviceWatcher*, DeviceInformationUpdate*>>(
+            [](IDeviceWatcher* watcher, IDeviceInformationUpdate* update) {
+              // TODO(shaochuan): Check for fields to be updated here.
+              return S_OK;
+            })
+            .Get(),
+        &token_Updated_);
+    if (FAILED(hr)) {
+      VLOG(1) << "add_Updated failed: " << PrintHr(hr);
+      return false;
+    }
+
+    hr = watcher_->Start();
+    if (FAILED(hr)) {
+      VLOG(1) << "Start failed: " << PrintHr(hr);
+      return false;
+    }
+
+    is_initialized_ = true;
+    return true;
+  }
+
+  ~MidiPortManager() {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!is_initialized_)

[Takashi Toyoshima, 2016/08/25 06:43:08]

When initialization fails, some callbacks could be registered, and the others
couldn't.

Can you use tokens to decide if we need to remove a callback for each?

Probably, we can use is_initialized_ for Stop().

[Shao-Chuan Lee, 2016/08/25 07:06:42]

I'm aware of this but I couldn't find definitions for an invalid
EventRegistrationToken (a INT64 wrapper). I wonder if it's safe to call all
remove_*'s and just ignore errors, otherwise we need another field to keep track
of which callbacks are registered successfully.

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Found that tokens with value = 0 is considered invalid in <wrl/event.h>. Now
initializing tokens to 0 and add checks before removing.

+      return;
+
+    watcher_->remove_Added(token_Added_);
+    watcher_->remove_EnumerationCompleted(token_EnumerationCompleted_);
+    watcher_->remove_Removed(token_Removed_);
+    watcher_->remove_Stopped(token_Stopped_);
+    watcher_->remove_Updated(token_Updated_);
+
+    watcher_->Stop();
+  }
+
+  MidiPort<InterfaceType>* GetPortByDeviceId(std::string dev_id) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!is_initialized_)

[Takashi Toyoshima, 2016/08/25 06:43:08]

When does this happen? If only on bugs, CHECK() is fine.

[Shao-Chuan Lee, 2016/08/29 08:27:52]

Using CHECK() now.

+      return nullptr;
+
+    auto it = ports_.find(dev_id);
+    if (it == ports_.end())
+      return nullptr;
+    return it->second.get();
+  }
+
+  MidiPort<InterfaceType>* GetPortByIndex(uint32_t port_index) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!is_initialized_)

[Takashi Toyoshima, 2016/08/25 06:43:08]

ditto

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Done.

+      return nullptr;
+
+    auto it = ports_.find(port_ids_[port_index]);
+    if (it == ports_.end())
+      return nullptr;
+    return it->second.get();
+  }
+
+ protected:
+  // Points to the MidiManagerWinrt instance, which is expected to outlive the
+  // MidiPortManager instance.
+  MidiManagerWinrt* midi_manager_;
+
+  // Task runner of the COM thread.
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
+  // Ensures all methods are called on the COM thread.
+  base::ThreadChecker thread_checker_;
+
+ private:
+  // DeviceWatcher callbacks:
+  void OnAdded(std::string dev_id, std::string dev_name) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!is_initialized_)

[Takashi Toyoshima, 2016/08/25 06:43:08]

ditto

[Shao-Chuan Lee, 2016/08/25 07:06:42]

Callbacks may be successfully registered and initialization fails later.
Otherwise we should unsubscribe registered events immediately after failure in
StartWatcher().

[Shao-Chuan Lee, 2016/08/29 08:27:52]

Done.

+      return;
+
+    // TODO(shaochuan): Disable Microsoft GS Wavetable Synth due to security
+    // reasons. http://crbug.com/499279
+
+    port_names_[dev_id] = dev_name;
+
+    base::string16 dev_id_string16 = base::UTF8ToWide(dev_id);
+    HSTRING dev_id_hstring;
+    HRESULT hr = WindowsCreateString(
+        dev_id_string16.c_str(), static_cast<UINT32>(dev_id_string16.length()),
+        &dev_id_hstring);
+    if (FAILED(hr)) {
+      VLOG(1) << "WindowsCreateString failed: " << PrintHr(hr);
+      return;
+    }
+
+    IAsyncOperation<RuntimeType*>* async_op;
+
+    hr = midi_port_statics_->FromIdAsync(dev_id_hstring, &async_op);
+    if (FAILED(hr)) {
+      VLOG(1) << "FromIdAsync failed: " << PrintHr(hr);
+      return;
+    }
+
+    WindowsDeleteString(dev_id_hstring);  // Always returns S_OK.

[Takashi Toyoshima, 2016/08/25 06:43:08]

Oh, should we manage HSTRING with ScopedHandle-ish (base/win/scoped_handle)
class?

Removing manual handle management is highly recommended. In this case, you
actually leak the object on returning for errors above.

[Shao-Chuan Lee, 2016/08/25 07:06:42]

I can write a wrapper class for this.

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Now using Microsoft::WRL::Wrappers::HString which provides this functionality.

[Takashi Toyoshima, 2016/08/29 09:48:12]

Acknowledged.

+    dev_id_hstring = nullptr;
+
+    base::WeakPtr<MidiPortManager> weak_ptr = GetWeakPtrFromFactory();
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner = task_runner_;
+
+    hr = async_op->put_Completed(
+        WRL::Callback<IAsyncOperationCompletedHandler<RuntimeType*>>(
+            [weak_ptr, task_runner](IAsyncOperation<RuntimeType*>* async_op,
+                                    AsyncStatus status) {
+              // TODO(shaochuan): Check if port open time is accurate.
+              const auto now = base::TimeTicks::Now();
+
+              InterfaceType* handle;
+              HRESULT hr = async_op->GetResults(&handle);
+              if (FAILED(hr)) {
+                VLOG(1) << "GetResults failed: " << PrintHr(hr);
+                return hr;
+              }
+
+              // A reference to |async_op| is kept in |async_ops_|, safe to pass
+              // outside.
+              task_runner->PostTask(
+                  FROM_HERE,
+                  base::Bind(&MidiPortManager::OnCompletedGetPortFromIdAsync,
+                             weak_ptr, handle, now, async_op));
+
+              return S_OK;
+            })
+            .Get());
+    if (FAILED(hr)) {
+      VLOG(1) << "put_Completed failed: " << PrintHr(hr);
+      return;
+    }
+
+    // Keep a reference to incompleted |async_op| for releasing later.
+    async_ops_.insert(async_op);
+  }
+
+  void OnEnumerationCompleted() {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!is_initialized_)

[Takashi Toyoshima, 2016/08/25 06:43:08]

ditto

[Shao-Chuan Lee, 2016/08/29 08:27:52]

Done.

+      return;
+
+    if (async_ops_.empty())
+      midi_manager_->OnPortManagerReady();
+    else
+      enumeration_completed_not_ready_ = true;
+  }
+
+  void OnRemoved(std::string dev_id) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!is_initialized_)

[Takashi Toyoshima, 2016/08/25 06:43:08]

ditto

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Done.

+      return;
+
+    MidiPort<InterfaceType>* port = GetPortByDeviceId(dev_id);
+    if (!port) {
+      VLOG(1) << "Removing non-existent port " << dev_id;

[Takashi Toyoshima, 2016/08/25 06:43:08]

Shall we return here? Otherwise, following dereferences will crash.
Or CHECK() as you do for OnMessageReceived?

[Shao-Chuan Lee, 2016/08/25 07:06:42]

Return is missing, will fix.

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Done.

+    }
+
+    SetPortState(port->index, MIDI_PORT_DISCONNECTED);
+
+    port->handle = nullptr;
+  }
+
+  void OnCompletedGetPortFromIdAsync(InterfaceType* handle,
+                                     base::TimeTicks start_time,
+                                     IAsyncOperation<RuntimeType*>* async_op) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!is_initialized_)

[Takashi Toyoshima, 2016/08/25 06:43:08]

ditto

[Shao-Chuan Lee, 2016/08/29 08:27:52]

Done.

+      return;
+
+    RegisterOnMessageReceived(handle);
+
+    std::string dev_id = GetDeviceIdString(handle);
+
+    MidiPort<InterfaceType>* port = GetPortByDeviceId(dev_id);
+
+    if (port == nullptr) {
+      // TODO(shaochuan): Fill in manufacturer and driver version.
+      AddPort(MidiPortInfo(dev_id, std::string("Manufacturer"),
+                           port_names_[dev_id], std::string("DriverVersion"),
+                           MIDI_PORT_OPENED));
+
+      port = new MidiPort<InterfaceType>;
+      port->index = static_cast<uint32_t>(port_ids_.size());
+
+      ports_[dev_id].reset(port);
+      port_ids_.push_back(dev_id);
+    } else {
+      SetPortState(port->index, MIDI_PORT_CONNECTED);
+    }
+
+    port->handle = handle;
+    port->start_time = start_time;
+
+    // Manually release COM interface to completed |async_op|.
+    auto it = async_ops_.find(async_op);
+    CHECK(it != async_ops_.end());
+    (*it)->Release();
+    async_ops_.erase(it);
+
+    if (enumeration_completed_not_ready_ && async_ops_.empty()) {
+      midi_manager_->OnPortManagerReady();
+      enumeration_completed_not_ready_ = false;
+    }
+  }
+
+  // Overrided by MidiInPortManager to listen to input ports.
+  virtual void RegisterOnMessageReceived(InterfaceType* handle) {}
+
+  // Calls midi_manager_->Add{Input,Output}Port.
+  virtual void AddPort(MidiPortInfo info) = 0;
+
+  // Calls midi_manager_->Set{Input,Output}PortState.
+  virtual void SetPortState(uint32_t port_index, MidiPortState state) = 0;
+
+  // WeakPtrFactory has to be declared in derived class, use this method to
+  // retrieve upcasted WeakPtr for posting tasks.
+  virtual base::WeakPtr<MidiPortManager> GetWeakPtrFromFactory() = 0;
+
+  // Midi{In,Out}PortStatics instance.
+  ScopedComPtr<StaticsInterfaceType> midi_port_statics_;
+
+  // DeviceWatcher instance and event registration tokens for unsubscribing
+  // events in destructor.
+  ScopedComPtr<IDeviceWatcher> watcher_;
+  EventRegistrationToken token_Added_, token_EnumerationCompleted_,
+      token_Removed_, token_Stopped_, token_Updated_;
+
+  // All manipulations to these fields should be done on COM thread.
+  base::hash_map<std::string, std::unique_ptr<MidiPort<InterfaceType>>> ports_;
+  std::vector<std::string> port_ids_;
+  base::hash_map<std::string, std::string> port_names_;
+
+  // Keeps AsyncOperation references before the operation completes. Note that
+  // raw pointers are used here and the COM interfaces should be released
+  // manually.
+  std::set<IAsyncOperation<RuntimeType*>*> async_ops_;

[Shao-Chuan Lee, 2016/08/24 09:40:30]

Now using raw pointers and is hashable, may use std::unordered_set now.

Is memory usage a concern? If so, std::list should be a good alternative as
|async_op|s seems likely, though not guaranteed, to complete in the order they
were issued.

+
+  // Set when device enumeration is completed but OnPortManagerReady() is not
+  // called since some ports are not yet ready (i.e. |async_ops_| is not empty).
+  // In such cases, OnPortManagerReady() will be called in
+  // OnCompletedGetPortFromIdAsync() when the last pending port is ready.
+  bool enumeration_completed_not_ready_ = false;
+
+  // Set if the instance is initialized without error. Should be checked in all
+  // methods on COM thread except StartWatcher().
+  bool is_initialized_ = false;
+};
+
+class MidiManagerWinrt::MidiInPortManager final
+    : public MidiPortManager<IMidiInPort,
+                             MidiInPort,
+                             IMidiInPortStatics,
+                             RuntimeClass_Windows_Devices_Midi_MidiInPort> {
+ public:
+  MidiInPortManager(MidiManagerWinrt* midi_manager)
+      : MidiPortManager(midi_manager), weak_factory_(this) {}
+
+ private:
+  // MidiPortManager overrides:
+  void RegisterOnMessageReceived(IMidiInPort* handle) override {
+    DCHECK(thread_checker_.CalledOnValidThread());
+
+    EventRegistrationToken& token = tokens_[GetDeviceIdString(handle)];
+
+    base::WeakPtr<MidiInPortManager> weak_ptr = weak_factory_.GetWeakPtr();
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner = task_runner_;
+
+    handle->add_MessageReceived(

[Shao-Chuan Lee, 2016/08/25 02:50:16]

Add error handling here, this MidiInPort should not be available if event
registration fails.

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Done.

+        WRL::Callback<
+            ITypedEventHandler<MidiInPort*, MidiMessageReceivedEventArgs*>>(
+            [weak_ptr, task_runner](IMidiInPort* handle,
+                                    IMidiMessageReceivedEventArgs* args) {
+              std::string dev_id = GetDeviceIdString(handle);
+
+              ScopedComPtr<IMidiMessage> message;
+              HRESULT hr = args->get_Message(message.Receive());
+              if (FAILED(hr)) {
+                VLOG(1) << "get_Message failed: " << PrintHr(hr);
+                return hr;
+              }
+
+              ScopedComPtr<IBuffer> buffer;
+              hr = message->get_RawData(buffer.Receive());
+              if (FAILED(hr)) {
+                VLOG(1) << "get_RawData failed: " << PrintHr(hr);
+                return hr;
+              }
+
+              uint8_t* p_buffer_data = nullptr;
+              hr = GetPointerToBufferData(buffer.get(), &p_buffer_data);
+              if (FAILED(hr))
+                return hr;
+
+              uint32_t data_length = 0;
+              hr = buffer->get_Length(&data_length);
+              if (FAILED(hr)) {
+                VLOG(1) << "get_Length failed: " << PrintHr(hr);
+                return hr;
+              }
+
+              std::vector<uint8_t> data(p_buffer_data,
+                                        p_buffer_data + data_length);
+
+              // Time since port opened in 100-nanosecond units.
+              TimeSpan time_span;
+              hr = message->get_Timestamp(&time_span);
+              if (FAILED(hr)) {
+                VLOG(1) << "get_Timestamp failed: " << PrintHr(hr);
+                return hr;
+              }
+
+              task_runner->PostTask(
+                  FROM_HERE,
+                  base::Bind(&MidiInPortManager::OnMessageReceived, weak_ptr,
+                             dev_id, data, base::TimeDelta::FromMicroseconds(
+                                               time_span.Duration / 10)));
+
+              return S_OK;
+            })
+            .Get(),
+        &token);
+  }
+
+  void AddPort(MidiPortInfo info) final { midi_manager_->AddInputPort(info); }
+
+  void SetPortState(uint32_t port_index, MidiPortState state) final {
+    midi_manager_->SetInputPortState(port_index, state);
+  }
+
+  base::WeakPtr<MidiPortManager> GetWeakPtrFromFactory() final {
+    DCHECK(thread_checker_.CalledOnValidThread());
+
+    return weak_factory_.GetWeakPtr();
+  }
+
+  // Callback on receiving MIDI input message.
+  void OnMessageReceived(std::string dev_id,
+                         std::vector<uint8_t> data,
+                         base::TimeDelta time_since_start) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+
+    MidiPort<IMidiInPort>* port = GetPortByDeviceId(dev_id);
+    CHECK(port);
+
+    midi_manager_->ReceiveMidiData(port->index, &data[0], data.size(),
+                                   port->start_time + time_since_start);
+  }
+
+  // Event tokens for input message received events.
+  base::hash_map<std::string, EventRegistrationToken> tokens_;
+
+  // Last member to ensure destructed first.
+  base::WeakPtrFactory<MidiInPortManager> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(MidiInPortManager);
+};
+
+class MidiManagerWinrt::MidiOutPortManager final
+    : public MidiPortManager<IMidiOutPort,
+                             IMidiOutPort,
+                             IMidiOutPortStatics,
+                             RuntimeClass_Windows_Devices_Midi_MidiOutPort> {
+ public:
+  MidiOutPortManager(MidiManagerWinrt* midi_manager)
+      : MidiPortManager(midi_manager), weak_factory_(this) {}
+
+ private:
+  // MidiPortManager overrides:
+  void AddPort(MidiPortInfo info) final { midi_manager_->AddOutputPort(info); }
+
+  void SetPortState(uint32_t port_index, MidiPortState state) final {
+    midi_manager_->SetOutputPortState(port_index, state);
+  }
+
+  base::WeakPtr<MidiPortManager> GetWeakPtrFromFactory() final {
+    DCHECK(thread_checker_.CalledOnValidThread());
+
+    return weak_factory_.GetWeakPtr();
+  }
+
+  // Last member to ensure destructed first.
+  base::WeakPtrFactory<MidiOutPortManager> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(MidiOutPortManager);
+};
+
+MidiManagerWinrt::MidiManagerWinrt() : com_thread_("Windows MIDI COM Thread") {}
+
+MidiManagerWinrt::~MidiManagerWinrt() {
+  base::AutoLock auto_lock(lazy_init_member_lock_);
+
+  CHECK(!com_thread_checker_);
+  CHECK(!port_manager_in_);
+  CHECK(!port_manager_out_);
+  CHECK(!scheduler_);
+}
+
+void MidiManagerWinrt::StartInitialization() {
+  DCHECK(base::win::GetVersion() >= base::win::VERSION_WIN10);
+
+  com_thread_.init_com_with_mta(true);
+  com_thread_.Start();
+
+  com_thread_.task_runner()->PostTask(
+      FROM_HERE, base::Bind(&MidiManagerWinrt::InitializeOnComThread,
+                            base::Unretained(this)));
+}
+
+void MidiManagerWinrt::Finalize() {
+  com_thread_.task_runner()->PostTask(
+      FROM_HERE, base::Bind(&MidiManagerWinrt::FinalizeOnComThread,
+                            base::Unretained(this)));
+
+  // Blocks until FinalizeOnComThread() returns. Delayed MIDI send data tasks
+  // will be ignored.
+  com_thread_.Stop();
+}
+
+void MidiManagerWinrt::DispatchSendMidiData(MidiManagerClient* client,
+                                            uint32_t port_index,
+                                            const std::vector<uint8_t>& data,
+                                            double timestamp) {
+  base::AutoLock auto_lock(lazy_init_member_lock_);
+
+  if (!scheduler_)

[Takashi Toyoshima, 2016/08/25 06:09:45]

Could this happen?
If this happens only because of program errors, this could be CHECK(). Probably,
we need to reset scheduler after resetting port_manager_* to make this CHECK()?

[Shao-Chuan Lee, 2016/08/25 07:06:42]

I see, this may be replaced with CHECK() since DispatchSendMidiData() shouldn't
be called after Finalize() which blocks until FinalizeOnComThread finishes. I
guess lock is not required here either.

At least |port_manager_out_| should outlive the scheduler since
SendOnComThread() uses it. If the scheduler is destroyed SendOnComThread() won't
be called.

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Done.

+    return;
+
+  scheduler_->PostSendDataTask(
+      client, data.size(), timestamp,
+      base::Bind(&MidiManagerWinrt::SendOnComThread, base::Unretained(this),
+                 port_index, data));
+}
+
+void MidiManagerWinrt::InitializeOnComThread() {
+  base::AutoLock auto_lock(lazy_init_member_lock_);
+
+  com_thread_checker_.reset(new base::ThreadChecker);
+
+  port_manager_in_.reset(new MidiInPortManager(this));
+  port_manager_out_.reset(new MidiOutPortManager(this));
+
+  scheduler_.reset(new MidiScheduler(this));
+
+  if (!(port_manager_in_->StartWatcher() &&
+        port_manager_out_->StartWatcher())) {

[Shao-Chuan Lee, 2016/08/25 02:50:16]

Should initialize MidiOutPortManager first in case the initialization failed but
MidiInPortManager begins to receive data.

[Takashi Toyoshima, 2016/08/25 06:09:45]

MidiManager<Platform> would ask MidiManager to deliver received messages, and
MidiManager deliver it to clients that are registered to the manager. The first
client is registered when the initialization finishes successfully, or
CompleteInitialization() is called with OK. So, even if this class start
receiving, received data will be just ignored in MidiManager. (I didn't try this
scenario actually. So, I may overlook something)

[Shao-Chuan Lee, 2016/08/25 07:06:42]

It will eventually be ignored but I guess it's better to stop here early?

[Takashi Toyoshima, 2016/08/29 09:48:12]

Yeah, not having any assumption for other modules is a good strategy to make it
work correctly :)

+    CompleteInitialization(Result::INITIALIZATION_ERROR);
+  }
+}
+
+void MidiManagerWinrt::FinalizeOnComThread() {
+  base::AutoLock auto_lock(lazy_init_member_lock_);
+
+  DCHECK(com_thread_checker_->CalledOnValidThread());
+
+  scheduler_.reset();
+
+  port_manager_in_.reset();
+  port_manager_out_.reset();
+
+  com_thread_checker_.reset();
+}
+
+void MidiManagerWinrt::SendOnComThread(uint32_t port_index,
+                                       const std::vector<uint8_t>& data) {
+  DCHECK(com_thread_checker_->CalledOnValidThread());
+
+  auto buffer_factory = GetBufferFactory();
+  if (!buffer_factory)

[Takashi Toyoshima, 2016/08/25 06:09:44]

If this happens only when out of memory case, or rare situation OS facing a
critical error, CHECK would be fine, or how about using VLOG?

[Shao-Chuan Lee, 2016/08/25 07:06:42]

Error logs will emit in GetBufferFactory(), do we need additional logs here?

[Takashi Toyoshima, 2016/08/29 09:48:12]

If you think this won't happen in general, CHECK is enough. Otherwise right
recovery code is needed. There VLOG is nice to have.

+    return;
+
+  ScopedComPtr<IBuffer> buffer;
+  HRESULT hr = buffer_factory->Create(static_cast<UINT32>(data.size()),
+                                      buffer.Receive());
+  if (FAILED(hr)) {
+    VLOG(1) << "Create failed: " << PrintHr(hr);
+    return;
+  }
+
+  hr = buffer->put_Length(static_cast<UINT32>(data.size()));
+  if (FAILED(hr)) {
+    VLOG(1) << "put_Length failed: " << PrintHr(hr);
+    return;
+  }
+
+  uint8_t* p_buffer_data = nullptr;
+  hr = GetPointerToBufferData(buffer.get(), &p_buffer_data);
+  if (FAILED(hr))

[Takashi Toyoshima, 2016/08/25 06:09:44]

Similar to the buffer_factory, why no VLOG here?

[Shao-Chuan Lee, 2016/08/25 07:06:42]

ditto

+    return;
+
+  std::copy(data.begin(), data.end(), p_buffer_data);
+
+  MidiPort<IMidiOutPort>* port = port_manager_out_->GetPortByIndex(port_index);
+  if (!port)

[Takashi Toyoshima, 2016/08/25 06:09:45]

Probably, this is the case we should log something, but this also could happen
when our code has a bug.

[Shao-Chuan Lee, 2016/08/29 08:27:52]

Now emitting logs.

+    return;

[Shao-Chuan Lee, 2016/08/25 02:50:16]

Move this to the beginning of function to return early if port is invalid.
|port->handle| should also be checked in case the port is disconnected.

[Takashi Toyoshima, 2016/08/25 06:09:45]

Yeah, can you add TODO with comments?

[Shao-Chuan Lee, 2016/08/25 07:06:42]

Will fix in next patch set.

[Shao-Chuan Lee, 2016/08/29 08:27:51]

Done.

+
+  hr = port->handle->SendBuffer(buffer.get());
+  if (FAILED(hr)) {
+    VLOG(1) << "SendBuffer failed: " << PrintHr(hr);
+    return;
+  }
+}
+
+void MidiManagerWinrt::OnPortManagerReady() {
+  DCHECK(com_thread_checker_->CalledOnValidThread());
+  DCHECK(port_manager_ready_count_ < 2);
+
+  if (++port_manager_ready_count_ == 2)
+    CompleteInitialization(Result::OK);
+}
+
+MidiManager* MidiManager::Create() {
+  return new MidiManagerWinrt();
+}
+
+}  // namespace midi
+}  // namespace media