Web MIDI: dynamic instantiation mode support for macOS

Use TaskService to run methods on dedicated threads, and
fix potential race condition on Finalize(). It could
cause system resource leaks that results in CoreMIDI
failing on the initialization eventually.

Bug: 694138
Change-Id: I4192e93d135f6ed8d550bdb9fecd5ff912eef7d2
Reviewed-on: https://chromium-review.googlesource.com/575794Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491004}

media/midi/midi_manager_mac.h

@@ -7,13 +7,12 @@
 
 #include <CoreMIDI/MIDIServices.h>
 #include <stdint.h>
-#include <map>
-#include <string>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
 #include "media/midi/midi_export.h"
 #include "media/midi/midi_manager.h"
@@ -37,12 +36,6 @@ class MIDI_EXPORT MidiManagerMac final : public MidiManager {
                             double timestamp) override;
 
  private:
-  // Runs a closure on |client_thread_|. It starts the thread if it isn't
-  // running and the destructor isn't called.
-  // Caller can bind base::Unretained(this) to |closure| since we join
-  // |client_thread_| in the destructor.
-  void RunOnClientThread(const base::Closure& closure);
-
   // Initializes CoreMIDI on |client_thread_| asynchronously. Called from
   // StartInitialization().
   void InitializeCoreMIDI();
@@ -59,7 +52,6 @@ class MIDI_EXPORT MidiManagerMac final : public MidiManager {
   static void ReadMidiDispatch(const MIDIPacketList* packet_list,
                                void* read_proc_refcon,
                                void* src_conn_refcon);
-  virtual void ReadMidi(MIDIEndpointRef source, const MIDIPacketList *pktlist);
 
   // An internal callback that runs on MidiSendThread.
   void SendMidiData(MidiManagerClient* client,
@@ -67,25 +59,24 @@ class MIDI_EXPORT MidiManagerMac final : public MidiManager {
                     const std::vector<uint8_t>& data,
                     double timestamp);
 
-  // CoreMIDI
-  MIDIClientRef midi_client_;
-  MIDIPortRef coremidi_input_;
-  MIDIPortRef coremidi_output_;
-  std::vector<uint8_t> midi_buffer_;
+  // CoreMIDI client reference, should be protected by |midi_client_lock_|.
+  MIDIClientRef midi_client_ = 0;
+  base::Lock midi_client_lock_;
 
-  // Keeps track of the index (0-based) for each of our sources.
-  typedef std::map<MIDIEndpointRef, uint32_t> SourceMap;
-  SourceMap source_map_;
+  // Following members can be accessed without any lock on kClientTaskRunner,
+  // or on I/O thread before calling BindInstance() or after calling
+  // UnbindInstance().
 
-  // Keeps track of all destinations.
-  typedef std::vector<MIDIEndpointRef> DestinationVector;
-  DestinationVector destinations_;
+  // CoreMIDI other references.
+  MIDIPortRef midi_input_ = 0;
+  MIDIPortRef midi_output_ = 0;
+  std::vector<uint8_t> midi_buffer_;
 
-  // |client_thread_| is used to handle platform dependent operations.
-  base::Thread client_thread_;
+  // Keeps track of all sources.
+  std::vector<MIDIEndpointRef> sources_;
 
-  // Sets true on destructing object to avoid starting |client_thread_| again.
-  bool shutdown_;
+  // Keeps track of all destinations.
+  std::vector<MIDIEndpointRef> destinations_;
 
   DISALLOW_COPY_AND_ASSIGN(MidiManagerMac);
 };

media/midi/midi_manager_mac_unittest.cc

@@ -12,9 +12,11 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/synchronization/lock.h"
+#include "media/midi/midi_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace midi {
@@ -112,24 +114,22 @@ class FakeMidiManagerClient : public MidiManagerClient {
 class MidiManagerMacTest : public ::testing::Test {
  public:
   MidiManagerMacTest()
-      : manager_(new MidiManagerMac(nullptr)),
-        message_loop_(new base::MessageLoop) {}
+      : service_(base::MakeUnique<MidiService>()),
+        message_loop_(base::MakeUnique<base::MessageLoop>()) {}
   ~MidiManagerMacTest() override {
-    manager_->Shutdown();
+    service_->Shutdown();
     base::RunLoop run_loop;
     run_loop.RunUntilIdle();
   }
 
  protected:
   void StartSession(MidiManagerClient* client) {
-    manager_->StartSession(client);
-  }
-  void EndSession(MidiManagerClient* client) {
-    manager_->EndSession(client);
+    service_->StartSession(client);
   }
+  void EndSession(MidiManagerClient* client) { service_->EndSession(client); }
 
  private:
-  std::unique_ptr<MidiManager> manager_;
+  std::unique_ptr<MidiService> service_;
   std::unique_ptr<base::MessageLoop> message_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(MidiManagerMacTest);

media/midi/task_service.cc

@@ -58,6 +58,21 @@ bool TaskService::UnbindInstance() {
   return true;
 }
 
+bool TaskService::IsOnTaskRunner(RunnerId runner_id) {
+  base::AutoLock lock(lock_);
+  if (bound_instance_id_ == kInvalidInstanceId)
+    return false;
+
+  if (runner_id == kDefaultRunnerId)
+    return default_task_runner_->BelongsToCurrentThread();
+
+  size_t thread = runner_id - 1;
+  if (threads_.size() <= thread || !threads_[thread])
+    return false;
+
+  return threads_[thread]->task_runner()->BelongsToCurrentThread();
+}
+
 void TaskService::PostStaticTask(RunnerId runner_id, base::OnceClosure task) {
   {
     // Disallow to post a task when no instance is bound, so that new threads

media/midi/task_service.h

@@ -39,6 +39,9 @@ class MIDI_EXPORT TaskService final {
   bool BindInstance();
   bool UnbindInstance();
 
+  // Checks if the current thread belongs to the specified runner.
+  bool IsOnTaskRunner(RunnerId runner_id);
+
   // Posts a task to run on a specified TaskRunner. |runner_id| should be
   // kDefaultRunnerId or a positive number. If kDefaultRunnerId is specified
   // the task runs on the thread on which BindInstance() is called. Other number