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..1f4f7af483dcdb4a89d74e9e602268e6cabe0e63
--- /dev/null
+++ b/media/midi/midi_manager_winrt.cc
@@ -0,0 +1,537 @@
+// 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 <functional>
+
+#include "base/bind.h"
+#include "base/containers/hash_tables.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/timer/timer.h"
+#include "base/win/windows_version.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 std::placeholders::_1;
+using std::placeholders::_2;

[Shao-Chuan Lee, 2016/08/18 08:48:57]

Unused, removed

+
+template <typename InterfaceType>
+InterfaceType* GetStatics(const base::string16& class_id) {
+  InterfaceType* ptr;

[Takashi Toyoshima, 2016/08/16 09:30:29]

Probably com_ptr or something is better.
Usually we avoid abbreviation in chromium code base as much as possible. Only
exceptional cases are using it for a well-known terms like HTTP or API providing
names like this case. |ptr| sounds too generic even here.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done. Also changed to using ComPtr to have ref-counting work correctly. "In the
general case any COM pointer you get back from a function should have its
reference count incremented so that you're the owner of that object."

+
+  HRESULT hr = GetActivationFactory(
+      WRL::Wrappers::HStringReference(class_id.c_str()).Get(), &ptr);
+  DCHECK(!FAILED(hr));
+
+  return ptr;
+}
+
+auto GetBufferFactory() {

[Takashi Toyoshima, 2016/08/16 09:30:29]

Generally speaking, trailing return types are not encouraged to declare as
"auto" because it makes code less readable.

In this case, explicit declaration makes it easy to understand object ownership,
like this returns ComPtrRef and we do not need to care for it.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done. Now these factory functions return ComPtr which is owned by the caller.
Added comment on this.

+  return GetStatics<IBufferFactory>(
+      RuntimeClass_Windows_Storage_Streams_Buffer);
+}
+
+auto GetDeviceInformationStatics() {
+  return GetStatics<IDeviceInformationStatics>(
+      RuntimeClass_Windows_Devices_Enumeration_DeviceInformation);
+}
+
+auto GetMidiInPortStatics() {
+  return GetStatics<IMidiInPortStatics>(
+      RuntimeClass_Windows_Devices_Midi_MidiInPort);
+}
+
+auto GetMidiOutPortStatics() {
+  return GetStatics<IMidiOutPortStatics>(
+      RuntimeClass_Windows_Devices_Midi_MidiOutPort);
+}
+
+template <typename T>
+inline std::string GetIdString(T obj) {

[Takashi Toyoshima, 2016/08/16 09:30:29]

Shall we make this "inline" compiler's decision so that compiler can optimize
for size or speed on demand. I think this code paths are not performance
sensitive.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done.

+  HSTRING hs;

[Takashi Toyoshima, 2016/08/16 09:30:28]

"HRESULT hr" is very traditional abbreviation, but I'm not sure about HSTRING.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Using `HSTRING result' now.

+  HRESULT hr = obj->get_Id(&hs);
+  DCHECK(!FAILED(hr));

[Takashi Toyoshima, 2016/08/16 09:30:29]

Just a question: can we assume this call should success always unless OS panics?
In some platforms, OS returns null or empty string for this kind of information,
and we made some crash bugs before.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

HSTRING uses nullptr to represent empty strings, in such cases I guess it should
still return S_OK, however I'll check if the users of HSTRING accepts nullptr.

It seems that WRL suggests checking every returning HRESULT. Simple calls like
these may return errors in cases such as running out of memory. Checks may be
skipped for methods that are documented to always return S_OK.

[Takashi Toyoshima, 2016/08/18 11:09:21]

Acknowledged.

+  return base::WideToUTF8(WindowsGetStringRawBuffer(hs, nullptr));

[Shao-Chuan Lee, 2016/08/18 08:48:56]

This is calling base::WideToUTF8(const std::wstring&), and constructing
std::wstring from nullptr is undefined behavior. Added additional checks.

+}
+
+template <typename T>

[Takashi Toyoshima, 2016/08/16 09:30:29]

same comments on the following methods.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done.

