Web MIDI: add dynamic MidiManager instantiation support for Linux

media/midi/midi_service.h

Index: media/midi/midi_service.h
diff --git a/media/midi/midi_service.h b/media/midi/midi_service.h
index 5b870d6ce5b2719f4389ca01d56abebf37f74a13..a36ad6150ed6a4cdbbaae6361b2edb2e191c4d6a 100644
--- a/media/midi/midi_service.h
+++ b/media/midi/midi_service.h
@@ -11,12 +11,17 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
+#include "base/threading/thread.h"
 #include "media/midi/midi_export.h"
 #include "media/midi/midi_manager.h"
 
 namespace midi {
 
+class MidiManagerAlsa;
+
 // Manages MidiManager backends.  This class expects to be constructed and
 // destructed on the browser main thread, but methods can be called on both
 // the main thread and the I/O thread.
@@ -37,14 +42,49 @@ class MIDI_EXPORT MidiService final {
   void EndSession(MidiManagerClient* client);
 
   // A client calls DispatchSendMidiData() to send MIDI data.
-  virtual void DispatchSendMidiData(MidiManagerClient* client,
-                                    uint32_t port_index,
-                                    const std::vector<uint8_t>& data,
-                                    double timestamp);
+  void DispatchSendMidiData(MidiManagerClient* client,
+                            uint32_t port_index,
+                            const std::vector<uint8_t>& data,
+                            double timestamp);
+
+ protected:

[yhirano, 2017/02/08 03:50:24]

Why is this protected? Even if you place the friend declaration in protected
area, the friend class can access private members.

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

Done.

+  friend class MidiManagerAlsa;
+
+  // Returns a SingleThreadTaskRunner reference to serve MidiManager. Each
+  // TaskRunner will be constructed on demand.
+  // MidiManager that supports the dynamic instantiation feature will use this
+  // method to post tasks that should not run on I/O. Since TaskRunners outlive
+  // MidiManager, each task should be bound with WeakPtr<> if the task is not a
+  // static method. TaskRunners will be reused when another MidiManager is
+  // instantiated.
+  static scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner(

[yhirano, 2017/02/08 03:50:24]

Can you tell me what this means? Does that mean each bound |this| should be
weak? Is so, are Unretained(this) in MidiManagerAlsa problematic?

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

Oops, later part of this comment was stale (explained about one of my prototype)

+      size_t runner_id);
 
+ private:
+  scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunnerImpl(
+      size_t runner_id);
+
+  // Holds MidiManager instance. If the dynamic instantiation feature is
+  // enabled, the MidiManager would be constructed and destructed on the I/O
+  // thread, and all MidiManager methods would be called on the I/O thread.
   std::unique_ptr<MidiManager> manager_;
+
+  // A flag to indicate if the dynamic instantiation feature is supported and
+  // actually enabled.
+  bool is_dynamic_instantiation_enabled_;

[yhirano, 2017/02/08 03:50:24]

I think you don't need to protect this member, right? Making this variable
constant would be helpful.

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

Done.

+
+  // Counts active clients to manage dynamic MidiManager instantiation.
+  size_t active_clients_;
+
+  // Protects all members above.
   base::Lock lock_;
 
+  // Threads to host SingleThreadTaskRunners.
+  std::vector<std::unique_ptr<base::Thread>> threads_;
+
+  // Protects |threads_|.
+  base::Lock threads_lock_;
+
   DISALLOW_COPY_AND_ASSIGN(MidiService);
 };