Remove VideoRenderer::NaturalSizeChangedCB.

Instead WebMediaPlayerImpl detects frame size changes and bypasses the previous plumbing through Pipeline.

Includes a workaround for http://crbug.com/334325 by calling Demuxer::GetStream() while not holding Pipeline::lock_.

BUG=319000

Review URL: https://codereview.chromium.org/135403005

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

5 files changed, 25 insertions, 49 deletions

diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 7b08564..96c5604 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -111,11 +111,10 @@ class MockVideoRenderer : public VideoRenderer {
   virtual ~MockVideoRenderer();
 
   // VideoRenderer implementation.
-  MOCK_METHOD9(Initialize, void(DemuxerStream* stream,
+  MOCK_METHOD8(Initialize, void(DemuxerStream* stream,
                                 const PipelineStatusCB& init_cb,
                                 const StatisticsCB& statistics_cb,
                                 const TimeCB& time_cb,
-                                const NaturalSizeChangedCB& size_changed_cb,
                                 const base::Closure& ended_cb,
                                 const PipelineStatusCB& error_cb,
                                 const TimeDeltaCB& get_time_cb,
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index aefaedc8..824585f 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -40,7 +40,6 @@ Pipeline::Pipeline(
       running_(false),
       did_loading_progress_(false),
       total_bytes_(0),
-      natural_size_(0, 0),
       volume_(1.0f),
       playback_rate_(0.0f),
       clock_(new Clock(&default_tick_clock_)),
@@ -189,10 +188,9 @@ int64 Pipeline::GetTotalBytes() const {
   return total_bytes_;
 }
 
-void Pipeline::GetNaturalVideoSize(gfx::Size* out_size) const {
-  CHECK(out_size);
+gfx::Size Pipeline::GetInitialNaturalSize() const {
   base::AutoLock auto_lock(lock_);
-  *out_size = natural_size_;
+  return initial_natural_size_;
 }
 
 bool Pipeline::DidLoadingProgress() const {
@@ -703,15 +701,6 @@ void Pipeline::AddBufferedTimeRange(base::TimeDelta start,
   did_loading_progress_ = true;
 }
 
-void Pipeline::OnNaturalVideoSizeChanged(const gfx::Size& size) {
-  DCHECK(IsRunning());
-  media_log_->AddEvent(media_log_->CreateVideoSizeSetEvent(
-      size.width(), size.height()));
-
-  base::AutoLock auto_lock(lock_);
-  natural_size_ = size;
-}
-
 void Pipeline::OnAudioRendererEnded() {
   // Force post to process ended tasks after current execution frame.
   task_runner_->PostTask(FROM_HERE, base::Bind(
@@ -985,13 +974,18 @@ void Pipeline::InitializeAudioRenderer(const PipelineStatusCB& done_cb) {
 void Pipeline::InitializeVideoRenderer(const PipelineStatusCB& done_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
+  // Get an initial natural size so we have something when we signal
+  // the kHaveMetadata buffering state.
+  //
+  // TODO(acolwell): We have to query demuxer outside of the lock to prevent a
+  // deadlock between ChunkDemuxer and Pipeline. See http://crbug.com/334325 for
+  // ideas on removing locking from ChunkDemuxer.
   DemuxerStream* stream = demuxer_->GetStream(DemuxerStream::VIDEO);
-
+  gfx::Size initial_natural_size =
+      stream->video_decoder_config().natural_size();
   {
-    // Get an initial natural size so we have something when we signal
-    // the kHaveMetadata buffering state.
     base::AutoLock l(lock_);
-    natural_size_ = stream->video_decoder_config().natural_size();
+    initial_natural_size_ = initial_natural_size;
   }
 
   video_renderer_ = filter_collection_->GetVideoRenderer();
@@ -1000,7 +994,6 @@ void Pipeline::InitializeVideoRenderer(const PipelineStatusCB& done_cb) {
       done_cb,
       base::Bind(&Pipeline::OnUpdateStatistics, base::Unretained(this)),
       base::Bind(&Pipeline::OnVideoTimeUpdate, base::Unretained(this)),
-      base::Bind(&Pipeline::OnNaturalVideoSizeChanged, base::Unretained(this)),
       base::Bind(&Pipeline::OnVideoRendererEnded, base::Unretained(this)),
       base::Bind(&Pipeline::SetError, base::Unretained(this)),
       base::Bind(&Pipeline::GetMediaTime, base::Unretained(this)),
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index bb91d29..9b0ebdf 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -70,8 +70,8 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost {
   // Buffering states the pipeline transitions between during playback.
   // kHaveMetadata:
   //   Indicates that the following things are known:
-  //   content duration, natural size, start time, and whether the content has
-  //   audio and/or video in supported formats.
+  //   content duration, container video size, start time, and whether the
+  //   content has audio and/or video in supported formats.
   // kPrerollCompleted:
   //   All renderers have buffered enough data to satisfy preroll and are ready
   //   to start playback.
@@ -178,10 +178,9 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost {
   // determined or can not be determined, this value is 0.
   int64 GetTotalBytes() const;
 
-  // Gets the natural size of the video output in pixel units.  If there is no
-  // video or the video has not been rendered yet, the width and height will
-  // be 0.
-  void GetNaturalVideoSize(gfx::Size* out_size) const;
+  // Get the video's initial natural size as reported by the container. Note
+  // that the natural size can change during playback.
+  gfx::Size GetInitialNaturalSize() const;
 
   // Return true if loading progress has been made since the last time this
   // method was called.
@@ -243,9 +242,6 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost {
   // Safe to call from any thread.
   void SetError(PipelineStatus error);
 
-  // Callback executed when the natural size of the video has changed.
-  void OnNaturalVideoSizeChanged(const gfx::Size& size);
-
   // Callbacks executed when a renderer has ended.
   void OnAudioRendererEnded();
   void OnVideoRendererEnded();
@@ -374,8 +370,8 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost {
   // Total size of the media.  Set by filters.
   int64 total_bytes_;
 
-  // Video's natural width and height.  Set by filters.
-  gfx::Size natural_size_;
+  // The initial natural size of the video as reported by the container.
+  gfx::Size initial_natural_size_;
 
   // Current volume level (from 0.0f to 1.0f).  This value is set immediately
   // via SetVolume() and a task is dispatched on the task runner to notify the
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index a7a8cae..3264226 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -165,7 +165,7 @@ class PipelineTest : public ::testing::Test {
 
   // Sets up expectations to allow the video renderer to initialize.
   void InitializeVideoRenderer(DemuxerStream* stream) {
-    EXPECT_CALL(*video_renderer_, Initialize(stream, _, _, _, _, _, _, _, _))
+    EXPECT_CALL(*video_renderer_, Initialize(stream, _, _, _, _, _, _, _))
         .WillOnce(RunCallback<1>(PIPELINE_OK));
     EXPECT_CALL(*video_renderer_, SetPlaybackRate(0.0f));
 
@@ -374,9 +374,8 @@ TEST_F(PipelineTest, NotStarted) {
 
   EXPECT_EQ(0, pipeline_->GetTotalBytes());
 
-  // Should always get set to zero.
-  gfx::Size size(1, 1);
-  pipeline_->GetNaturalVideoSize(&size);
+  // Should always be zero.
+  gfx::Size size = pipeline_->GetInitialNaturalSize();
   EXPECT_EQ(0, size.width());
   EXPECT_EQ(0, size.height());
 }
@@ -1058,13 +1057,13 @@ class PipelineTeardownTest : public PipelineTest {
 
     if (state == kInitVideoRenderer) {
       if (stop_or_error == kStop) {
-        EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _, _))
+        EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _))
             .WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
                             RunCallback<1>(PIPELINE_OK)));
         EXPECT_CALL(callbacks_, OnStop());
       } else {
         status = PIPELINE_ERROR_INITIALIZATION_FAILED;
-        EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _, _))
+        EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _))
             .WillOnce(RunCallback<1>(status));
       }
 
@@ -1074,7 +1073,7 @@ class PipelineTeardownTest : public PipelineTest {
       return status;
     }
 
-    EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _, _))
+    EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _))
         .WillOnce(RunCallback<1>(PIPELINE_OK));
 
     EXPECT_CALL(callbacks_, OnBufferingState(Pipeline::kHaveMetadata));
diff --git a/media/base/video_renderer.h b/media/base/video_renderer.h
index 2650221..fe46966 100644
--- a/media/base/video_renderer.h
+++ b/media/base/video_renderer.h
@@ -11,10 +11,6 @@
 #include "media/base/media_export.h"
 #include "media/base/pipeline_status.h"
 
-namespace gfx {
-class Size;
-}
-
 namespace media {
 
 class DemuxerStream;
@@ -26,9 +22,6 @@ class MEDIA_EXPORT VideoRenderer {
   // the clock should not exceed.
   typedef base::Callback<void(base::TimeDelta)> TimeCB;
 
-  // Executed when the natural size of the video has changed.
-  typedef base::Callback<void(const gfx::Size& size)> NaturalSizeChangedCB;
-
   // Used to query the current time or duration of the media.
   typedef base::Callback<base::TimeDelta()> TimeDeltaCB;
 
@@ -43,9 +36,6 @@ class MEDIA_EXPORT VideoRenderer {
   //
   // |time_cb| is executed whenever time has advanced by way of video rendering.
   //
-  // |size_changed_cb| is executed whenever the dimensions of the video has
-  // changed.
-  //
   // |ended_cb| is executed when video rendering has reached the end of stream.
   //
   // |error_cb| is executed if an error was encountered.
@@ -57,7 +47,6 @@ class MEDIA_EXPORT VideoRenderer {
                           const PipelineStatusCB& init_cb,
                           const StatisticsCB& statistics_cb,
                           const TimeCB& time_cb,
-                          const NaturalSizeChangedCB& size_changed_cb,
                           const base::Closure& ended_cb,
                           const PipelineStatusCB& error_cb,
                           const TimeDeltaCB& get_time_cb,