MidiScheduler binds to task runner of constructing thread

media/midi/midi_scheduler.h

 // Copyright 2015 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.
 
 #ifndef MEDIA_MIDI_MIDI_SCHEDULER_H_
 #define MEDIA_MIDI_MIDI_SCHEDULER_H_
 
 #include <stddef.h>
 
 #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 {
 namespace midi {
 
 class MidiManager;
 class MidiManagerClient;
 
 // TODO(crbug.com/467442): Make tasks cancelable per client.
 class MIDI_EXPORT MidiScheduler final {
  public:
   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,
                         const base::Closure& closure);
 
  private:
   void InvokeClosure(MidiManagerClient* client,
                      size_t length,
                      const base::Closure& closure);
 
   // 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);
 };
 
 }  // namespace midi
 }  // namespace media
 
 #endif  // MEDIA_MIDI_MIDI_SCHEDULER_H_