Web MIDI: adding backend to support dynamic instantiation on Windows

media/midi/dynamically_initialized_midi_manager_win.cc

+// Copyright 2017 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/dynamically_initialized_midi_manager_win.h"
+
+#include <windows.h>
+
+#include <mmreg.h>
+#include <mmsystem.h>
+
+#include <algorithm>
+#include <string>
+
+#include "base/callback.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string16.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/lock.h"
+#include "media/midi/midi_port_info.h"
+#include "media/midi/midi_service.h"
+
+namespace midi {
+
+namespace {
+
+// Global variables to identify MidiManager instance.

[Takashi Toyoshima, 2017/02/17 09:34:27]

Concept is same with ALSA's dynamic instantiation support

+constexpr int kInvalidInstanceId = -1;
+int g_active_instance_id = kInvalidInstanceId;
+int g_next_instance_id = 0;
+DynamicallyInitializedMidiManagerWin* g_manager_instance = nullptr;
+
+// Obtains base::Lock instance pointer to lock instance_id.
+base::Lock* GetInstanceIdLock() {
+  static base::Lock* lock = new base::Lock;

[yhirano, 2017/02/17 10:27:49]

Looks racy; If there is a guarantee that the first call will not be done
simultaneously, please write so (with reason).

[Takashi Toyoshima, 2017/02/17 11:46:36]

This looks a quite new idiom that is replacing LazyInstance.
Now LazyInstance is not permitted under media

+  return lock;
+}
+
+// Use single TaskRunner for all tasks running outside the I/O thread.
+constexpr int kTaskRunner = 0;
+
+// Obtains base::Lock instance pointer to ensure tasks run safely on TaskRunner.
+base::Lock* GetTaskLock() {
+  static base::Lock* lock = new base::Lock;
+  return lock;
+}
+
+// Helper function to run a posted task on TaskRunner safely.
+void RunTask(int instance_id, const base::Closure& task) {
+  // Obtains task lock to ensure that the instance should not complete
+  // Finalize() while running the |task|.
+  base::AutoLock task_lock(*GetTaskLock());
+  {
+    // If Finalize() finished before the lock avobe, do nothing.
+    base::AutoLock lock(*GetInstanceIdLock());
+    if (instance_id != g_active_instance_id)
+      return;
+  }
+  task.Run();
+}
+
+// TODO(toyoshim): Factor out TaskRunner related functionaliries above, and
+// deprecate MidiScheduler. It should be available via MidiManager::scheduler().
+
+enum class EnumerateState { Connected, Enumerating };
+
+class Port {
+ public:
+  Port(const std::string& type,
+       uint32_t device_id,
+       uint16_t manufacturer_id,
+       uint16_t product_id,
+       uint32_t driver_version,
+       const std::string& product_name)
+      : index_(0u),
+        type_(type),
+        device_id_(device_id),
+        manufacturer_id_(manufacturer_id),
+        product_id_(product_id),
+        driver_version_(driver_version),
+        product_name_(product_name),
+        enumerate_state_(EnumerateState::Enumerating) {
+    info_.manufacturer = "unknown";  // TODO(toyoshim): Use USB information.
+    info_.name = product_name_;
+    info_.version = base::StringPrintf("%d.%d", HIBYTE(driver_version_),
+                                       LOBYTE(driver_version_));
+    info_.state = mojom::PortState::DISCONNECTED;
+  }
+

[yhirano, 2017/02/17 10:27:49]

+virtual ~Port() {}

[Takashi Toyoshima, 2017/02/17 11:46:36]

Done.

+  bool operator==(const Port& other) const {
+    // Should not use |device_id| for comparison because it can be changed on
+    // each enumeration.
+    // Since the GUID will be changed on each enumeration for Microsoft GS
+    // Wavetable synth and might be done for others, do not use it for device
+    // comparison.
+    return manufacturer_id_ == other.manufacturer_id_ &&
+           product_id_ == other.product_id_ &&
+           driver_version_ == other.driver_version_ &&
+           product_name_ == other.product_name_;
+  }
+
+  bool IsConnected() const {
+    return info_.state != mojom::PortState::DISCONNECTED;
+  }
+
+  void set_index(size_t index) {
+    index_ = index;
+    // TODO(toyoshim): Use hashed ID.
+    info_.id = base::StringPrintf("%s-%d", type_.c_str(), index_);
+  }
+  size_t index() { return index_; }
+  void set_device_id(uint32_t device_id) { device_id_ = device_id; }
+  void set_enumerate_state(EnumerateState state) { enumerate_state_ = state; }
+  EnumerateState enumerate_state() { return enumerate_state_; }
+  const MidiPortInfo& info() { return info_; }
+
+  virtual void Connect() {
+    info_.state = mojom::PortState::CONNECTED;
+    // TODO(toyoshim) Until open() / close() are supported, open each device on
+    // connected.
+    Open();
+  }
+
+  virtual void Disconnect() { info_.state = mojom::PortState::DISCONNECTED; }
+
+  virtual void Open() { info_.state = mojom::PortState::OPENED; }
+
+ protected:
+  size_t index_;
+  std::string type_;
+  uint32_t device_id_;
+  const uint16_t manufacturer_id_;
+  const uint16_t product_id_;
+  const uint32_t driver_version_;
+  const std::string product_name_;
+  MidiPortInfo info_;
+  EnumerateState enumerate_state_;
+};  // class Port
+
+}  // namespace
+
+// TODO(toyoshim): Following patches will implement actual functions.
+class DynamicallyInitializedMidiManagerWin::InPort final : public Port {
+ public:
+  InPort(int instance_id, UINT device_id, const MIDIINCAPS2W& caps)

[Takashi Toyoshima, 2017/02/17 17:42:29]

this first argument wasn't used yet in this first cl

+      : Port("input",
+             device_id,
+             caps.wMid,
+             caps.wPid,
+             caps.vDriverVersion,
+             base::WideToUTF8(
+                 base::string16(caps.szPname, wcslen(caps.szPname)))) {}
+};
+
+// TODO(toyoshim): Following patches will implement actual functions.
+class DynamicallyInitializedMidiManagerWin::OutPort final : public Port {
+ public:
+  OutPort(UINT device_id, const MIDIOUTCAPS2W& caps)
+      : Port("output",
+             device_id,
+             caps.wMid,
+             caps.wPid,
+             caps.vDriverVersion,
+             base::WideToUTF8(
+                 base::string16(caps.szPname, wcslen(caps.szPname)))),
+        software_(caps.wTechnology == MOD_SWSYNTH) {}
+
+  // Port overrides:
+  void Connect() override {
+    // Until |software| option is supported, disable Microsoft GS Wavetable
+    // Synth that has a known security issue.
+    if (software_ && manufacturer_id_ == MM_MICROSOFT &&
+        (product_id_ == MM_MSFT_WDMAUDIO_MIDIOUT ||
+         product_id_ == MM_MSFT_GENERIC_MIDISYNTH)) {
+      return;
+    }
+    Port::Connect();
+  }
+
+ private:
+  const bool software_;
+};
+
+DynamicallyInitializedMidiManagerWin::DynamicallyInitializedMidiManagerWin(
+    MidiService* service)
+    : MidiManager(service), instance_id_(kInvalidInstanceId) {
+  base::AutoLock lock(*GetInstanceIdLock());
+  CHECK_EQ(kInvalidInstanceId, g_active_instance_id);
+
+  // Obtains the task runner for the current thread that hosts this instnace.
+  thread_runner_ = base::ThreadTaskRunnerHandle::Get();
+}
+
+DynamicallyInitializedMidiManagerWin::~DynamicallyInitializedMidiManagerWin() {
+  base::AutoLock lock(*GetInstanceIdLock());
+  CHECK_EQ(kInvalidInstanceId, g_active_instance_id);
+  CHECK(thread_runner_->BelongsToCurrentThread());
+}
+
+void DynamicallyInitializedMidiManagerWin::PostReplyTask(
+    const base::Closure& task) {

[yhirano, 2017/02/17 10:27:49]

base::AutoLock lock(*GetInstanceIdLock());

[Takashi Toyoshima, 2017/02/17 11:46:36]

instance id lock is used only for global variables that are related to instance
id.
|instance_id_| is a member variable, that are usually
 - read-only on task runner threads, and protected by task lock (at RunTask)
 - not protected on the I/O thread there read and write happen but on the same
thread

[yhirano, 2017/02/17 11:51:43]

Thank you! I overlooked that.
Is it possible to initialize instance_id_ in the constructor, and make it const?
That will make it clear that accessing instance_id_ is safe. What I'm suggesting
is, create IssueNextInstanceId(), call it in the constructor, and update
g_active_instance_id in StartInitialization.

+  thread_runner_->PostTask(FROM_HERE, base::Bind(&RunTask, instance_id_, task));
+}
+
+void DynamicallyInitializedMidiManagerWin::StartInitialization() {
+  {
+    base::AutoLock lock(*GetInstanceIdLock());
+    CHECK_EQ(kInvalidInstanceId, g_active_instance_id);
+    instance_id_ = g_next_instance_id++;
+    g_active_instance_id = instance_id_;
+    CHECK_EQ(nullptr, g_manager_instance);
+    g_manager_instance = this;
+  }
+  // Registers on the I/O thread to be notified on the I/O thread.
+  CHECK(thread_runner_->BelongsToCurrentThread());
+  base::SystemMonitor::Get()->AddDevicesChangedObserver(this);
+
+  // Starts asynchronous initialization on TaskRunner.
+  PostTask(
+      base::Bind(&DynamicallyInitializedMidiManagerWin::InitializeOnTaskRunner,
+                 base::Unretained(this)));
+}
+
+void DynamicallyInitializedMidiManagerWin::Finalize() {
+  // Unregisters on the I/O thread. OnDevicesChanged() won't be called any more.
+  CHECK(thread_runner_->BelongsToCurrentThread());
+  base::SystemMonitor::Get()->RemoveDevicesChangedObserver(this);
+  {
+    base::AutoLock lock(*GetInstanceIdLock());
+    CHECK_EQ(instance_id_, g_active_instance_id);
+    g_active_instance_id = kInvalidInstanceId;
+    CHECK_EQ(this, g_manager_instance);
+    g_manager_instance = nullptr;
+  }
+
+  // Ensures that no task runs on TaskRunner so to destruct the instance safely.
+  // Tasks that did not started yet will do nothing after invalidate the
+  // instance ID above.
+  base::AutoLock lock(*GetTaskLock());
+}
+
+void DynamicallyInitializedMidiManagerWin::DispatchSendMidiData(
+    MidiManagerClient* client,
+    uint32_t port_index,
+    const std::vector<uint8_t>& data,
+    double timestamp) {
+  // TODO(toyoshim): Following patches will implement.
+}
+
+void DynamicallyInitializedMidiManagerWin::OnDevicesChanged(
+    base::SystemMonitor::DeviceType device_type) {
+  // Notified on the I/O thread.
+  CHECK(thread_runner_->BelongsToCurrentThread());
+
+  switch (device_type) {
+    case base::SystemMonitor::DEVTYPE_AUDIO:
+    case base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE:
+      // Add case of other unrelated device types here.
+      return;
+    case base::SystemMonitor::DEVTYPE_UNKNOWN: {
+      base::AutoLock lock(*GetInstanceIdLock());

[yhirano, 2017/02/17 10:27:49]

I believe these two lines should be in PostTask. PostTask touches instance_id_
so it should need the instance id lock (some are missing in this patch). As
g_active_instance_id is modified only on the IO thread, expecting
g_active_instance_id is equal to instance_id_ is safe on the thread.

[Takashi Toyoshima, 2017/02/17 11:46:36]

For missing lock for instance_id_, see my previous comment.

My original intention of this check is to ensure that we unregister the instance
from the system monitor observer list currently. But in such case, we can not
access "this" correctly. So this check does not make sense.

I will remove this check.

+      CHECK_EQ(instance_id_, g_active_instance_id);
+      PostTask(base::Bind(
+          &DynamicallyInitializedMidiManagerWin::UpdateDeviceListOnTaskRunner,
+          base::Unretained(this)));
+      break;
+    }
+  }
+}
+
+void DynamicallyInitializedMidiManagerWin::PostTask(const base::Closure& task) {

[yhirano, 2017/02/17 10:27:49]

DCHECK(thread_runner_->BelongsToCurrentThread()); or CHECK?

[Takashi Toyoshima, 2017/02/17 11:46:36]

I will use this even from task runner.
I expect this method is still thread safe.
(service()->GetTaskRunner() is thread safe)

+  service()
+      ->GetTaskRunner(kTaskRunner)
+      ->PostTask(FROM_HERE, base::Bind(&RunTask, instance_id_, task));
+}
+
+void DynamicallyInitializedMidiManagerWin::InitializeOnTaskRunner() {
+  UpdateDeviceListOnTaskRunner();
+  PostReplyTask(
+      base::Bind(&DynamicallyInitializedMidiManagerWin::CompleteInitialization,
+                 base::Unretained(this), mojom::Result::OK));
+}
+
+void DynamicallyInitializedMidiManagerWin::UpdateDeviceListOnTaskRunner() {

[Takashi Toyoshima, 2017/02/17 09:34:27]

main logic of this patch.
100+ lines, but latter part for output ports is almost a duplication of one for
input ports actually.

[yhirano, 2017/02/17 10:27:49]

Splitting this method into two (one for input and one for output) would be good.

[Takashi Toyoshima, 2017/02/17 11:46:36]

will try rewriting this as we discussed

+  // Update the devie list for input ports.
+  // First, mark all registered input ports to |EnumerateState::Enumerating|.
+  for (const auto& port : input_ports_)
+    port->set_enumerate_state(EnumerateState::Enumerating);
+
+  std::vector<std::unique_ptr<InPort>> new_input_ports;
+  const UINT num_in_devices = midiInGetNumDevs();
+  for (UINT device_id = 0; device_id < num_in_devices; ++device_id) {
+    MIDIINCAPS2W caps;
+    MMRESULT result = midiInGetDevCaps(
+        device_id, reinterpret_cast<LPMIDIINCAPSW>(&caps), sizeof(caps));
+    if (result != MMSYSERR_NOERROR) {
+      LOG(ERROR) << "midiInGetDevCaps fails on device " << device_id;
+      continue;
+    }
+    std::unique_ptr<InPort> port =
+        base::MakeUnique<InPort>(instance_id_, device_id, caps);
+    const auto& found = std::find_if(
+        input_ports_.begin(), input_ports_.end(),
+        [&port](const auto& candidate) { return *candidate == *port; });
+    if (found == input_ports_.end()) {
+      new_input_ports.push_back(std::move(port));
+    } else {
+      // Update device ID which may be changed when device list is updated.
+      (*found)->set_device_id(device_id);
+      // Found devices in the list are marked to |EnumerateState::Connected|.
+      (*found)->set_enumerate_state(EnumerateState::Connected);
+    }
+  }
+  for (const auto& port : input_ports_) {
+    if (port->enumerate_state() == EnumerateState::Enumerating) {
+      // Disconnected ports have kept staying in |EnumerateState::Enumerating|
+      // state as they were marked first.
+      if (port->IsConnected()) {
+        // Change state and notify it.
+        port->Disconnect();
+        PostReplyTask(base::Bind(
+            &DynamicallyInitializedMidiManagerWin::SetInputPortState,
+            base::Unretained(this), port->index(), port->info().state));
+      }
+    } else if (port->enumerate_state() == EnumerateState::Connected) {
+      // Ports that have been connected, and are going to be connected now are
+      // in |EnumerateState::Connected| state.
+      if (!port->IsConnected()) {
+        // Change state and notify it.
+        port->Connect();
+        PostReplyTask(base::Bind(
+            &DynamicallyInitializedMidiManagerWin::SetInputPortState,
+            base::Unretained(this), port->index(), port->info().state));
+      }
+    }
+  }
+  // Assign port index to new devices and move them to the port list.
+  size_t in_index = input_ports_.size();
+  for (const auto& port : new_input_ports) {
+    port->set_index(in_index++);
+    port->Connect();
+    PostReplyTask(
+        base::Bind(&DynamicallyInitializedMidiManagerWin::AddInputPort,
+                   base::Unretained(this), port->info()));
+  }
+  input_ports_.insert(input_ports_.end(),
+                      std::make_move_iterator(new_input_ports.begin()),
+                      std::make_move_iterator(new_input_ports.end()));
+
+  // Update the devie list for output ports.
+  // First, mark all registered output ports to |EnumerateState::Enumerating|.
+  for (const auto& port : output_ports_)
+    port->set_enumerate_state(EnumerateState::Enumerating);
+
+  std::vector<std::unique_ptr<OutPort>> new_output_ports;
+  const UINT num_out_devices = midiOutGetNumDevs();
+  for (UINT device_id = 0; device_id < num_out_devices; ++device_id) {
+    MIDIOUTCAPS2W caps;
+    MMRESULT result = midiOutGetDevCaps(
+        device_id, reinterpret_cast<LPMIDIOUTCAPSW>(&caps), sizeof(caps));
+    if (result != MMSYSERR_NOERROR) {
+      LOG(ERROR) << "midiOutGetDevCaps fails on device " << device_id;
+      continue;
+    }
+    std::unique_ptr<OutPort> port = base::MakeUnique<OutPort>(device_id, caps);
+    const auto& found = std::find_if(
+        output_ports_.begin(), output_ports_.end(),
+        [&port](const auto& candidate) { return *candidate == *port; });
+    if (found == output_ports_.end()) {
+      new_output_ports.push_back(std::move(port));
+    } else {
+      // Update device ID which may be changed when device list is updated.
+      (*found)->set_device_id(device_id);
+      // Found devices in the list are marked to |EnumerateState::Connected|.
+      (*found)->set_enumerate_state(EnumerateState::Connected);
+    }
+  }
+  for (const auto& port : output_ports_) {
+    if (port->enumerate_state() == EnumerateState::Enumerating) {
+      // Disconnected ports have kept staying in |EnumerateState::Enumerating|
+      // state as they were marked first.
+      if (port->IsConnected()) {
+        // Change state and notify it.
+        port->Disconnect();
+        PostReplyTask(base::Bind(
+            &DynamicallyInitializedMidiManagerWin::SetOutputPortState,
+            base::Unretained(this), port->index(), port->info().state));
+      }
+    } else if (port->enumerate_state() == EnumerateState::Connected) {
+      // Ports that have been connected, and are going to be connected now are
+      // in |EnumerateState::Connected| state.
+      if (!port->IsConnected()) {
+        // Try changing state and notify it if it was changed. Note that
+        // Connect() won't change the state on output ports for software synth.
+        port->Connect();
+        if (port->IsConnected()) {
+          PostReplyTask(base::Bind(
+              &DynamicallyInitializedMidiManagerWin::SetOutputPortState,
+              base::Unretained(this), port->index(), port->info().state));
+        }
+      }
+    }
+  }
+  // Assign port index to new devices and move them to the port list.
+  size_t out_index = output_ports_.size();
+  for (const auto& port : new_output_ports) {
+    port->set_index(out_index++);
+    port->Connect();
+    PostReplyTask(
+        base::Bind(&DynamicallyInitializedMidiManagerWin::AddOutputPort,
+                   base::Unretained(this), port->info()));
+  }
+  output_ports_.insert(output_ports_.end(),
+                       std::make_move_iterator(new_output_ports.begin()),
+                       std::make_move_iterator(new_output_ports.end()));
+
+  // TODO(toyoshim): This method may run before internal MIDI device lists that
+  // Windows manages were updated. This may be because MIDI driver may be loaded
+  // after the raw device list was updated. To avoid this problem, we may want
+  // to retry device check later if no changes are detected here.
+}
+
+}  // namespace midi