Make AudioDebugFileWriter create its own file worker thread.

media/audio/audio_debug_file_writer.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_AUDIO_AUDIO_DEBUG_FILE_WRITER_H_
 #define MEDIA_AUDIO_AUDIO_DEBUG_FILE_WRITER_H_
 
 #include <stdint.h>
 
 #include <memory>
 
 #include "base/files/file.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequence_checker.h"
 #include "base/single_thread_task_runner.h"
+#include "base/task_scheduler/post_task.h"
 #include "media/base/audio_parameters.h"
 #include "media/base/media_export.h"
 
 namespace media {
 
 class AudioBus;
 
 // Writes audio data to a 16 bit PCM WAVE file used for debugging purposes. All
 // operations are non-blocking.
 // Functions are virtual for the purpose of test mocking.
 class MEDIA_EXPORT AudioDebugFileWriter {
  public:
   // Number of channels and sample rate are used from |params|, the other
   // parameters are ignored. The number of channels in the data passed to
   // Write() must match |params|.
-  AudioDebugFileWriter(
-      const AudioParameters& params,
-      scoped_refptr<base::SingleThreadTaskRunner> file_task_runner);
+  AudioDebugFileWriter(const AudioParameters& params);
 
   virtual ~AudioDebugFileWriter();
 
   // Must be called before calling Write() for the first time after creation or
   // Stop() call. Can be called on any sequence; Write() and Stop() must be
   // called on the same sequence as Start().
   virtual void Start(const base::FilePath& file);
 
   // Must be called to finish recording. Each call to Start() requires a call to
   // Stop(). Will be automatically called on destruction.
@@ skipping 32 matching lines @@
     scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   };
 
   using AudioFileWriterUniquePtr =
       std::unique_ptr<AudioFileWriter, OnThreadDeleter>;
 
   AudioFileWriterUniquePtr file_writer_;
   base::SequenceChecker client_sequence_checker_;
 
   // The task runner to do file output operations on.
-  scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;
+  const scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_ =
+      base::CreateSingleThreadTaskRunnerWithTraits(

[gab, 2017/05/18 16:10:10]

In general we don't want to create SingleThreadTaskRunners (except with a TODO
to remove it when another top-level bug is fixed or a real explanation of why).

In this case it's just because AudioFileWriter::Create() takes a
SingleThreadTaskRunner, but looking at it it doesn't need to.

AudioFileWriter::AudioFileWriter() merely takes the task runner to assert it's
running on it, that can easily be turned into a SequencedTaskRunner but even
better than that would be to have it use a SequenceChecker to check that all of
its methods run in sequence (will need to DetachFromSequence in
AudioFileWriter::AudioFileWriter() as it's not constructed on same sequence it's
used -- see sequence_checker.h for details).

SequenceChecker + implicit AssertIOAllowed() calls from using base file methods
guarantees this class is being used in sequence with MayBlock() trait. That's
preferred and cleaner than taking a task_runner just to assume running on it.

Then there's OnThreadDeleter which could easily be made OnSequenceDeleter +
SequenceTaskRunner.

And that's it.

So I'd say, please make those changes ahead of this CL and have this one use
base::CreateSequenceTaskRunnerWithTraits() :)

[Sébastien Marchand, 2017/05/18 17:43:06]

Thanks for the detailed comment! I'm addressing this in
https://codereview.chromium.org/2891983002/

+          {base::MayBlock(), base::TaskPriority::BACKGROUND});
 
   DISALLOW_COPY_AND_ASSIGN(AudioDebugFileWriter);
 };
 
 }  // namspace media
 
 #endif  // MEDIA_AUDIO_AUDIO_DEBUG_FILE_WRITER_H_