Moved DCHECK() to prevent NULL deref of codec_context_.

media/filters/audio_file_reader.cc

Index: media/filters/audio_file_reader.cc
diff --git a/media/filters/audio_file_reader.cc b/media/filters/audio_file_reader.cc
index 706a9aac45b727c18c7639b7905e7543a68c102a..a2d0c6839934519b7adf77a083178c45d9ef14bc 100644
--- a/media/filters/audio_file_reader.cc
+++ b/media/filters/audio_file_reader.cc
@@ -112,12 +112,13 @@ void AudioFileReader::Close() {
 
 bool AudioFileReader::Read(const std::vector<float*>& audio_data,
                            size_t number_of_frames) {
+  DCHECK(format_context_ && codec_context_ && codec_);
+
   size_t channels = this->channels();
   DCHECK_EQ(audio_data.size(), channels);
   if (audio_data.size() != channels)
     return false;
 
-  DCHECK(format_context_ && codec_context_ && codec_);
   if (!format_context_ || !codec_context_ || !codec_) {

[Tyler Breisacher (Chromium), 2012/07/25 21:53:03]

Isn't if () redundant then? Maybe it should just say:

DCHECK(format_context_ && codec_context_ && codec_) << "AudioFileReader::Read()
: reader is not opened!";

[Kyle Horimoto, 2012/07/25 21:57:25]

If we did that, we wouldn't hit the "return false;". Don't think we should be
changing the behavior in this case.

[Tyler Breisacher (Chromium), 2012/07/25 22:14:23]

The DCHECK is saying "none of these should ever be null" and this line is saying
"if one of these is null, then do the following". Check with someone who knows
this file better, to determine whether this is something that should NEVER
happen, or not.

     DLOG(WARNING) << "AudioFileReader::Read() : reader is not opened!";
     return false;