media::BindToLoop() is born, with example uses to slim down WebMediaPlayerProxy.

WMPP could use further slimming-down / flamethrowering, but this pass just removes
the lowest-hanging fruit, where WMPP is used as a silent trampoline to WMPI on
the render thread.


Review URL: https://chromiumcodereview.appspot.com/10855188

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

media/base/bind_to_loop.h

+// This file was GENERATED by command:
+//     pump.py bind_to_loop.h.pump
+// DO NOT EDIT BY HAND!!!
+
+
+// Copyright (c) 2012 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.
+
+#ifndef MEDIA_BASE_BIND_TO_LOOP_H_
+#define MEDIA_BASE_BIND_TO_LOOP_H_
+
+#include "base/bind.h"
+#include "base/callback_internal.h" // Avoid re-inventing CallbackForward.
+#include "base/location.h"
+#include "base/message_loop_proxy.h"
+
+// This is a helper utility for base::Bind()ing callbacks on to particular
+// MessageLoops.  A typical use is when |a| (of class |A|) wants to hand a
+// callback such as base::Bind(&A::AMethod, a) to |b|, but needs to ensure that
+// when |b| executes the callback, it does so on a particular MessageLoop.
+//
+// Typical usage: request to be called back on the current thread:
+// other->StartAsyncProcessAndCallMeBack(
+//    media::BindToLoop(MessageLoopProxy::current(),
+//                      base::Bind(&MyClass::MyMethod, this)));
+//
+// Note that like base::Bind(), BindToLoop() can't bind non-constant references,
+// and that *unlike* base::Bind(), BindToLoop() makes copies of its arguments,
+// and thus can't be used with arrays.
+
+namespace media {
+
+template <typename T> struct TrampolineHelper;
+
+template <>
+struct TrampolineHelper<void()> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void()>& cb) {
+    loop->PostTask(FROM_HERE, base::Bind(cb));
+  }
+};
+
+
+template <typename A1>
+struct TrampolineHelper<void(A1)> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1)>& cb, A1 a1) {
+    loop->PostTask(FROM_HERE, base::Bind(cb,
+        base::internal::CallbackForward(a1)));
+  }
+};
+
+
+template <typename A1, typename A2>
+struct TrampolineHelper<void(A1, A2)> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2)>& cb, A1 a1, A2 a2) {
+    loop->PostTask(FROM_HERE, base::Bind(cb,
+        base::internal::CallbackForward(a1),
+        base::internal::CallbackForward(a2)));
+  }
+};
+
+
+template <typename A1, typename A2, typename A3>
+struct TrampolineHelper<void(A1, A2, A3)> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2, A3)>& cb, A1 a1, A2 a2, A3 a3) {
+    loop->PostTask(FROM_HERE, base::Bind(cb,
+        base::internal::CallbackForward(a1),
+        base::internal::CallbackForward(a2),
+        base::internal::CallbackForward(a3)));
+  }
+};
+
+
+template <typename A1, typename A2, typename A3, typename A4>
+struct TrampolineHelper<void(A1, A2, A3, A4)> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2, A3, A4)>& cb, A1 a1, A2 a2, A3 a3,
+          A4 a4) {
+    loop->PostTask(FROM_HERE, base::Bind(cb,
+        base::internal::CallbackForward(a1),
+        base::internal::CallbackForward(a2),
+        base::internal::CallbackForward(a3),
+        base::internal::CallbackForward(a4)));
+  }
+};
+
+
+template <typename A1, typename A2, typename A3, typename A4, typename A5>
+struct TrampolineHelper<void(A1, A2, A3, A4, A5)> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2, A3, A4, A5)>& cb, A1 a1, A2 a2, A3 a3,
+          A4 a4, A5 a5) {
+    loop->PostTask(FROM_HERE, base::Bind(cb,
+        base::internal::CallbackForward(a1),
+        base::internal::CallbackForward(a2),
+        base::internal::CallbackForward(a3),
+        base::internal::CallbackForward(a4),
+        base::internal::CallbackForward(a5)));
+  }
+};
+
+
+template <typename A1, typename A2, typename A3, typename A4, typename A5,
+    typename A6>
+struct TrampolineHelper<void(A1, A2, A3, A4, A5, A6)> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2, A3, A4, A5, A6)>& cb, A1 a1, A2 a2,
+          A3 a3, A4 a4, A5 a5, A6 a6) {
+    loop->PostTask(FROM_HERE, base::Bind(cb,
+        base::internal::CallbackForward(a1),
+        base::internal::CallbackForward(a2),
+        base::internal::CallbackForward(a3),
+        base::internal::CallbackForward(a4),
+        base::internal::CallbackForward(a5),
+        base::internal::CallbackForward(a6)));
+  }
+};
+
+
+template <typename A1, typename A2, typename A3, typename A4, typename A5,
+    typename A6, typename A7>
+struct TrampolineHelper<void(A1, A2, A3, A4, A5, A6, A7)> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2, A3, A4, A5, A6, A7)>& cb, A1 a1, A2 a2,
+          A3 a3, A4 a4, A5 a5, A6 a6, A7 a7) {
+    loop->PostTask(FROM_HERE, base::Bind(cb,
+        base::internal::CallbackForward(a1),
+        base::internal::CallbackForward(a2),
+        base::internal::CallbackForward(a3),
+        base::internal::CallbackForward(a4),
+        base::internal::CallbackForward(a5),
+        base::internal::CallbackForward(a6),
+        base::internal::CallbackForward(a7)));
+  }
+};
+
+
+template<typename T>
+static base::Callback<T> BindToLoop(
+    const scoped_refptr<base::MessageLoopProxy>& loop,
+    const base::Callback<T>& cb) {
+  return base::Bind(&TrampolineHelper<T>::Run, loop, cb);
+}
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_BIND_TO_LOOP_H_

