Web MIDI: add dynamic MidiManager instantiation support for Linux

media/midi/midi_manager_alsa.cc

Index: media/midi/midi_manager_alsa.cc
diff --git a/media/midi/midi_manager_alsa.cc b/media/midi/midi_manager_alsa.cc
index d99751bff01be9161f404b4681d980b7b54d839d..2c98fd8c7b26a662a01331186b4fe980c406980e 100644
--- a/media/midi/midi_manager_alsa.cc
+++ b/media/midi/midi_manager_alsa.cc
@@ -15,6 +15,7 @@
 
 #include "base/bind.h"
 #include "base/json/json_string_value_serializer.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ -27,6 +28,7 @@
 #include "base/time/time.h"
 #include "crypto/sha2.h"
 #include "media/midi/midi_port_info.h"
+#include "media/midi/midi_service.h"
 
 namespace midi {
 
@@ -35,6 +37,10 @@ namespace {
 using mojom::PortState;
 using mojom::Result;
 
+// TODO(toyoshim): use constexpr for following const values.
+const int kEventTaskRunner = 0;
+const int kSendTaskRunner = 1;
+
 // Per-output buffer. This can be smaller, but then large sysex messages
 // will be (harmlessly) split across multiple seq events. This should
 // not have any real practical effect, except perhaps to slightly reorder
@@ -90,6 +96,20 @@ const unsigned int kCreateInputPortCaps =
 const unsigned int kCreatePortType =
     SND_SEQ_PORT_TYPE_MIDI_GENERIC | SND_SEQ_PORT_TYPE_APPLICATION;
 
+// Global variables to protect tasks running on external TaskRunners from
+// running with another MidiManagerAlsa instance. These IDs can be read
+// under one of following task_runner_lock, and can be written under both
+// task_runner_lock.
+const int kInvalidManagerId = -1;
+int g_active_manager_id = kInvalidManagerId;
+int g_next_manager_id = 0;
+
+// Protects tasks that run on external TaskRunners.
+base::LazyInstance<base::Lock> g_event_task_runner_lock =
+    LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<base::Lock> g_send_task_runner_lock =
+    LAZY_INSTANCE_INITIALIZER;
+
 int AddrToInt(int client, int port) {
   return (client << 8) | port;
 }
@@ -153,8 +173,12 @@ void SetStringIfNonEmpty(base::DictionaryValue* value,
 
 }  // namespace
 
-MidiManagerAlsa::MidiManagerAlsa()
-    : event_thread_("MidiEventThread"), send_thread_("MidiSendThread") {}
+MidiManagerAlsa::MidiManagerAlsa() {
+  base::AutoLock event_runner_lock(g_event_task_runner_lock.Get());
+  base::AutoLock send_runner_lock(g_send_task_runner_lock.Get());
+  instance_id_ = g_next_manager_id++;
+  g_active_manager_id = instance_id_;
+}
 
 MidiManagerAlsa::~MidiManagerAlsa() {
   // Take lock to ensure that the members initialized on the IO thread
@@ -168,9 +192,6 @@ MidiManagerAlsa::~MidiManagerAlsa() {
   CHECK(!decoder_);
   CHECK(!udev_);
   CHECK(!udev_monitor_);
-
-  CHECK(!send_thread_.IsRunning());
-  CHECK(!event_thread_.IsRunning());
 }
 
 void MidiManagerAlsa::StartInitialization() {
@@ -288,11 +309,9 @@ void MidiManagerAlsa::StartInitialization() {
 
   // Start processing events. Don't do this before enumeration of both
   // ALSA and udev.
-  event_thread_.Start();
-  event_thread_.task_runner()->PostTask(
-      FROM_HERE,
-      base::Bind(&MidiManagerAlsa::ScheduleEventLoop, base::Unretained(this)));
-  send_thread_.Start();
+  MidiService::GetTaskRunner(kEventTaskRunner)
+      ->PostTask(FROM_HERE, base::Bind(&MidiManagerAlsa::EventLoop,
+                                       base::Unretained(this), instance_id_));
 
   CompleteInitialization(Result::OK);
 }
@@ -309,15 +328,18 @@ void MidiManagerAlsa::Finalize() {
     event_thread_shutdown_ = true;
   }
 
-  // Stop the send thread.
-  send_thread_.Stop();
+  // Ensure that no tasks run on kSendTaskRunner.
+  base::AutoLock send_runner_lock(g_send_task_runner_lock.Get());
 
   // Close the out client. This will trigger the event thread to stop,
   // because of SND_SEQ_EVENT_CLIENT_EXIT.
   out_client_.reset();
 
-  // Wait for the event thread to stop.
-  event_thread_.Stop();
+  // Ensure that no tasks run on kEventTaskRunner.
+  base::AutoLock event_runner_lock(g_event_task_runner_lock.Get());
+
+  // All pending tasks including one blocked on a lock do nothing from now on.
+  g_active_manager_id = kInvalidManagerId;
 
   // Destruct the other stuff we initialized in StartInitialization().
   udev_monitor_.reset();
@@ -339,15 +361,12 @@ void MidiManagerAlsa::DispatchSendMidiData(MidiManagerClient* client,
     delay = std::max(time_to_send - base::TimeTicks::Now(), base::TimeDelta());
   }
 
-  send_thread_.task_runner()->PostDelayedTask(
-      FROM_HERE, base::Bind(&MidiManagerAlsa::SendMidiData,
-                            base::Unretained(this), port_index, data),
-      delay);
-
-  // Acknowledge send.
-  send_thread_.task_runner()->PostTask(
-      FROM_HERE, base::Bind(&MidiManagerAlsa::AccumulateMidiBytesSent,
-                            base::Unretained(this), client, data.size()));
+  MidiService::GetTaskRunner(kSendTaskRunner)
+      ->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&MidiManagerAlsa::SendMidiData, base::Unretained(this),
+                     instance_id_, client, port_index, data),
+          delay);
 }
 
 MidiManagerAlsa::MidiPort::Id::Id() = default;
@@ -864,9 +883,14 @@ std::string MidiManagerAlsa::AlsaCard::ExtractManufacturerString(
   return "";
 }
 
-void MidiManagerAlsa::SendMidiData(uint32_t port_index,
+void MidiManagerAlsa::SendMidiData(int instance_id,
+                                   MidiManagerClient* client,
+                                   uint32_t port_index,
                                    const std::vector<uint8_t>& data) {
-  DCHECK(send_thread_.task_runner()->BelongsToCurrentThread());
+  DCHECK(MidiService::GetTaskRunner(kSendTaskRunner)->BelongsToCurrentThread());
+  base::AutoLock lock(g_send_task_runner_lock.Get());
+  if (instance_id != g_active_manager_id)
+    return;
 
   snd_midi_event_t* encoder;
   snd_midi_event_new(kSendBufferSize, &encoder);
@@ -886,15 +910,16 @@ void MidiManagerAlsa::SendMidiData(uint32_t port_index,
     }
   }
   snd_midi_event_free(encoder);
-}
 
-void MidiManagerAlsa::ScheduleEventLoop() {
-  event_thread_.task_runner()->PostTask(
-      FROM_HERE,
-      base::Bind(&MidiManagerAlsa::EventLoop, base::Unretained(this)));
+  // Acknowledge send.
+  AccumulateMidiBytesSent(client, data.size());
 }
 
-void MidiManagerAlsa::EventLoop() {
+void MidiManagerAlsa::EventLoop(int instance_id) {
+  base::AutoLock lock(g_event_task_runner_lock.Get());

[Adam Goode, 2017/02/07 13:31:14]

Can you add a comment here (or above) explaining the logic?

It looks like we only want to run a single instance at any time, and need to
protect against the case where we are still shutting down one instance and then
starting another?

[Takashi Toyoshima, 2017/02/08 09:08:00]

Done.

+  if (instance_id != g_active_manager_id)
+    return;
+
   bool loop_again = true;
 
   struct pollfd pfd[2];
@@ -973,8 +998,11 @@ void MidiManagerAlsa::EventLoop() {
   }
 
   // Do again.
-  if (loop_again)
-    ScheduleEventLoop();
+  if (loop_again) {
+    MidiService::GetTaskRunner(kEventTaskRunner)
+        ->PostTask(FROM_HERE, base::Bind(&MidiManagerAlsa::EventLoop,
+                                         base::Unretained(this), instance_id));
+  }
 }
 
 void MidiManagerAlsa::ProcessSingleEvent(snd_seq_event_t* event,