Improve input handling for WaveAudioHandler.

media/audio/simple_sources.cc

Index: media/audio/simple_sources.cc
diff --git a/media/audio/simple_sources.cc b/media/audio/simple_sources.cc
index 877f9fd5bdac20866823b14574e1c46e03c7e23e..9b150233d4fb8d492ff13bff41cb7d61d83f7cbc 100644
--- a/media/audio/simple_sources.cc
+++ b/media/audio/simple_sources.cc
@@ -21,43 +21,32 @@ namespace media {
 // return a null pointer if we can't read the file or if it's malformed. The
 // caller takes ownership of the returned data. The size of the data is stored
 // in |read_length|.
-static scoped_ptr<uint8[]> ReadWavFile(const base::FilePath& wav_filename,
-                                       size_t* file_length) {
+bool ReadWavFile(base::StringPiece* data_out,
+                 const base::FilePath& wav_filename) {
+  DCHECK(data_out);
   base::File wav_file(
       wav_filename, base::File::FLAG_OPEN | base::File::FLAG_READ);
   if (!wav_file.IsValid()) {
     LOG(ERROR) << "Failed to read " << wav_filename.value()
                << " as input to the fake device.";
-    return nullptr;
+    return false;
   }
 
   size_t wav_file_length = wav_file.GetLength();
   if (wav_file_length == 0u) {
     LOG(ERROR) << "Input file to fake device is empty: "
                << wav_filename.value();
-    return nullptr;
+    return false;
   }
 
-  uint8* wav_file_data = new uint8[wav_file_length];
-  size_t read_bytes = wav_file.Read(0, reinterpret_cast<char*>(wav_file_data),
-                                    wav_file_length);
+  scoped_ptr<char> data = make_scoped_ptr(new char[wav_file_length]);
+  size_t read_bytes = wav_file.Read(0, data.get(), wav_file_length);
   if (read_bytes != wav_file_length) {
     LOG(ERROR) << "Failed to read all bytes of " << wav_filename.value();
-    return nullptr;
+    return false;
   }
-  *file_length = wav_file_length;
-  return scoped_ptr<uint8[]>(wav_file_data);
-}
-
-// Opens |wav_filename|, reads it and loads it as a wav file. This function will
-// bluntly trigger CHECKs if we can't read the file or if it's malformed.
-static scoped_ptr<WavAudioHandler> CreateWavAudioHandler(
-    const base::FilePath& wav_filename, const uint8* wav_file_data,
-    size_t wav_file_length, const AudioParameters& expected_params) {
-  base::StringPiece wav_data(reinterpret_cast<const char*>(wav_file_data),
-                             wav_file_length);
-  scoped_ptr<WavAudioHandler> wav_audio_handler(new WavAudioHandler(wav_data));
-  return wav_audio_handler.Pass();
+  data_out->set(data.release(), wav_file_length);

[tommi (sloooow) - chröme, 2015/11/20 09:45:23]

how is this memory freed?

[DaleCurtis, 2015/11/20 17:31:39]

Drive by, StringPiece does not make a copy of the data, so if you free this
you'll now have a use-after-free :)

Presumably you're switching away from scoped_ptr because you want the size
information? I think instead you want something like
scoped_ptr<std::vector<uint8_t>> to get length and avoid copies.

[tommi (sloooow) - chröme, 2015/11/20 18:01:27]

At the moment I don't think this is ever freed actually.

I was thinking something along those lines, but the CL has already grown well
beyond the initial intent :)

[slan, 2015/11/20 18:09:36]

Thanks, Dale. This was a case of me not understanding StringPiece. I've changed
this back to its original signature (return a scoped_ptr<char[]> and taking
size_t* out param.

tommi@, if you would prefer scoped_ptr<vector<char>> to eliminate the size
param, we can do that too. I am impartial.

+  return true;
 }
 
 // These values are based on experiments for local-to-local
@@ -172,15 +161,19 @@ void FileSource::LoadWavFile(const base::FilePath& path_to_wav_file) {
     return;
 
   // Read the file, and put its data in a scoped_ptr so it gets deleted later.
-  size_t file_length = 0;
-  wav_file_data_ = ReadWavFile(path_to_wav_file, &file_length);
-  if (!wav_file_data_) {
+  base::StringPiece data;
+  if (!ReadWavFile(&data, path_to_wav_file)) {
     load_failed_ = true;
     return;
   }
 
-  wav_audio_handler_ = CreateWavAudioHandler(
-      path_to_wav_file, wav_file_data_.get(), file_length, params_);
+  // Attempt to create a handler with this data. If the data is invalid, return.
+  wav_audio_handler_ = WavAudioHandler::Create(data);
+  if (!wav_audio_handler_) {
+    LOG(ERROR) << "WAV data could be read but is not valid";
+    load_failed_ = true;
+    return;
+  }
 
   // Hook us up so we pull in data from the file into the converter. We need to
   // modify the wav file's audio parameters since we'll be reading small slices