MidiScheduler binds to task runner of constructing thread

media/midi/midi_scheduler.h

Index: media/midi/midi_scheduler.h
diff --git a/media/midi/midi_scheduler.h b/media/midi/midi_scheduler.h
index 17354a96a0d3b5d6c57b6b0ee074c82f15a838b8..2039280fbc56959964cbb206249163251fe82b4a 100644
--- a/media/midi/midi_scheduler.h
+++ b/media/midi/midi_scheduler.h
@@ -10,6 +10,8 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/thread_checker.h"
 #include "media/midi/midi_export.h"
 
 namespace media {
@@ -24,9 +26,9 @@ class MIDI_EXPORT MidiScheduler final {
   explicit MidiScheduler(MidiManager* manager);
   ~MidiScheduler();
 
-  // Post |closure| to the current message loop safely. The |closure| will not
-  // be invoked after MidiScheduler is deleted. AccumulateMidiBytesSent() of
-  // |client| is called internally.
+  // Post |closure| to |task_runner| safely. The |closure| will not be invoked

[Takashi Toyoshima, 2016/08/23 05:30:41]

Now that this method does not take |task_runner|, you need to rephrase it.
Also, probably we should explicitly say that only this method can be called from
any thread.

[Shao-Chuan Lee, 2016/08/23 09:36:07]

Should be |task_runner_|. Added comments.

+  // after MidiScheduler is deleted. AccumulateMidiBytesSent() of |client| is
+  // called internally.
   void PostSendDataTask(MidiManagerClient* client,
                         size_t length,
                         double timestamp,
@@ -39,6 +41,14 @@ class MIDI_EXPORT MidiScheduler final {
 
   // MidiManager should own the MidiScheduler and be alive longer.
   MidiManager* manager_;
+
+  // The TaskRunner of the thread on which the instance is constructed.

[Takashi Toyoshima, 2016/08/23 05:30:41]

Can we add some comments why it's fine that we can read task_runner_ without
lock inside PostSendDataTask?

I think it contains following points.
- task_runner_ is modified only inside ctor and dtor
- the pointer to the instance should be ensured by an external lock to be used
in multiple-threads.
- When the pointer to the instance is valid to the caller thread inside the
lock, task_runner_ is also valid.

[Shao-Chuan Lee, 2016/08/23 09:36:07]

Added comments at PostSendDataTask().

+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
+  // Ensures |weak_factory_| is destructed, and WeakPtrs are dereferenced on the
+  // same thread.
+  base::ThreadChecker thread_checker_;
+
   base::WeakPtrFactory<MidiScheduler> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MidiScheduler);