Remove dropped frame counting and task posting from VideoFrameCompositor.

content/renderer/media/video_frame_compositor.cc

Index: content/renderer/media/video_frame_compositor.cc
diff --git a/content/renderer/media/video_frame_compositor.cc b/content/renderer/media/video_frame_compositor.cc
index e991357bb370b7f2f30b3778e222ca66a8100f98..9d534da7ff4229b4e06fdfcd835c9f84680fbacd 100644
--- a/content/renderer/media/video_frame_compositor.cc
+++ b/content/renderer/media/video_frame_compositor.cc
@@ -4,11 +4,6 @@
 
 #include "content/renderer/media/video_frame_compositor.h"
 
-#include "base/bind.h"
-#include "base/location.h"
-#include "base/single_thread_task_runner.h"
-#include "cc/layers/video_frame_provider.h"
-#include "content/renderer/render_thread_impl.h"
 #include "media/base/video_frame.h"
 
 namespace content {
@@ -32,141 +27,52 @@ static bool IsOpaque(const scoped_refptr<media::VideoFrame>& frame) {
   return false;
 }
 
-class VideoFrameCompositor::Internal : public cc::VideoFrameProvider {
- public:
-  Internal(
-      const scoped_refptr<base::SingleThreadTaskRunner>& compositor_task_runner,
-      const base::Callback<void(gfx::Size)>& natural_size_changed_cb,
-      const base::Callback<void(bool)>& opacity_changed_cb)
-      : compositor_task_runner_(compositor_task_runner),
-        natural_size_changed_cb_(natural_size_changed_cb),
-        opacity_changed_cb_(opacity_changed_cb),
-        client_(NULL),
-        compositor_notification_pending_(false),
-        frames_dropped_before_compositor_was_notified_(0) {}
-
-  virtual ~Internal() {
-    if (client_)
-      client_->StopUsingProvider();
-  }
-
-  void DeleteSoon() {
-    compositor_task_runner_->DeleteSoon(FROM_HERE, this);
-  }
-
-  void UpdateCurrentFrame(const scoped_refptr<media::VideoFrame>& frame) {
-    base::AutoLock auto_lock(lock_);
-
-    if (current_frame_ &&
-        current_frame_->natural_size() != frame->natural_size()) {
-      natural_size_changed_cb_.Run(frame->natural_size());
-    }
-
-    if (!current_frame_ || IsOpaque(current_frame_) != IsOpaque(frame)) {
-      opacity_changed_cb_.Run(IsOpaque(frame));
-    }
-
-    current_frame_ = frame;
-
-    // Count frames as dropped if and only if we updated the frame but didn't
-    // finish notifying the compositor for the previous frame.
-    if (compositor_notification_pending_) {
-      if (frames_dropped_before_compositor_was_notified_ < kuint32max)
-        ++frames_dropped_before_compositor_was_notified_;
-      return;
-    }
-
-    compositor_notification_pending_ = true;
-    compositor_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&Internal::NotifyCompositorOfNewFrame,
-                   base::Unretained(this)));
-  }
-
-  uint32 GetFramesDroppedBeforeCompositorWasNotified() {
-    base::AutoLock auto_lock(lock_);
-    return frames_dropped_before_compositor_was_notified_;
-  }
-
-  void SetFramesDroppedBeforeCompositorWasNotifiedForTesting(
-      uint32 dropped_frames) {
-    base::AutoLock auto_lock(lock_);
-    frames_dropped_before_compositor_was_notified_ = dropped_frames;
-  }
-
-  // cc::VideoFrameProvider implementation.
-  virtual void SetVideoFrameProviderClient(
-      cc::VideoFrameProvider::Client* client) OVERRIDE {
-    if (client_)
-      client_->StopUsingProvider();
-    client_ = client;
-  }
-
-  virtual scoped_refptr<media::VideoFrame> GetCurrentFrame() OVERRIDE {
-    base::AutoLock auto_lock(lock_);
-    return current_frame_;
-  }
-
-  virtual void PutCurrentFrame(const scoped_refptr<media::VideoFrame>& frame)
-      OVERRIDE {}
-
- private:
-  void NotifyCompositorOfNewFrame() {
-    base::AutoLock auto_lock(lock_);
-    compositor_notification_pending_ = false;
-    if (client_)
-      client_->DidReceiveFrame();
-  }
-
-  scoped_refptr<base::SingleThreadTaskRunner> compositor_task_runner_;
-  base::Callback<void(gfx::Size)> natural_size_changed_cb_;
-  base::Callback<void(bool)> opacity_changed_cb_;
-
-  cc::VideoFrameProvider::Client* client_;
-
-  base::Lock lock_;
-  scoped_refptr<media::VideoFrame> current_frame_;
-  bool compositor_notification_pending_;
-  uint32 frames_dropped_before_compositor_was_notified_;
-
-  DISALLOW_COPY_AND_ASSIGN(Internal);
-};
-
 VideoFrameCompositor::VideoFrameCompositor(
-    const scoped_refptr<base::SingleThreadTaskRunner>& compositor_task_runner,
     const base::Callback<void(gfx::Size)>& natural_size_changed_cb,
     const base::Callback<void(bool)>& opacity_changed_cb)
