Don't process pending prefetch events if we fail to create video decoder job

media/base/android/media_source_player_unittest.cc

 // Copyright 2013 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.
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "media/base/android/media_codec_bridge.h"
@@ skipping 868 matching lines @@
   EXPECT_EQ(1, demuxer_->num_data_requests());
 
   // Simulate browser seek is done and confirm player requests more data for new
   // video decoder job.
   player_.OnDemuxerSeekDone(player_.GetCurrentTime());
   EXPECT_TRUE(GetMediaDecoderJob(false));
   EXPECT_EQ(2, demuxer_->num_data_requests());
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 }
 
+TEST_F(MediaSourcePlayerTest, SetEmptySurfaceWhileDecoding) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test player pauses if an empty surface is passed.
+  CreateNextTextureAndSetVideoSurface();
+  StartVideoDecoderJob(true);
+
+  // Send the first input chunk.
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+
+  // While the decoder is decoding, pass an empty surface.
+  gfx::ScopedJavaSurface empty_surface;
+  player_.SetVideoSurface(empty_surface.Pass());

[wolenetz, 2014/02/20 20:52:11]

Is this where the NPE previously occurred prior to this patch? Shouldn't the
player already have entered PREFETCH_DONE_EVENT_PENDING in
StartVideoDecoderJob(true), above? Then, OnPrefetchDone() is invoked by
OnDemuxerDataAvailable(), above, and PREFETCH_DONE_EVENT_PENDING is cleared and
VDJ->Decode() is invoked on the prefetched data. So at this point, no event is
pending, IIUC.

Is perhaps there some race between OnPrefetchDone()'s calling of
DecodeMoreVideo() (which could NPE if VDJ is null) and resetting the VDJ? If so,
I'm not sure this patch fixes that issue.

In general, the early ProcessPendingEvents() return condition added by the patch
looks good, but I'm not sure that it solves the problem (or how this test tests
the problem's existence and solution).

[qinmin, 2014/02/20 22:36:05]

Added a TriggerPlayerStarvation() call to let the player to have a
PREFETCH_DONE_EVENT_PENDING status after setting the empty surface.

On 2014/02/20 20:52:11, wolenetz wrote:

+
+  // Wait for the decoder job to finish decoding and be reset.
+  while (GetMediaDecoderJob(false))
+    message_loop_.RunUntilIdle();
+
+  // No seek request should have been received since the surface is empty.
+  EXPECT_EQ(0, demuxer_->num_browser_seek_requests());
+
+  // Playback resumes once a non-empty surface is passed.
+  CreateNextTextureAndSetVideoSurface();
+  EXPECT_EQ(1, demuxer_->num_browser_seek_requests());
+}
+
 TEST_F(MediaSourcePlayerTest, AudioOnlyStartAfterSeekFinish) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
   // Test audio decoder job will not start until pending seek event is handled.
   DemuxerConfigs configs = CreateAudioDemuxerConfigs(kCodecVorbis);
   player_.OnDemuxerConfigsAvailable(configs);
   EXPECT_FALSE(GetMediaDecoderJob(true));
 
   // Initiate a seek. Skip requesting element seek of renderer.
   // Instead behave as if the renderer has asked us to seek.
@@ skipping 1164 matching lines @@
 
   std::vector<std::string> codec_avc(1, "avc1");
   EXPECT_FALSE(IsTypeSupported(invalid_uuid, kL3, kVideoMp4, codec_avc));
   EXPECT_FALSE(IsTypeSupported(invalid_uuid, kL1, kVideoMp4, codec_avc));
 }
 
 // TODO(xhwang): Are these IsTypeSupported tests device specific?
 // TODO(xhwang): Add more IsTypeSupported tests.
 
 }  // namespace media