Don't clear the list of source buffers in ChunkDemuxer::Shutdown()

Clearing the stream_parser_map_ (which represents the list of source buffers) creates an inconsistent state where source buffers should still be available to WebKit (JavaScript) but they have been removed internally. Source buffers should only be removed when RemoveId() is called or upon destruction. 

This change was originally committed as http://src.chromium.org/viewvc/chrome?view=rev&revision=148149 (Patch Set 1) and then reverted because it exposed a memory leak. Turns out the StreamParser was holding on to a reference to the ChunkDemuxer until after the initialization in order to ensure the init callback got called. So for tests that never append an initialization segment, the StreamParser reference prevented the ChunkDemuxer from being deconstructed, causing the memory leak. Before this patch, Shutdown() would remove all the StreamParsers so this wasn't a problem. 

A subsequent patch will not allow StreamParser to hang on to its ChunkDemuxer reference. This is ok because if ChunkDemuxer gets deconstructed, the callback is no longer relevant. 

For reference, the memory leak looks like this: 
http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20Mac%20%28valgrind%29%281%29/builds/11523/steps/memory%20test%3A%20media/logs/stdio 

BUG=138578
, WK88949 
TEST=http/tests/media/media-source/video-media-source-add-and-remove-ids.html, ChunkDemuxerTest.TestEndOfStreamWithNoAppends


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

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

2 files changed, 16 insertions, 7 deletions

diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index e28a2d3..c548467 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -655,7 +655,7 @@ ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
   CHECK(stream_parser.get());
 
   stream_parser->Init(
-      base::Bind(&ChunkDemuxer::OnStreamParserInitDone, this),
+      base::Bind(&ChunkDemuxer::OnStreamParserInitDone, base::Unretained(this)),
       base::Bind(&ChunkDemuxer::OnNewConfigs, base::Unretained(this),
                  has_audio, has_video),
       audio_cb,
@@ -857,12 +857,6 @@ void ChunkDemuxer::Shutdown() {
     if (video_)
       video_->Shutdown();
 
-    for (StreamParserMap::iterator it = stream_parser_map_.begin();
-         it != stream_parser_map_.end(); ++it) {
-      delete it->second;
-    }
-    stream_parser_map_.clear();
-
     ChangeState_Locked(SHUTDOWN);
   }
 
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index a05d234..c364bc3 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -905,6 +905,21 @@ TEST_F(ChunkDemuxerTest, TestEOSDuringInit) {
 }
 
 TEST_F(ChunkDemuxerTest, TestEndOfStreamWithNoAppend) {
+  EXPECT_CALL(*client_, DemuxerOpened(_));
+  demuxer_->Initialize(
+      &host_, NewExpectedStatusCB(DEMUXER_ERROR_COULD_NOT_OPEN));
+
+  ASSERT_EQ(AddId(), ChunkDemuxer::kOk);
+
+  CheckExpectedRanges("{ }");
+  demuxer_->EndOfStream(PIPELINE_OK);
+  ShutdownDemuxer();
+  CheckExpectedRanges("{ }");
+  demuxer_->RemoveId(kSourceId);
+  demuxer_ = NULL;
+}
+
+TEST_F(ChunkDemuxerTest, TestEndOfStreamWithNoMediaAppend) {
   ASSERT_TRUE(InitDemuxer(true, true, false));
 
   CheckExpectedRanges("{ }");