media/base/bind_to_loop.h.pump

+$$ This is a pump file for generating file templates.  Pump is a python
+$$ script that is part of the Google Test suite of utilities.  Description
+$$ can be found here:
+$$
+$$ http://code.google.com/p/googletest/wiki/PumpManual
+$$
+
+$$ See comment for MAX_ARITY in base/bind.h.pump.
+$var MAX_ARITY = 7
+
+// Copyright (c) 2012 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.
+
+#ifndef MEDIA_BASE_BIND_TO_LOOP_H_
+#define MEDIA_BASE_BIND_TO_LOOP_H_
+
+#include "base/bind.h"
+#include "base/callback_internal.h" // Avoid re-inventing CallbackForward.
+#include "base/location.h"
+#include "base/message_loop_proxy.h"
+
+// This is a helper utility for base::Bind()ing callbacks on to particular
+// MessageLoops.  A typical use is when |a| (of class |A|) wants to hand a
+// callback such as base::Bind(&A::AMethod, a) to |b|, but needs to ensure that
+// when |b| executes the callback, it does so on a particular MessageLoop.
+//
+// Typical usage: request to be called back on the current thread:
+// other->StartAsyncProcessAndCallMeBack(
+//    media::BindToLoop(MessageLoopProxy::current(),
+//                      base::Bind(&MyClass::MyMethod, this)));
+//
+// Note that like base::Bind(), BindToLoop() can't bind non-constant references,
+// and that *unlike* base::Bind(), BindToLoop() makes copies of its arguments,
+// and thus can't be used with arrays.
+
+namespace media {
+
+template <typename T> struct TrampolineHelper;
+
+$range ARITY 0..MAX_ARITY
+$for ARITY [[
+$range ARG 1..ARITY
+
+template <$for ARG , [[typename A$(ARG)]]>
+struct TrampolineHelper<void($for ARG , [[A$(ARG)]])> {
+  static void Run(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void($for ARG , [[A$(ARG)]])>& cb
+$if ARITY != 0 [[, ]]
+$for ARG , [[A$(ARG) a$(ARG)]]
+) {
+    loop->PostTask(FROM_HERE, base::Bind(cb
+$if ARITY != 0 [[, ]]
+$for ARG , [[base::internal::CallbackForward(a$(ARG))]]));
+  }
+};
+
+
+]]  $$ for ARITY
+
+template<typename T>
+static base::Callback<T> BindToLoop(
+    const scoped_refptr<base::MessageLoopProxy>& loop,
+    const base::Callback<T>& cb) {
+  return base::Bind(&TrampolineHelper<T>::Run, loop, cb);
+}
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_BIND_TO_LOOP_H_

media/base/bind_to_loop_unittest.cc

+// Copyright (c) 2012 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 "media/base/bind_to_loop.h"
+
+#include "base/message_loop.h"
+#include "base/synchronization/waitable_event.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+void BoundBoolSet(bool* var, bool val) {
+  *var = val;
+}
+
+void BoundBoolSetFromScopedPtr(bool* var, scoped_ptr<bool> val) {
+  *var = *val;
+}
+
+void BoundBoolSetFromConstRef(bool* var, const bool& val) {
+  *var = val;
+}
+
+void BoundIntegersSet(int* a_var, int* b_var, int a_val, int b_val) {
+  *a_var = a_val;
+  *b_var = b_val;
+}
+
+// Various tests that check that the bound function is only actually executed
+// on the message loop, not during the original Run.
+class BindToLoopTest : public ::testing::Test {
+ public:
+  BindToLoopTest() : proxy_(loop_.message_loop_proxy()) {}
+
+ protected:
+  MessageLoop loop_;
+  scoped_refptr<base::MessageLoopProxy> proxy_;
+};
+
+TEST_F(BindToLoopTest, Closure) {
+  // Test the closure is run inside the loop, not outside it.
+  base::WaitableEvent waiter(false, false);
+  base::Closure cb = BindToLoop(proxy_, base::Bind(
+      &base::WaitableEvent::Signal, base::Unretained(&waiter)));
+  cb.Run();
+  EXPECT_FALSE(waiter.IsSignaled());
+  loop_.RunUntilIdle();
+  EXPECT_TRUE(waiter.IsSignaled());
+}
+
+TEST_F(BindToLoopTest, Bool) {
+  bool bool_var = false;
+  base::Callback<void(bool)> cb = BindToLoop(proxy_, base::Bind(
+      &BoundBoolSet, &bool_var));
+  cb.Run(true);
+  EXPECT_FALSE(bool_var);
+  loop_.RunUntilIdle();
+  EXPECT_TRUE(bool_var);
+}
+
+TEST_F(BindToLoopTest, ScopedPtrBool) {
+  bool bool_val = false;
+  scoped_ptr<bool> scoped_ptr_bool(new bool(true));
+  base::Closure cb = BindToLoop(proxy_, base::Bind(
+      &BoundBoolSetFromScopedPtr, &bool_val, base::Passed(&scoped_ptr_bool)));
+  cb.Run();
+  EXPECT_FALSE(bool_val);
+  loop_.RunUntilIdle();
+  EXPECT_TRUE(bool_val);
+}
+
+TEST_F(BindToLoopTest, BoolConstRef) {
+  bool bool_var = false;
+  bool true_var = true;
+  const bool& true_ref = true_var;
+  base::Closure cb = BindToLoop(proxy_, base::Bind(
+      &BoundBoolSetFromConstRef, &bool_var, true_ref));
+  cb.Run();
+  EXPECT_FALSE(bool_var);
+  loop_.RunUntilIdle();
+  EXPECT_TRUE(bool_var);
+}
+
+TEST_F(BindToLoopTest, Integers) {
+  int a = 0;
+  int b = 0;
+  base::Callback<void(int, int)> cb = BindToLoop(proxy_, base::Bind(
+      &BoundIntegersSet, &a, &b));
+  cb.Run(1, -1);
+  EXPECT_EQ(a, 0);
+  EXPECT_EQ(b, 0);
+  loop_.RunUntilIdle();
+  EXPECT_EQ(a, 1);
+  EXPECT_EQ(b, -1);
+}
+
+}  // namespace media

