Fix ChunkDemuxer crash & parse errors on audio-only or video-only content.

BUG=111128
TEST=ChunkDemuxerTest.TestParseErrorDuringInit, ChunkDemuxerTest.TestAudioOnlyWebMFile, ChunkDemuxerTest.TestVideoOnlyWebMFile

Review URL: https://chromiumcodereview.appspot.com/9271042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@119461 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 151 insertions, 64 deletions

diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index e4f1700..0c27733 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -254,7 +254,8 @@ ChunkDemuxer::ChunkDemuxer(ChunkDemuxerClient* client)
     : state_(WAITING_FOR_INIT),
       client_(client),
       buffered_bytes_(0),
-      seek_waits_for_data_(true) {
+      seek_waits_for_data_(true),
+      deferred_error_(PIPELINE_OK) {
   DCHECK(client);
 }
 
@@ -281,10 +282,14 @@ void ChunkDemuxer::Init(const PipelineStatusCB& cb) {
 }
 
 void ChunkDemuxer::set_host(DemuxerHost* host) {
-  DCHECK_EQ(state_, INITIALIZED);
+  DCHECK(state_ == INITIALIZED || state_ == PARSE_ERROR);
   Demuxer::set_host(host);
   host->SetDuration(duration_);
   host->SetCurrentReadPosition(0);
+  if (deferred_error_ != PIPELINE_OK) {
+    host->OnDemuxerError(deferred_error_);
+    deferred_error_ = PIPELINE_OK;
+  }
 }
 
 void ChunkDemuxer::Stop(const base::Closure& callback) {
@@ -407,6 +412,7 @@ bool ChunkDemuxer::AppendData(const uint8* data, size_t length) {
         case INITIALIZING:
           result = stream_parser_->Parse(cur, cur_size);
           if (result < 0) {
+            DCHECK_EQ(state_, INITIALIZING);
             ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
             return true;
           }
@@ -566,14 +572,20 @@ void ChunkDemuxer::ReportError_Locked(PipelineStatus error) {
       video_->Shutdown();
   }
 
-  {
+  if (!cb.is_null()) {
     base::AutoUnlock auto_unlock(lock_);
-    if (cb.is_null()) {
-      host()->OnDemuxerError(error);
-      return;
-    }
     cb.Run(error);
+    return;
   }
+
+  DemuxerHost* demuxer_host = host();
+  if (demuxer_host) {
+    base::AutoUnlock auto_unlock(lock_);
+    demuxer_host->OnDemuxerError(error);
+    return;
+  }
+
+  deferred_error_ = error;
 }
 
 void ChunkDemuxer::OnStreamParserInitDone(bool success,
diff --git a/media/filters/chunk_demuxer.h b/media/filters/chunk_demuxer.h
index 9ce5845..6732c16 100644
--- a/media/filters/chunk_demuxer.h
+++ b/media/filters/chunk_demuxer.h
@@ -96,6 +96,10 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer, public StreamParserHost {
 
   ByteQueue byte_queue_;
 
+  // Stores an error that happens after initilization but before set_host().
+  // TODO(acolwell): Remove this when http://crbug.com/111585 is fixed.
+  PipelineStatus deferred_error_;
+
   DISALLOW_COPY_AND_ASSIGN(ChunkDemuxer);
 };
 
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index 4f628d6..039d689 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -36,6 +36,13 @@ MATCHER_P(HasTimestamp, timestamp_in_ms, "") {
       arg->GetTimestamp().InMilliseconds() == timestamp_in_ms;
 }
 
+static void OnReadDone(const base::TimeDelta& expected_time,
+                       bool* called,
+                       const scoped_refptr<Buffer>& buffer) {
+  EXPECT_EQ(expected_time, buffer->GetTimestamp());
+  *called = true;
+}
+
 class MockChunkDemuxerClient : public ChunkDemuxerClient {
  public:
   MockChunkDemuxerClient() {}
@@ -149,6 +156,7 @@ class ChunkDemuxerTest : public testing::Test {
 
   void InitDoneCalled(const base::TimeDelta& expected_duration,
                       PipelineStatus expected_status,
+                      bool call_set_host,
                       PipelineStatus status) {
     EXPECT_EQ(status, expected_status);
 
@@ -156,16 +164,24 @@ class ChunkDemuxerTest : public testing::Test {
       EXPECT_CALL(mock_demuxer_host_, SetDuration(expected_duration));
       EXPECT_CALL(mock_demuxer_host_, SetCurrentReadPosition(_));
 
-      demuxer_->set_host(&mock_demuxer_host_);
+      if (call_set_host)
+        demuxer_->set_host(&mock_demuxer_host_);
     }
   }
 
   PipelineStatusCB CreateInitDoneCB(int duration,
                                     PipelineStatus expected_status) {
+    return CreateInitDoneCB(duration, expected_status, true);
+  }
+
+  PipelineStatusCB CreateInitDoneCB(int duration,
+                                    PipelineStatus expected_status,
+                                    bool call_set_host) {
     return base::Bind(&ChunkDemuxerTest::InitDoneCalled,
                       base::Unretained(this),
                       base::TimeDelta::FromMilliseconds(duration),
-                      expected_status);
+                      expected_status,
+                      call_set_host);
   }
 
   void InitDemuxer(bool has_audio, bool has_video) {
@@ -206,6 +222,66 @@ class ChunkDemuxerTest : public testing::Test {
 
   MOCK_METHOD1(Checkpoint, void(int id));
 
+  struct BufferTimestamps {
+    int video_time_ms;
+    int audio_time_ms;
+  };
+  static const int kSkip = -1;
+
+  // Test parsing a WebM file.
+  // |filename| - The name of the file in media/test/data to parse.
+  // |timestamps| - The expected timestamps on the parsed buffers.
+  //    a timestamp of kSkip indicates that a Read() call for that stream
+  //    shouldn't be made on that iteration of the loop. If both streams have
+  //    a kSkip then the loop will terminate.
+  void ParseWebMFile(const std::string& filename,
+                     const BufferTimestamps* timestamps,
+                     int duration) {
+    scoped_array<uint8> buffer;
+    int buffer_size = 0;
+
+    EXPECT_CALL(*client_, DemuxerOpened(_));
+    demuxer_->Init(CreateInitDoneCB(duration, PIPELINE_OK));
+
+    // Read a WebM file into memory and send the data to the demuxer.
+    ReadTestDataFile(filename, &buffer, &buffer_size);
+    AppendDataInPieces(buffer.get(), buffer_size, 512);
+
+    scoped_refptr<DemuxerStream> audio =
+        demuxer_->GetStream(DemuxerStream::AUDIO);
+    scoped_refptr<DemuxerStream> video =
+        demuxer_->GetStream(DemuxerStream::VIDEO);
+
+    // Verify that the timestamps on the first few packets match what we
+    // expect.
+    for (size_t i = 0;
+         (timestamps[i].audio_time_ms != kSkip ||
+          timestamps[i].video_time_ms != kSkip);
+         i++) {
+      bool audio_read_done = false;
+      bool video_read_done = false;
+
+      if (timestamps[i].audio_time_ms != kSkip) {
+        DCHECK(audio);
+        audio->Read(base::Bind(&OnReadDone,
+                               base::TimeDelta::FromMilliseconds(
+                                   timestamps[i].audio_time_ms),
+                               &audio_read_done));
+        EXPECT_TRUE(audio_read_done);
+      }
+
+      if (timestamps[i].video_time_ms != kSkip) {
+        DCHECK(video);
+        video->Read(base::Bind(&OnReadDone,
+                               base::TimeDelta::FromMilliseconds(
+                                   timestamps[i].video_time_ms),
+                               &video_read_done));
+
+        EXPECT_TRUE(video_read_done);
+      }
+    }
+  }
+
   MockDemuxerHost mock_demuxer_host_;
 
   scoped_ptr<MockChunkDemuxerClient> client_;
@@ -349,13 +425,6 @@ TEST_F(ChunkDemuxerTest, TestAppendDataBeforeInit) {
   EXPECT_FALSE(demuxer_->AppendData(info_tracks.get(), info_tracks_size));
 }
 
-static void OnReadDone(const base::TimeDelta& expected_time,
-                       bool* called,
-                       const scoped_refptr<Buffer>& buffer) {
-  EXPECT_EQ(expected_time, buffer->GetTimestamp());
-  *called = true;
-}
-
 // Make sure Read() callbacks are dispatched with the proper data.
 TEST_F(ChunkDemuxerTest, TestRead) {
   InitDemuxer(true, true);
@@ -789,56 +858,43 @@ TEST_F(ChunkDemuxerTest, TestAppendingInPieces) {
   EXPECT_TRUE(video_read_done);
 }
 
-struct BufferTimestamps {
-  int video_time;
-  int audio_time;
-};
-
-TEST_F(ChunkDemuxerTest, TestWebMFile) {
-  scoped_array<uint8> buffer;
-  int buffer_size = 0;
-
-  EXPECT_CALL(*client_, DemuxerOpened(_));
-  demuxer_->Init(CreateInitDoneCB(2744, PIPELINE_OK));
-
-  // Read a WebM file into memory and send the data to the demuxer.
-  ReadTestDataFile("bear-320x240.webm", &buffer, &buffer_size);
-  AppendDataInPieces(buffer.get(), buffer_size, 512);
-
-  scoped_refptr<DemuxerStream> audio =
-      demuxer_->GetStream(DemuxerStream::AUDIO);
-  scoped_refptr<DemuxerStream> video =
-      demuxer_->GetStream(DemuxerStream::VIDEO);
-
-  ASSERT_TRUE(audio);
-  ASSERT_TRUE(video);
-
+TEST_F(ChunkDemuxerTest, TestWebMFile_AudioAndVideo) {
   struct BufferTimestamps buffer_timestamps[] = {
     {0, 0},
     {33, 3},
     {67, 6},
     {100, 9},
     {133, 12},
+    {kSkip, kSkip},
   };
 
-  // Verify that the timestamps on the first few packets match what we
-  // expect.
-  for (size_t i = 0; i < arraysize(buffer_timestamps); i++) {
-    bool audio_read_done = false;
-    bool video_read_done = false;
-    audio->Read(base::Bind(&OnReadDone,
-                           base::TimeDelta::FromMilliseconds(
-                               buffer_timestamps[i].audio_time),
-                           &audio_read_done));
-
-    video->Read(base::Bind(&OnReadDone,
-                           base::TimeDelta::FromMilliseconds(
-                               buffer_timestamps[i].video_time),
-                           &video_read_done));
-
-    EXPECT_TRUE(audio_read_done);
-    EXPECT_TRUE(video_read_done);
-  }
+  ParseWebMFile("bear-320x240.webm", buffer_timestamps, 2744);
+}
+
+TEST_F(ChunkDemuxerTest, TestWebMFile_AudioOnly) {
+  struct BufferTimestamps buffer_timestamps[] = {
+    {kSkip, 0},
+    {kSkip, 3},
+    {kSkip, 6},
+    {kSkip, 9},
+    {kSkip, 12},
+    {kSkip, kSkip},
+  };
+
+  ParseWebMFile("bear-320x240-audio-only.webm", buffer_timestamps, 2744);
+}
+
+TEST_F(ChunkDemuxerTest, TestWebMFile_VideoOnly) {
+  struct BufferTimestamps buffer_timestamps[] = {
+    {0, kSkip},
+    {33, kSkip},
+    {67, kSkip},
+    {100, kSkip},
+    {133, kSkip},
+    {kSkip, kSkip},
+  };
+
+  ParseWebMFile("bear-320x240-video-only.webm", buffer_timestamps, 2703);
 }
 
 // Verify that we output buffers before the entire cluster has been parsed.
@@ -912,4 +968,17 @@ TEST_F(ChunkDemuxerTest, TestIncrementalClusterParsing) {
   EXPECT_TRUE(video_read_done);
 }
 
+
+TEST_F(ChunkDemuxerTest, TestParseErrorDuringInit) {
+  EXPECT_CALL(*client_, DemuxerOpened(_));
+  demuxer_->Init(CreateInitDoneCB(201224, PIPELINE_OK, false));
+  AppendInfoTracks(true, true);
+
+  uint8 tmp = 0;
+  EXPECT_TRUE(demuxer_->AppendData(&tmp, 1));
+
+  EXPECT_CALL(mock_demuxer_host_, OnDemuxerError(PIPELINE_ERROR_DECODE));
+  demuxer_->set_host(&mock_demuxer_host_);
+}
+
 }  // namespace media
diff --git a/media/webm/webm_stream_parser.cc b/media/webm/webm_stream_parser.cc
index 26decfc8..f1d6001 100644
--- a/media/webm/webm_stream_parser.cc
+++ b/media/webm/webm_stream_parser.cc
@@ -305,6 +305,7 @@ int WebMStreamParser::ParseInfoAndTracks(const uint8* data, int size) {
 
   ChangeState(PARSING_CLUSTERS);
   init_cb_.Run(true, duration);
+  init_cb_.Reset();
 
   return bytes_parsed;
 }
@@ -334,14 +335,15 @@ int WebMStreamParser::ParseCluster(const uint8* data, int size) {
   if (bytes_parsed <= 0)
     return bytes_parsed;
 
-  if (cluster_parser_->audio_buffers().empty() &&
-      cluster_parser_->video_buffers().empty())
-    return bytes_parsed;
+  const StreamParserHost::BufferQueue& audio_buffers =
+      cluster_parser_->audio_buffers();
+  const StreamParserHost::BufferQueue& video_buffers =
+      cluster_parser_->video_buffers();
 
-  if (!host_->OnAudioBuffers(cluster_parser_->audio_buffers()))
+  if (!audio_buffers.empty() && !host_->OnAudioBuffers(audio_buffers))
     return -1;
 
-  if (!host_->OnVideoBuffers(cluster_parser_->video_buffers()))
+  if (!video_buffers.empty() && !host_->OnVideoBuffers(video_buffers))
     return -1;
 
   return bytes_parsed;