Directly call ChunkDemuxer::Initialize completion callback.

media/filters/chunk_demuxer_unittest.cc

Index: media/filters/chunk_demuxer_unittest.cc
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index 0d7c9a1c0d04b668ef4789ac40576f72dcad1290..0da843d902c3907fa3f03c6eb907449056d2ab06 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -244,6 +244,10 @@ static void OnSeekDone_OKExpected(bool* called, PipelineStatus status) {
 }
 
 class ChunkDemuxerTest : public ::testing::Test {
+ public:
+  // Public method becuase test cases use it directly.

[wolenetz, 2016/09/27 21:01:14]

nit: spelling

[alokp, 2016/09/28 17:22:45]

Done.

+  MOCK_METHOD1(DemuxerInitialized, void(PipelineStatus));
+
  protected:
   enum CodecsIndex {
     AUDIO,
@@ -786,11 +790,6 @@ class ChunkDemuxerTest : public ::testing::Test {
     ASSERT_FALSE(AppendData(garbage_cluster.get(), garbage_cluster_size));
   }
 
-  void InitDoneCalled(PipelineStatus expected_status,
-                      PipelineStatus status) {
-    EXPECT_EQ(status, expected_status);
-  }
-
   PipelineStatusCB CreateInitDoneCB(const base::TimeDelta& expected_duration,
                                     PipelineStatus expected_status) {
     if (expected_duration != kNoTimestamp)
@@ -799,9 +798,9 @@ class ChunkDemuxerTest : public ::testing::Test {
   }
 
   PipelineStatusCB CreateInitDoneCB(PipelineStatus expected_status) {
-    return base::Bind(&ChunkDemuxerTest::InitDoneCalled,
-                      base::Unretained(this),
-                      expected_status);
+    EXPECT_CALL(*this, DemuxerInitialized(expected_status));

[wolenetz, 2016/10/05 23:41:24]

Good :)

+    return base::Bind(&ChunkDemuxerTest::DemuxerInitialized,
+                      base::Unretained(this));
   }
 
   enum StreamFlags {
@@ -1676,9 +1675,9 @@ TEST_F(ChunkDemuxerTest, InitSegmentSetsNeedRandomAccessPointFlag) {
 // is called before all the initialization segments are appended.
 TEST_F(ChunkDemuxerTest, Shutdown_BeforeAllInitSegmentsAppended) {
   EXPECT_CALL(*this, DemuxerOpened());
-  demuxer_->Initialize(
-      &host_, CreateInitDoneCB(
-          kDefaultDuration(), DEMUXER_ERROR_COULD_NOT_OPEN), true);
+  demuxer_->Initialize(&host_, base::Bind(&ChunkDemuxerTest::DemuxerInitialized,
+                                          base::Unretained(this)),
+                       true);
 
   EXPECT_EQ(AddId("audio", HAS_AUDIO), ChunkDemuxer::kOk);
   EXPECT_EQ(AddId("video", HAS_VIDEO), ChunkDemuxer::kOk);
@@ -1692,9 +1691,9 @@ TEST_F(ChunkDemuxerTest, Shutdown_BeforeAllInitSegmentsAppended) {
 
 TEST_F(ChunkDemuxerTest, Shutdown_BeforeAllInitSegmentsAppendedText) {
   EXPECT_CALL(*this, DemuxerOpened());
-  demuxer_->Initialize(
-      &host_, CreateInitDoneCB(
-          kDefaultDuration(), DEMUXER_ERROR_COULD_NOT_OPEN), true);
+  demuxer_->Initialize(&host_, base::Bind(&ChunkDemuxerTest::DemuxerInitialized,

[wolenetz, 2016/09/27 21:01:14]

Ditto.

[alokp, 2016/09/28 17:22:45]

ditto.

[wolenetz, 2016/10/03 21:16:23]

ditto :)

[wolenetz, 2016/10/05 23:41:23]

ignore

[alokp, 2016/10/06 05:49:17]

Acknowledged.

+                                          base::Unretained(this)),
+                       true);
 
   EXPECT_EQ(AddId("audio", HAS_AUDIO), ChunkDemuxer::kOk);
   EXPECT_EQ(AddId("video_and_text", HAS_VIDEO), ChunkDemuxer::kOk);
@@ -1765,8 +1764,8 @@ TEST_F(ChunkDemuxerTest, AppendDataAfterSeek) {
 
 // Test that parsing errors are handled for clusters appended after init.
 TEST_F(ChunkDemuxerTest, ErrorWhileParsingClusterAfterInit) {
+  InSequence s;
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));
-  ASSERT_TRUE(AppendCluster(kDefaultFirstCluster()));
 
   EXPECT_MEDIA_LOG(StreamParsingFailed());
   EXPECT_CALL(host_, OnDemuxerError(CHUNK_DEMUXER_ERROR_APPEND_FAILED));
@@ -3221,7 +3220,9 @@ TEST_F(ChunkDemuxerTest, EndOfStreamStillSetAfterSeek) {
 
 TEST_F(ChunkDemuxerTest, GetBufferedRangesBeforeInitSegment) {
   EXPECT_CALL(*this, DemuxerOpened());
-  demuxer_->Initialize(&host_, CreateInitDoneCB(PIPELINE_OK), true);
+  demuxer_->Initialize(&host_, base::Bind(&ChunkDemuxerTest::DemuxerInitialized,

[wolenetz, 2016/09/27 21:01:14]

Where is *lack* of error on CDT::DemuxerInitialized now verified?

[alokp, 2016/09/28 17:22:45]

This again was silently not getting verified because the test completes before
Demuxer initializes. I found about this after I added EXPECT_CALL in
CreateInitDoneCB, which was not getting satisfied.

[wolenetz, 2016/10/03 21:16:23]

I wonder if this could be fixed with a test class destructor calling
RunUntilIdle() on the media thread. I would like positive verification of
expected initialization results, or at least lack of any error being dropped too
early by early test completion.

[alokp, 2016/10/04 20:48:40]

RunUntilIdle does not help here because init_cb is not even posted. The test
exits while demuxer was waiting for initialization segments to be appended.

out/Debug/media_unittests
--gtest_filter=ChunkDemuxerTest.GetBufferedRangesBeforeInitSegment
--vmodule=*/media/*=4

[ RUN      ] ChunkDemuxerTest.GetBufferedRangesBeforeInitSegment
[66548:66548:1004/134303:7701287375808:VERBOSE1:chunk_demuxer.cc(430)] Init()
[66548:66548:1004/134303:7701287375868:VERBOSE1:chunk_demuxer.cc(1045)]
ChunkDemuxer::ChangeState_Locked() : 0 -> 1
[66548:66548:1004/134303:7701287375948:VERBOSE1:chunk_demuxer.cc(582)] AddId
id=audio mime_type=audio/webm codecs=vorbis
[66548:66548:1004/134303:7701287376374:VERBOSE2:frame_processor.cc(169)]
FrameProcessor()
[66548:66548:1004/134303:7701287376450:VERBOSE1:webm_stream_parser.cc(121)]
ChangeState() : 0 -> 1
[66548:66548:1004/134303:7701287376502:VERBOSE1:chunk_demuxer.cc(582)] AddId
id=video mime_type=video/webm codecs=vp8
[66548:66548:1004/134303:7701287376567:VERBOSE2:frame_processor.cc(169)]
FrameProcessor()
[66548:66548:1004/134303:7701287376632:VERBOSE1:webm_stream_parser.cc(121)]
ChangeState() : 0 -> 1
[66548:66548:1004/134303:7701287376733:VERBOSE1:chunk_demuxer.cc(1021)]
Shutdown()
[66548:66548:1004/134303:7701287376768:VERBOSE1:chunk_demuxer.cc(1045)]
ChunkDemuxer::ChangeState_Locked() : 1 -> 5
[66548:66548:1004/134303:7701287376804:VERBOSE2:frame_processor.cc(174)]
~FrameProcessor()
[66548:66548:1004/134303:7701287376852:VERBOSE2:frame_processor.cc(174)]
~FrameProcessor()
../../media/filters/chunk_demuxer_unittest.cc:699: Failure
Actual function call count doesn't match EXPECT_CALL(*this,
DemuxerInitialized(expected_status))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
[  FAILED  ] ChunkDemuxerTest.GetBufferedRangesBeforeInitSegment (2 ms)
[----------] 1 test from ChunkDemuxerTest (2 ms total)

[wolenetz, 2016/10/05 23:41:24]

I see. My comment was that RunUntilIdle() might let unexpected posted error cb
to fire and get caught by strictmock, but that's reasonably out of scope of this
CL. (I wasn't asking for EXPECT_CALL(init_cb_ called with some status) to be
added back to this test, since that cb shouldn't get called until after any/all
expected (1 per each AddId) initial init segments are successfully appended (or
some error), unlike what I think the most recent previous reply on this comment
was doing.)

[alokp, 2016/10/06 05:49:17]

Acknowledged.

+                                          base::Unretained(this)),
+                       true);
   ASSERT_EQ(AddId("audio", HAS_AUDIO), ChunkDemuxer::kOk);
   ASSERT_EQ(AddId("video", HAS_VIDEO), ChunkDemuxer::kOk);