media/media.gyp

@@ -149,6 +149,7 @@
         'base/audio_renderer_mixer.h',
         'base/audio_renderer_mixer_input.cc',
         'base/audio_renderer_mixer_input.h',
+        'base/bind_to_loop.h',
         'base/bitstream_buffer.h',
         'base/bit_reader.cc',
         'base/bit_reader.h',
@@ -702,6 +703,7 @@
         'base/audio_renderer_mixer_input_unittest.cc',
         'base/audio_renderer_mixer_unittest.cc',
         'base/bit_reader_unittest.cc',
+        'base/bind_to_loop_unittest.cc',
         'base/buffers_unittest.cc',
         'base/clock_unittest.cc',
         'base/data_buffer_unittest.cc',

webkit/media/webmediaplayer_impl.cc

@@ -16,6 +16,7 @@
 #include "base/string_number_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "media/audio/null_audio_sink.h"
+#include "media/base/bind_to_loop.h"
 #include "media/base/filter_collection.h"
 #include "media/base/limits.h"
 #include "media/base/media_log.h"
@@ -90,6 +91,10 @@ COMPILE_ASSERT_MATCHING_ENUM(Anonymous);
 COMPILE_ASSERT_MATCHING_ENUM(UseCredentials);
 #undef COMPILE_ASSERT_MATCHING_ENUM
 