+inline std::string GetDeviceIdString(T obj) {
+  HSTRING hs;
+  HRESULT hr = obj->get_DeviceId(&hs);
+  DCHECK(!FAILED(hr));
+  return base::WideToUTF8(WindowsGetStringRawBuffer(hs, nullptr));
+}
+
+inline std::string GetNameString(IDeviceInformation* info) {
+  HSTRING hs;
+  HRESULT hr = info->get_Name(&hs);
+  DCHECK(!FAILED(hr));
+  return base::WideToUTF8(WindowsGetStringRawBuffer(hs, nullptr));
+}
+
+template <typename InterfaceType>
+struct MidiPort {
+  MidiPort() {}
+
+  uint32_t index;
+  WRL::ComPtr<InterfaceType> handle;
+  base::TimeTicks start_time;

[Takashi Toyoshima, 2016/08/16 09:30:29]

This looks working differently from the spec.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MidiPort);
+};
+
+template <typename InterfaceType, typename RuntimeType>
+class MidiPortManager {
+ public:
+  MidiPortManager() {}

[Takashi Toyoshima, 2016/08/16 09:30:29]

FYI, we are allowed to use "= default" if you want.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done.

+
+  void StartWatcher() {
+    HRESULT hr;
+
+    hr = GetDeviceInformationStatics()->CreateWatcherAqsFilter(
+        GetDeviceSelector(), &watcher_);
+    DCHECK(!FAILED(hr));
+
+    hr = watcher_->add_Added(
+        WRL::Callback<ITypedEventHandler<DeviceWatcher*, DeviceInformation*>>(
+            std::bind(&MidiPortManager::OnAdded, this, _1, _2))
+            .Get(),
+        &token_Added_);
+    DCHECK(!FAILED(hr));
+
+    hr = watcher_->add_EnumerationCompleted(
+        WRL::Callback<ITypedEventHandler<DeviceWatcher*, IInspectable*>>(
+            std::bind(&MidiPortManager::OnEnumerationCompleted, this, _1, _2))
+            .Get(),
+        &token_EnumerationCompleted_);
+    DCHECK(!FAILED(hr));
+
+    hr = watcher_->add_Removed(
+        WRL::Callback<
+            ITypedEventHandler<DeviceWatcher*, DeviceInformationUpdate*>>(
+            std::bind(&MidiPortManager::OnRemoved, this, _1, _2))
+            .Get(),
+        &token_Removed_);
+    DCHECK(!FAILED(hr));
+
+    hr = watcher_->add_Stopped(
+        WRL::Callback<ITypedEventHandler<DeviceWatcher*, IInspectable*>>(
+            std::bind(&MidiPortManager::OnStopped, this, _1, _2))
+            .Get(),
+        &token_Stopped_);
+    DCHECK(!FAILED(hr));
+
+    hr = watcher_->add_Updated(
+        WRL::Callback<
+            ITypedEventHandler<DeviceWatcher*, DeviceInformationUpdate*>>(
+            std::bind(&MidiPortManager::OnUpdated, this, _1, _2))
+            .Get(),
+        &token_Updated_);
+    DCHECK(!FAILED(hr));
+
+    hr = watcher_->Start();
+    DCHECK(!FAILED(hr));
+  }
+
+  MidiPort<InterfaceType>* GetPortByDeviceId(std::string dev_id) {
+    base::AutoLock auto_lock(ports_lock_);
+    auto it = ports_.find(dev_id);
+    if (it == ports_.end())
+      return nullptr;
+    return it->second.get();
+  }
+
+  MidiPort<InterfaceType>* GetPortByIndex(uint32_t port_index) {
+    base::AutoLock auto_lock(ports_lock_);
+    auto it = ports_.find(port_ids_[port_index]);
+    if (it == ports_.end())
+      return nullptr;
+    return it->second.get();
+  }
+
+  // DeviceWatcher callbacks:
+  HRESULT OnAdded(IDeviceWatcher* watcher, IDeviceInformation* info) {
+    // TODO(shaochuan): Disable Microsoft GS Wavetable Synth due to security

[Takashi Toyoshima, 2016/08/16 09:30:29]

Do you know if we can identify software virtual ports here?

Or plan is to have an explicit flag for the first Web MIDI call,
requestMIDIAccess(), and if software is allowed, we popup a permission bubble to
ask permission to users.

So, it's ok to disable all software synths including MS one here at this point
if it's easy to implement.

Also probably we want to try running attack code against MS synth with this flag
enabled later in order to understand what happens if it's exploited.

[Shao-Chuan Lee, 2016/08/18 08:48:57]

The MS synth can be identified using WinRT MidiSynthesizer class
(https://msdn.microsoft.com/en-us/library/windows/apps/windows.devices.midi.mi...).
Will look for ways to identify other software synths.

[Takashi Toyoshima, 2016/08/18 11:09:21]

Acknowledged.

+    // reasons. https://bugs.chromium.org/p/project-zero/issues/detail?id=454
+
+    HSTRING dev_id_hs;

[Takashi Toyoshima, 2016/08/16 09:30:29]

Can you use GetIdString you defined?

[Shao-Chuan Lee, 2016/08/18 08:48:57]

This HSTRING is required for RegisterGetPortFromIdAsync afterwards. Renamed to
dev_id_hstring.

[Takashi Toyoshima, 2016/08/18 11:09:21]

Ah, I see. I misunderstood these lines.

+    HRESULT hr = info->get_Id(&dev_id_hs);
+    DCHECK(!FAILED(hr));
+
+    std::string dev_id =
+        base::WideToUTF8(WindowsGetStringRawBuffer(dev_id_hs, nullptr));

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Now using GetIdString to reuse nullptr checking code.

+
+    {
+      base::AutoLock auto_lock(ports_lock_);
+      port_names_[dev_id] = GetNameString(info);
+    }
+
+    WRL::ComPtr<IAsyncOperation<RuntimeType*>> async_op;
+
+    RegisterGetPortFromIdAsync(&async_op, dev_id_hs);
+
+    hr = async_op->put_Completed(
+        WRL::Callback<IAsyncOperationCompletedHandler<RuntimeType*>>(
+            std::bind(&MidiPortManager::OnCompletedGetPortFromIdAsync, this, _1,
+                      _2))
+            .Get());
+    DCHECK(!FAILED(hr));
+
+    {
+      base::AutoLock auto_lock(async_ops_lock_);
+      async_ops_.insert(std::move(async_op));
+    }
+
+    return S_OK;
+  }
+
+  HRESULT OnEnumerationCompleted(IDeviceWatcher* watcher, IInspectable* insp) {
+    // TODO(shaochuan)

[Takashi Toyoshima, 2016/08/16 09:30:28]

What's missing here?
At least, we should complete the initialization after the first enumeration
finishes.
It's ok to implement it in the next CL, but please write a clear TODO.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Should CompleteInitialization() be moved here? I wonder if this would slow down
Chrome startup.

[Takashi Toyoshima, 2016/08/18 11:09:21]

This isn't a problem for two reasons.

- MidiManager's initialization was designed to be asynchronous. Thus, slow
initialization does not block anything.
- The initialization happens on the first JavaScript call to use Web MIDI API.
So this does not run until Web MIDI is used.

Here is a scenario how the initialization happens for Web MIDI.

1. JavaScript calls navigator.requestMIDIAccess() that returns a Promise object
immediately
2. renderer ask browser to initialize the MidiManager asynchronously
3. MidiManager.StartSession is called for each Web MIDI request on the I/O
thread
4. MidiManagerWinrt.StartInitialization runs on the I/O thead if this is the
first StartSession call
5. Existing devices should be enumerated here
6. CompleteInitialization() is called when asynchronous platform dependent
initialization finishes
7. the renderer was notified to resolve the promise object.
8. For JavaScript the Promise object was resolved, and success or error callback
will be invoked to obtain the first set of MIDIPort objects.

+    return S_OK;
+  }
+
+  HRESULT OnRemoved(IDeviceWatcher* watcher, IDeviceInformationUpdate* update) {
+    MidiPort<InterfaceType>* port = GetPortByDeviceId(GetIdString(update));
+    DCHECK(port != nullptr);
+
+    SetPortState(port->index, MIDI_PORT_DISCONNECTED);
+
+    port->handle = nullptr;
+
+    return S_OK;
+  }
+
+  HRESULT OnStopped(IDeviceWatcher* watcher, IInspectable* insp) {
+    // TODO(shaochuan)

[Takashi Toyoshima, 2016/08/16 09:30:29]

ditto

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Does nothing for now, leaving as a placeholder for future usage.

+    return S_OK;
+  }
+
+  HRESULT OnUpdated(IDeviceWatcher* watcher, IDeviceInformationUpdate* update) {
+    // TODO(shaochuan)

[Takashi Toyoshima, 2016/08/16 09:30:29]

ditto

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Should update device information here, will check which fields are updated
through this callback.

+    return S_OK;
+  }
+
+ private:
+  HRESULT OnCompletedGetPortFromIdAsync(IAsyncOperation<RuntimeType*>* async_op,
+                                        AsyncStatus status) {
+    const auto now = base::TimeTicks::Now();
+
+    InterfaceType* handle;
+    HRESULT hr = async_op->GetResults(&handle);
+    DCHECK(!FAILED(hr));
+
+    RegisterOnMessageReceived(handle);
+
+    std::string dev_id = GetDeviceIdString(handle);
+
+    MidiPort<InterfaceType>* port = GetPortByDeviceId(dev_id);
+
+    if (port == nullptr) {
+      base::AutoLock auto_lock(ports_lock_);
+
+      // TODO(shaochuan)

[Takashi Toyoshima, 2016/08/16 09:30:28]

Clear comments for this TODO?

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done.

+      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 = now;
+
+    {
+      base::AutoLock auto_lock(async_ops_lock_);
+      auto it = async_ops_.find(async_op);
+      DCHECK(it != async_ops_.end());
+      async_ops_.erase(it);
+    }
+
+    return S_OK;
+  }
+
+  // Implemented by child classes to call corresponding input/output methods.

[Takashi Toyoshima, 2016/08/16 09:30:28]

"to return a corresponding device selector for input and output" sounds clear
for me.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

This comment was meant to describe all the following virtual functions... Now
GetDeviceSelector and RegisterGetPortFromIdAsync is removed, and comments added
for the rest 3 methods.

+  virtual HSTRING GetDeviceSelector() = 0;
+
+  virtual void RegisterGetPortFromIdAsync(

[Takashi Toyoshima, 2016/08/16 09:30:29]

Also can you add some comments for other virtual methods declared in this class?

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done.

+      IAsyncOperation<RuntimeType*>** p_async_op,
+      HSTRING dev_id) = 0;
+
+  virtual void RegisterOnMessageReceived(InterfaceType* handle) = 0;
+
+  virtual void AddPort(MidiPortInfo info) = 0;
+
+  virtual void SetPortState(uint32_t port_index, MidiPortState state) = 0;
+
+  WRL::ComPtr<IDeviceWatcher> watcher_;
+  EventRegistrationToken token_Added_, token_EnumerationCompleted_,
+      token_Removed_, token_Stopped_, token_Updated_;
+
+  base::Lock ports_lock_;

[Takashi Toyoshima, 2016/08/16 09:30:29]

Can you explain (== add comments) why locks are needed for them?
Or rephrasing it, can you have a comment on threading in this file.

Also can you have base/threading/thread_checker?
It's useful to avoid methods running on unexpected threads, and also useful to
provide thread expectation for other developers reading your code.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Locks are required since fields are manipulated from WinRT callback threads,
where fields may be accessed concurrently. Added comments.
For now only WinRT methods should be either call on the COM thread or on the OS
callback thread.  Will further investigate the WinRT threading model and revise
this.

+  base::hash_map<std::string, std::unique_ptr<MidiPort<InterfaceType>>>
+      ports_;                          // GUARDED_BY(ports_lock_)
+  std::vector<std::string> port_ids_;  // GUARDED_BY(ports_lock_)
+  base::hash_map<std::string, std::string>
+      port_names_;  // GUARDED_BY(ports_lock_)
+
+  base::Lock async_ops_lock_;
+  std::set<WRL::ComPtr<IAsyncOperation<RuntimeType*>>>
+      async_ops_;  // GUARDED_BY(async_ops_lock_)
+};
+
+}  // namespace
+
+class MidiManagerWinrt::MidiInPortManager final
+    : public MidiPortManager<IMidiInPort, MidiInPort> {
+ public:
+  MidiInPortManager(MidiManagerWinrt* midi_manager)
+      : midi_manager_(midi_manager) {}
+
+ private:
+  // MidiPortManager overrides:
+  HSTRING GetDeviceSelector() override {
+    HSTRING selector = nullptr;
+    HRESULT hr = GetMidiInPortStatics()->GetDeviceSelector(&selector);
+    DCHECK(!FAILED(hr));
+    return selector;
+  }
+
+  void RegisterGetPortFromIdAsync(IAsyncOperation<MidiInPort*>** p_async_op,
+                                  HSTRING dev_id) override {
+    HRESULT hr = GetMidiInPortStatics()->FromIdAsync(dev_id, p_async_op);
+    DCHECK(!FAILED(hr));
+  }
+
+  void RegisterOnMessageReceived(IMidiInPort* handle) override {
+    base::AutoLock auto_lock(tokens_lock_);
+    EventRegistrationToken& token = tokens_[GetDeviceIdString(handle)];
+
+    handle->add_MessageReceived(
+        WRL::Callback<
+            ITypedEventHandler<MidiInPort*, MidiMessageReceivedEventArgs*>>(
+            std::bind(&MidiInPortManager::OnMessageReceived, this, _1, _2))
+            .Get(),
+        &token);
+  }
+
+  void AddPort(MidiPortInfo info) { midi_manager_->AddInputPort(info); }
+
+  void SetPortState(uint32_t port_index, MidiPortState state) {
+    midi_manager_->SetInputPortState(port_index, state);

[Takashi Toyoshima, 2016/08/16 09:30:29]

Probably, this call should be called on the I/O thread, but is this on com
thread?

[Shao-Chuan Lee, 2016/08/18 08:48:56]

This is currently called on the OS callback thread. Are there documents on which
methods should be called on which threads?

[Takashi Toyoshima, 2016/08/18 11:09:21]

Basically, if you can not find a clear comment, it isn't thread safe.
But as you requested, we should have some comments on MidiManagerClient because
MidiManager runs on multiple threads, and it needs to interact with the class.

Another question:
Can we ensure such OS running callbacks always run before the MidiManager
instance is destructed? Otherwise, these callbacks will touch the pointer for
the destructed instance, and result in read-after-free issues.

Some approaches I see often are:
 - All callbacks are aborted when a kind of interface pointer is released, or
interface is closed.
 - Use static method with a weak pointer to call a member method indirectly only
when the pointer is valid.

+  }
+
+  // Callback on receiving MIDI input message.
+  HRESULT OnMessageReceived(IMidiInPort* handle,
+                            IMidiMessageReceivedEventArgs* args) {
+    MidiPort<IMidiInPort>* port = GetPortByDeviceId(GetDeviceIdString(handle));
+    DCHECK(port != nullptr);
+
+    WRL::ComPtr<IMidiMessage> message;
+    HRESULT hr = args->get_Message(&message);
+    DCHECK(!FAILED(hr));
+
+    WRL::ComPtr<IBuffer> buffer;
+    hr = message->get_RawData(&buffer);
+    DCHECK(!FAILED(hr));
+
+    // Use BufferByteAccess object to access buffer memory directly.
+    WRL::ComPtr<IInspectable> insp(

[Takashi Toyoshima, 2016/08/16 09:30:29]

Shall we have a clear name?

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Moved to a separate function, using `buffer_byte_access' and `inspectable'.

+        reinterpret_cast<IInspectable*>(buffer.Get()));
+    WRL::ComPtr<Windows::Storage::Streams::IBufferByteAccess> bba;

[Takashi Toyoshima, 2016/08/16 09:30:29]

ditto

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done.

+    hr = insp.As(&bba);
+    DCHECK(!FAILED(hr));
+
+    uint8_t* data_arr = nullptr;

[Takashi Toyoshima, 2016/08/16 09:30:29]

ditto

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Using `p_buffer_data'.

+    hr = bba->Buffer(reinterpret_cast<byte**>(&data_arr));

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Removing reinterpret_cast here since byte** is compatible with uint8_t**, and to
be able to detect errors during compilation if any mistake is made in the
future.

+    DCHECK(!FAILED(hr));
+
+    uint32_t len;

[Takashi Toyoshima, 2016/08/16 09:30:29]

probably we may also want to avoid the name like len here.

[Shao-Chuan Lee, 2016/08/18 08:48:57]

Using `data_length'.

+    hr = buffer->get_Length(&len);
+    DCHECK(!FAILED(hr));
+    DCHECK(len != 0);
+
+    std::vector<uint8_t> data(data_arr, data_arr + len);
+
+    // Time since port opened in 100-nanosecond units.
+    TimeSpan time_span;
+    hr = message->get_Timestamp(&time_span);
+    DCHECK(!FAILED(hr));
+
+    midi_manager_->ReceiveMidiData(
+        port->index, &data[0], data.size(),
+        port->start_time +

[Takashi Toyoshima, 2016/08/16 09:30:29]

Can we check if how much start_time+time_span can be different from
TimeTicks::Now() here. OnMessageReceived must be delivered on time as we can
perceive, but I have a little concern about port opening, that means start_time
might be inaccurate. Just in case.

[Shao-Chuan Lee, 2016/08/18 08:48:57]

message->get_Timestamp() returns "the duration from when the MidiInPort was
created to the time the message was received," according to
https://msdn.microsoft.com/en-us/library/windows/apps/windows.devices.midi.im....
Now retrieving the "create time" in the beginning of
OnCompletedGetPortFromIdAsync, will check accuracy.

+            base::TimeDelta::FromMicroseconds(time_span.Duration / 10));
+
+    return S_OK;
+  }
+
+  MidiManagerWinrt* midi_manager_;
+
+  base::Lock tokens_lock_;
+  base::hash_map<std::string, EventRegistrationToken>
+      tokens_;  // GUARDED_BY(tokens_lock_)
+
+  DISALLOW_COPY_AND_ASSIGN(MidiInPortManager);
+};
+
+class MidiManagerWinrt::MidiOutPortManager final
+    : public MidiPortManager<IMidiOutPort, IMidiOutPort> {
+ public:
+  MidiOutPortManager(MidiManagerWinrt* midi_manager)
+      : midi_manager_(midi_manager) {}
+
+ private:
+  // MidiPortManager overrides:
+  HSTRING GetDeviceSelector() override {
+    HSTRING selector = nullptr;
+    HRESULT hr = GetMidiOutPortStatics()->GetDeviceSelector(&selector);
+    DCHECK(!FAILED(hr));
+    return selector;
+  }
+
+  void RegisterGetPortFromIdAsync(IAsyncOperation<IMidiOutPort*>** p_async_op,
+                                  HSTRING dev_id) override {
+    HRESULT hr = GetMidiOutPortStatics()->FromIdAsync(dev_id, p_async_op);
+    DCHECK(!FAILED(hr));
+  }
+
+  void RegisterOnMessageReceived(IMidiOutPort* handle) override {}
+
+  void AddPort(MidiPortInfo info) { midi_manager_->AddOutputPort(info); }
+
+  void SetPortState(uint32_t port_index, MidiPortState state) {
+    midi_manager_->SetOutputPortState(port_index, state);
+  }
+
+  MidiManagerWinrt* midi_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(MidiOutPortManager);
+};
+
+MidiManagerWinrt::MidiManagerWinrt() : com_thread_("Windows MIDI COM Thread") {}
+
+MidiManagerWinrt::~MidiManagerWinrt() {}
+
+void MidiManagerWinrt::StartInitialization() {
+  DCHECK(base::win::GetVersion() >= base::win::VERSION_WIN10);
+
+  scheduler_.reset(new MidiScheduler(this));
+
+  com_thread_.init_com_with_mta(true);
+  com_thread_.Start();
+
+  com_thread_.message_loop()->PostTask(
+      FROM_HERE, base::Bind(&MidiManagerWinrt::InitializeOnComThread,
+                            base::Unretained(this)));
+
+  CompleteInitialization(Result::OK);
+}
+
+void MidiManagerWinrt::Finalize() {
+  // TODO(shaochuan): Check if everything is destructed gracefully.
+  port_manager_in_.reset();
+  port_manager_out_.reset();
+
+  scheduler_.reset();
+
+  com_thread_.Stop();
+}
+
+void MidiManagerWinrt::DispatchSendMidiData(MidiManagerClient* client,
+                                            uint32_t port_index,
+                                            const std::vector<uint8_t>& data,
+                                            double timestamp) {
+  com_thread_.message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&MidiManagerWinrt::PostSendDataTaskOnComThread,
+                 base::Unretained(this), client, port_index, data, timestamp));
+}
+
+void MidiManagerWinrt::AssertOnComThread() {
+  DCHECK_EQ(com_thread_.GetThreadId(), base::PlatformThread::CurrentId());

[Takashi Toyoshima, 2016/08/16 09:30:29]

Ah, you may want to replace this with the thread checker I commented above.

[Takashi Toyoshima, 2016/08/18 11:09:21]

not fixed yet?

+}
+
+void MidiManagerWinrt::InitializeOnComThread() {
+  AssertOnComThread();
+
+  port_manager_in_.reset(new MidiInPortManager(this));
+  port_manager_out_.reset(new MidiOutPortManager(this));
+
+  port_manager_in_->StartWatcher();
+  port_manager_out_->StartWatcher();
+}
+
+void MidiManagerWinrt::PostSendDataTaskOnComThread(
+    MidiManagerClient* client,
+    uint32_t port_index,
+    const std::vector<uint8_t>& data,
+    double timestamp) {
+  AssertOnComThread();
+
+  scheduler_->PostSendDataTask(
+      client, data.size(), timestamp,
+      base::Bind(&MidiManagerWinrt::SendOnComThread, base::Unretained(this),
+                 port_index, data));
+}
+
+void MidiManagerWinrt::SendOnComThread(uint32_t port_index,
+                                       const std::vector<uint8_t>& data) {
+  AssertOnComThread();
+
+  WRL::ComPtr<IBuffer> buffer;
+  HRESULT hr =
+      GetBufferFactory()->Create(static_cast<UINT32>(data.size()), &buffer);
+  DCHECK(!FAILED(hr));
+
+  hr = buffer->put_Length(static_cast<UINT32>(data.size()));
+  DCHECK(!FAILED(hr));
+
+  // Use BufferByteAccess object to access buffer memory directly.

[Takashi Toyoshima, 2016/08/16 09:30:29]

Clear names.

[Shao-Chuan Lee, 2016/08/18 08:48:56]

Done.

+  WRL::ComPtr<IInspectable> insp(reinterpret_cast<IInspectable*>(buffer.Get()));
+  WRL::ComPtr<Windows::Storage::Streams::IBufferByteAccess> bba;
+  hr = insp.As(&bba);
+  DCHECK(!FAILED(hr));
+
+  uint8_t* data_arr = nullptr;
+  hr = bba->Buffer(reinterpret_cast<byte**>(&data_arr));

[Shao-Chuan Lee, 2016/08/18 08:48:56]

ditto

+  DCHECK(!FAILED(hr));
+
+  std::copy(data.begin(), data.end(), data_arr);
+
+  MidiPort<IMidiOutPort>* port = port_manager_out_->GetPortByIndex(port_index);
+  DCHECK(port != nullptr);
+
+  hr = port->handle->SendBuffer(buffer.Get());
+  DCHECK(!FAILED(hr));
+}
+
+MidiManager* MidiManager::Create() {
+  return new MidiManagerWinrt();
+}
+
+}  // namespace midi
+}  // namespace media