-    : internal_(new Internal(compositor_task_runner,
-                             natural_size_changed_cb,
-                             opacity_changed_cb)) {
+    : natural_size_changed_cb_(natural_size_changed_cb),
+      opacity_changed_cb_(opacity_changed_cb),
+      client_(NULL) {
 }
 
 VideoFrameCompositor::~VideoFrameCompositor() {
-  internal_->DeleteSoon();
+  if (client_)
+    client_->StopUsingProvider();
 }
 
-cc::VideoFrameProvider* VideoFrameCompositor::GetVideoFrameProvider() {
-  return internal_;
-}
-
-void VideoFrameCompositor::UpdateCurrentFrame(
-    const scoped_refptr<media::VideoFrame>& frame) {
-  internal_->UpdateCurrentFrame(frame);
+void VideoFrameCompositor::SetVideoFrameProviderClient(
+    cc::VideoFrameProvider::Client* client) {
+  if (client_)
+    client_->StopUsingProvider();
+  client_ = client;
 }
 
 scoped_refptr<media::VideoFrame> VideoFrameCompositor::GetCurrentFrame() {
-  return internal_->GetCurrentFrame();
+  base::AutoLock auto_lock(lock_);
+  return current_frame_;
 }
 
-uint32 VideoFrameCompositor::GetFramesDroppedBeforeCompositorWasNotified() {
-  return internal_->GetFramesDroppedBeforeCompositorWasNotified();
+void VideoFrameCompositor::PutCurrentFrame(
+    const scoped_refptr<media::VideoFrame>& frame) {
 }
 
-void
-VideoFrameCompositor::SetFramesDroppedBeforeCompositorWasNotifiedForTesting(
-    uint32 dropped_frames) {
-  internal_->SetFramesDroppedBeforeCompositorWasNotifiedForTesting(
-      dropped_frames);
+void VideoFrameCompositor::UpdateCurrentFrame(
+    const scoped_refptr<media::VideoFrame>& frame) {
+  base::AutoLock auto_lock(lock_);
+
+  if (current_frame_ &&
+      current_frame_->natural_size() != frame->natural_size()) {
+    natural_size_changed_cb_.Run(frame->natural_size());
+  }
+
+  if (!current_frame_ || IsOpaque(current_frame_) != IsOpaque(frame)) {
+    opacity_changed_cb_.Run(IsOpaque(frame));
+  }

[xhwang, 2014/04/28 06:42:59]

These two "if" blocks use different logic. It'd be nice to have a comment about
why.

[scherkus (not reviewing), 2014/04/28 16:59:47]

there's a comment in the .h with a TODO that talks about the inconsistency
between the two

do you feel that's adequate, or would you rather move it here / duplicate it
here?

[xhwang, 2014/04/28 17:15:34]

okay, I missed that one.

+
+  current_frame_ = frame;
+
+  if (client_)
+    client_->DidReceiveFrame();
 }
 
 }  // namespace content