+#define BIND_TO_RENDER_LOOP(function) \
+  media::BindToLoop(main_loop_->message_loop_proxy(), base::Bind( \
+      function, AsWeakPtr()))
+
 static WebKit::WebTimeRanges ConvertToWebTimeRanges(
     const media::Ranges<base::TimeDelta>& ranges) {
   WebKit::WebTimeRanges result(ranges.size());
@@ -160,7 +165,7 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
   scoped_refptr<media::VideoRendererBase> video_renderer =
       new media::VideoRendererBase(
           base::Bind(&WebMediaPlayerProxy::Repaint, proxy_),
-          base::Bind(&WebMediaPlayerProxy::SetOpaque, proxy_.get()),
+          BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::SetOpaque),
           true);
   filter_collection_->AddVideoRenderer(video_renderer);
   proxy_->set_frame_provider(video_renderer);
@@ -257,12 +262,12 @@ void WebMediaPlayerImpl::load(const WebKit::WebURL& url, CORSMode cors_mode) {
   proxy_->set_data_source(
       new BufferedDataSource(main_loop_, frame_, media_log_,
                              base::Bind(&WebMediaPlayerImpl::NotifyDownloading,
-                                        base::Unretained(this))));
+                                        AsWeakPtr())));
   proxy_->data_source()->Initialize(
       url, static_cast<BufferedResourceLoader::CORSMode>(cors_mode),
       base::Bind(
           &WebMediaPlayerImpl::DataSourceInitialized,
-          base::Unretained(this), gurl));
+          AsWeakPtr(), gurl));
 
   is_local_source_ = !gurl.SchemeIs("http") && !gurl.SchemeIs("https");
 
@@ -336,8 +341,7 @@ void WebMediaPlayerImpl::seek(float seconds) {
   // Kick off the asynchronous seek!
   pipeline_->Seek(
       seek_time,
-      base::Bind(&WebMediaPlayerProxy::PipelineSeekCallback,
-                 proxy_.get()));
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineSeek));
 }
 
 void WebMediaPlayerImpl::setEndTime(float seconds) {
@@ -969,11 +973,10 @@ void WebMediaPlayerImpl::StartPipeline() {
   starting_ = true;
   pipeline_->Start(
       filter_collection_.Pass(),
-      base::Bind(&WebMediaPlayerProxy::PipelineEndedCallback, proxy_.get()),
-      base::Bind(&WebMediaPlayerProxy::PipelineErrorCallback, proxy_.get()),
-      base::Bind(&WebMediaPlayerProxy::PipelineSeekCallback, proxy_.get()),
-      base::Bind(&WebMediaPlayerProxy::PipelineBufferingStateCallback,
-                 proxy_.get()));
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineEnded),
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineError),
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineSeek),
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineBufferingState));
 }
 
 void WebMediaPlayerImpl::SetNetworkState(WebMediaPlayer::NetworkState state) {

webkit/media/webmediaplayer_proxy.cc

@@ -45,11 +45,6 @@ void WebMediaPlayerProxy::Repaint() {
   }
 }
 
