WebMediaPlayerMSCompositor cleanup

In WebMediaPlayerMSCompositor, GetCurentFrame() no longer has side effect.
This CL removes GetCurrentFrameWithoutUpdatingStatistics() and uses
GetCurrentFrame() instead. This CL also removes an outdated comment.

Bug: 948472

Change-Id: If18ccb69cfe47a8944bdb91a1a8cf72f91d34985
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1542783Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652505}

content/renderer/media/stream/webmediaplayer_ms.cc

@@ -699,8 +699,7 @@ blink::WebSize WebMediaPlayerMS::NaturalSize() const {
 
 blink::WebSize WebMediaPlayerMS::VisibleRect() const {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  scoped_refptr<media::VideoFrame> video_frame =
-      compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  scoped_refptr<media::VideoFrame> video_frame = compositor_->GetCurrentFrame();
   if (!video_frame)
     return blink::WebSize();
 
@@ -781,8 +780,7 @@ void WebMediaPlayerMS::Paint(cc::PaintCanvas* canvas,
   DVLOG(3) << __func__;
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  const scoped_refptr<media::VideoFrame> frame =
-      compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  const scoped_refptr<media::VideoFrame> frame = compositor_->GetCurrentFrame();
 
   media::Context3D context_3d;
   gpu::ContextSupport* context_support = nullptr;
@@ -954,8 +952,7 @@ bool WebMediaPlayerMS::CopyVideoTextureToPlatformTexture(
   TRACE_EVENT0("media", "copyVideoTextureToPlatformTexture");
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  scoped_refptr<media::VideoFrame> video_frame =
-      compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  scoped_refptr<media::VideoFrame> video_frame = compositor_->GetCurrentFrame();
 
   if (!video_frame.get() || !video_frame->HasTextures())
     return false;
@@ -989,8 +986,7 @@ bool WebMediaPlayerMS::CopyVideoYUVDataToPlatformTexture(
   TRACE_EVENT0("media", "copyVideoYUVDataToPlatformTexture");
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-  scoped_refptr<media::VideoFrame> video_frame =
-      compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  scoped_refptr<media::VideoFrame> video_frame = compositor_->GetCurrentFrame();
 
   if (!video_frame)
     return false;
@@ -1028,7 +1024,7 @@ bool WebMediaPlayerMS::TexImageImpl(TexImageFunctionID functionID,
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
   const scoped_refptr<media::VideoFrame> video_frame =
-      compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+      compositor_->GetCurrentFrame();
   if (!video_frame || !video_frame->IsMappable() ||
       video_frame->HasTextures() ||
       video_frame->format() != media::PIXEL_FORMAT_Y16) {

content/renderer/media/stream/webmediaplayer_ms_compositor.cc

@@ -386,7 +386,6 @@ bool WebMediaPlayerMSCompositor::HasCurrentFrame() {
 
 scoped_refptr<media::VideoFrame> WebMediaPlayerMSCompositor::GetCurrentFrame() {
   DVLOG(3) << __func__;
-  DCHECK(video_frame_compositor_task_runner_->BelongsToCurrentThread());
   base::AutoLock auto_lock(current_frame_lock_);
   TRACE_EVENT_INSTANT1("media", "WebMediaPlayerMSCompositor::GetCurrentFrame",
                        TRACE_EVENT_SCOPE_THREAD, "Timestamp",
@@ -403,16 +402,6 @@ void WebMediaPlayerMSCompositor::PutCurrentFrame() {
   current_frame_rendered_ = true;
 }
 
-scoped_refptr<media::VideoFrame>
-WebMediaPlayerMSCompositor::GetCurrentFrameWithoutUpdatingStatistics() {
-  DVLOG(3) << __func__;
-  base::AutoLock auto_lock(current_frame_lock_);
-  if (!render_started_)
-    return nullptr;
-
-  return current_frame_;
-}
-
 void WebMediaPlayerMSCompositor::StartRendering() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   {

content/renderer/media/stream/webmediaplayer_ms_compositor.h

@@ -58,10 +58,6 @@ class CONTENT_EXPORT WebMediaPlayerMSCompositor
     : public cc::VideoFrameProvider,
       public base::RefCountedDeleteOnSequence<WebMediaPlayerMSCompositor> {
  public:
-  // This |url| represents the media stream we are rendering. |url| is used to
-  // find out what web stream this WebMediaPlayerMSCompositor is playing, and
-  // together with flag "--disable-rtc-smoothness-algorithm" determine whether
-  // we enable algorithm or not.
   WebMediaPlayerMSCompositor(
       scoped_refptr<base::SingleThreadTaskRunner> task_runner,
       scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
@@ -106,13 +102,6 @@ class CONTENT_EXPORT WebMediaPlayerMSCompositor
   scoped_refptr<media::VideoFrame> GetCurrentFrame() override;
   void PutCurrentFrame() override;
 
-  // Return the current frame being rendered.
-  // Difference between GetCurrentFrame(): GetCurrentFrame() is designed for
-  // chrome compositor to pull frame from WebMediaPlayerMSCompositor, and thus
-  // calling GetCurrentFrame() will affect statistics like |dropped_frames_|
-  // etc. Calling this function has no side effect.
-  scoped_refptr<media::VideoFrame> GetCurrentFrameWithoutUpdatingStatistics();
-
   void StartRendering();
   void StopRendering();
   void ReplaceCurrentFrameWithACopy();

content/renderer/media/stream/webmediaplayer_ms_unittest.cc

@@ -979,10 +979,10 @@ TEST_P(WebMediaPlayerMSTest, PlayThenPause) {
     EXPECT_CALL(*this, DoStopRendering());
 
   player_->Pause();
-  auto prev_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  auto prev_frame = compositor_->GetCurrentFrame();
   message_loop_controller_.RunAndWaitForStatus(
       media::PipelineStatus::PIPELINE_OK);
-  auto after_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  auto after_frame = compositor_->GetCurrentFrame();
   EXPECT_EQ(prev_frame->timestamp(), after_frame->timestamp());
   testing::Mock::VerifyAndClearExpectations(this);
 
@@ -1030,10 +1030,10 @@ TEST_P(WebMediaPlayerMSTest, PlayThenPauseThenPlay) {
     EXPECT_CALL(*this, DoStopRendering());
 
   player_->Pause();
-  auto prev_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  auto prev_frame = compositor_->GetCurrentFrame();
   message_loop_controller_.RunAndWaitForStatus(
       media::PipelineStatus::PIPELINE_OK);
-  auto after_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  auto after_frame = compositor_->GetCurrentFrame();
   EXPECT_EQ(prev_frame->timestamp(), after_frame->timestamp());
   testing::Mock::VerifyAndClearExpectations(this);
 
@@ -1044,10 +1044,10 @@ TEST_P(WebMediaPlayerMSTest, PlayThenPauseThenPlay) {
     EXPECT_CALL(*this, DoStartRendering());
 
   player_->Play();
-  prev_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  prev_frame = compositor_->GetCurrentFrame();
   message_loop_controller_.RunAndWaitForStatus(
       media::PipelineStatus::PIPELINE_OK);
-  after_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  after_frame = compositor_->GetCurrentFrame();
   EXPECT_NE(prev_frame->timestamp(), after_frame->timestamp());
   testing::Mock::VerifyAndClearExpectations(this);
 
@@ -1232,18 +1232,18 @@ TEST_P(WebMediaPlayerMSTest, BackgroundRendering) {
     EXPECT_CALL(*this, DoDidReceiveFrame()).Times(testing::AtLeast(1));
 
   SetBackgroundRendering(true);
-  auto prev_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  auto prev_frame = compositor_->GetCurrentFrame();
   message_loop_controller_.RunAndWaitForStatus(
       media::PipelineStatus::PIPELINE_OK);
-  auto after_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  auto after_frame = compositor_->GetCurrentFrame();
   EXPECT_NE(prev_frame->timestamp(), after_frame->timestamp());
 
   // Switch to foreground rendering.
   SetBackgroundRendering(false);
-  prev_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  prev_frame = compositor_->GetCurrentFrame();
   message_loop_controller_.RunAndWaitForStatus(
       media::PipelineStatus::PIPELINE_OK);
-  after_frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  after_frame = compositor_->GetCurrentFrame();
   EXPECT_NE(prev_frame->timestamp(), after_frame->timestamp());
   testing::Mock::VerifyAndClearExpectations(this);
 
@@ -1324,7 +1324,7 @@ TEST_P(WebMediaPlayerMSTest, CreateHardwareFrames) {
   std::move(frame_ready_cbs_[0]).Run();
   base::RunLoop().RunUntilIdle();
 
-  auto frame = compositor_->GetCurrentFrameWithoutUpdatingStatistics();
+  auto frame = compositor_->GetCurrentFrame();
   ASSERT_TRUE(frame != nullptr);
   testing::Mock::VerifyAndClearExpectations(this);