-void WebMediaPlayerProxy::SetOpaque(bool opaque) {
-  render_loop_->PostTask(FROM_HERE, base::Bind(
-      &WebMediaPlayerProxy::SetOpaqueTask, this, opaque));
-}
-
 void WebMediaPlayerProxy::Paint(SkCanvas* canvas,
                                 const gfx::Rect& dest_rect,
                                 uint8_t alpha) {
@@ -89,28 +84,6 @@ void WebMediaPlayerProxy::Detach() {
   frame_provider_ = NULL;
 }
 
-void WebMediaPlayerProxy::PipelineSeekCallback(PipelineStatus status) {
-  render_loop_->PostTask(FROM_HERE, base::Bind(
-      &WebMediaPlayerProxy::PipelineSeekTask, this, status));
-}
-
-void WebMediaPlayerProxy::PipelineEndedCallback(PipelineStatus status) {
-  render_loop_->PostTask(FROM_HERE, base::Bind(
-      &WebMediaPlayerProxy::PipelineEndedTask, this, status));
-}
-
-void WebMediaPlayerProxy::PipelineErrorCallback(PipelineStatus error) {
-  DCHECK_NE(error, media::PIPELINE_OK);
-  render_loop_->PostTask(FROM_HERE, base::Bind(
-      &WebMediaPlayerProxy::PipelineErrorTask, this, error));
-}
-
-void WebMediaPlayerProxy::PipelineBufferingStateCallback(
-    media::Pipeline::BufferingState buffering_state) {
-  render_loop_->PostTask(FROM_HERE, base::Bind(
-      &WebMediaPlayerProxy::PipelineBufferingStateTask, this, buffering_state));
-}
-
 void WebMediaPlayerProxy::RepaintTask() {
   DCHECK(render_loop_->BelongsToCurrentThread());
   {
@@ -123,37 +96,6 @@ void WebMediaPlayerProxy::RepaintTask() {
   }
 }
 
-void WebMediaPlayerProxy::PipelineSeekTask(PipelineStatus status) {
-  DCHECK(render_loop_->BelongsToCurrentThread());
-  if (webmediaplayer_)
-    webmediaplayer_->OnPipelineSeek(status);
-}
-
-void WebMediaPlayerProxy::PipelineEndedTask(PipelineStatus status) {
-  DCHECK(render_loop_->BelongsToCurrentThread());
-  if (webmediaplayer_)
-    webmediaplayer_->OnPipelineEnded(status);
-}
-
-void WebMediaPlayerProxy::PipelineErrorTask(PipelineStatus error) {
-  DCHECK(render_loop_->BelongsToCurrentThread());
-  if (webmediaplayer_)
-    webmediaplayer_->OnPipelineError(error);
-}
-
-void WebMediaPlayerProxy::PipelineBufferingStateTask(
-    media::Pipeline::BufferingState buffering_state) {
-  DCHECK(render_loop_->BelongsToCurrentThread());
-  if (webmediaplayer_)
-    webmediaplayer_->OnPipelineBufferingState(buffering_state);
-}
-
-void WebMediaPlayerProxy::SetOpaqueTask(bool opaque) {
-  DCHECK(render_loop_->BelongsToCurrentThread());
-  if (webmediaplayer_)
-    webmediaplayer_->SetOpaque(opaque);
-}
-
 void WebMediaPlayerProxy::GetCurrentFrame(
     scoped_refptr<media::VideoFrame>* frame_out) {
   if (frame_provider_)

webkit/media/webmediaplayer_proxy.h

@@ -62,7 +62,6 @@ class WebMediaPlayerProxy
 
   // Methods for Filter -> WebMediaPlayerImpl communication.
   void Repaint();
-  void SetOpaque(bool opaque);
 
   // Methods for WebMediaPlayerImpl -> Filter communication.
   void Paint(SkCanvas* canvas, const gfx::Rect& dest_rect, uint8_t alpha);
@@ -74,13 +73,6 @@ class WebMediaPlayerProxy
 
   void AbortDataSource();
 
-  // Methods for Pipeline -> WebMediaPlayerImpl communication.
-  void PipelineSeekCallback(media::PipelineStatus status);
-  void PipelineEndedCallback(media::PipelineStatus status);
-  void PipelineErrorCallback(media::PipelineStatus error);
-  void PipelineBufferingStateCallback(
-      media::Pipeline::BufferingState buffering_state);
-
   // ChunkDemuxerClient implementation.
   virtual void DemuxerOpened(media::ChunkDemuxer* demuxer) OVERRIDE;
   virtual void DemuxerClosed() OVERRIDE;
@@ -125,23 +117,6 @@ class WebMediaPlayerProxy
   // Invoke |webmediaplayer_| to perform a repaint.
   void RepaintTask();
 
-  // Notify |webmediaplayer_| that a seek has finished.
-  void PipelineSeekTask(media::PipelineStatus status);
-
-  // Notify |webmediaplayer_| that the media has ended.
-  void PipelineEndedTask(media::PipelineStatus status);
-
-  // Notify |webmediaplayer_| that a pipeline error has occurred during
-  // playback.
-  void PipelineErrorTask(media::PipelineStatus error);
-
-  // Notify |webmediaplayer_| of buffering state changes.
-  void PipelineBufferingStateTask(
-      media::Pipeline::BufferingState buffering_state);
-
-  // Inform |webmediaplayer_| whether the video content is opaque.
-  void SetOpaqueTask(bool opaque);
-
   void DemuxerOpenedTask(const scoped_refptr<media::ChunkDemuxer>& demuxer);
   void DemuxerClosedTask();