Remove threading from RendererGpuVideoAcceleratorFactories

media/base/bind_to_loop.h

Index: media/base/bind_to_loop.h
diff --git a/media/base/bind_to_loop.h b/media/base/bind_to_loop.h
index 92d358c7be9432ce6cc5f5b105816b1bf92b2414..ad160c868785c0682bf1577f201017061a664ca4 100644
--- a/media/base/bind_to_loop.h
+++ b/media/base/bind_to_loop.h
@@ -13,6 +13,7 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/synchronization/waitable_event.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
@@ -24,9 +25,19 @@
 //    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.
+// For synchronous usage, where the callback needs to be executed on another
+// MessageLoop, but the calling thread should block until the callback has been
+// completed, there is BindToLoopSync(), as below:
+//
+// Typical usage: make a request to another thread, and wait for it:
+// do_something_callback =
+//   media::BindToLoopSync(other_message_loop, &OtherClass::DoRequest, other);
+// // ...
+// do_something_callback.Run();  // will block until completion
+//
+// Note that like base::Bind(), BindToLoop() and BindToLoopSync() 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 {
 
@@ -50,6 +61,10 @@ base::internal::PassedWrapper<ScopedVector<T> > TrampolineForward(
 
 template <typename T> struct TrampolineHelper;
 
+// Caller helper to call a base::Closure synchronously
+void TrampolineSyncCaller(const base::Closure& closure,
+                          base::WaitableEvent* waiter);
+
 template <>
 struct TrampolineHelper<void()> {
   static void Run(
@@ -57,6 +72,19 @@ struct TrampolineHelper<void()> {
       const base::Callback<void()>& cb) {
     loop->PostTask(FROM_HERE, base::Bind(cb));
   }
+  static void RunSync(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void()>& cb) {
+    DCHECK(!loop->BelongsToCurrentThread());
+    base::WaitableEvent waiter(false, false);
+    loop->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &TrampolineSyncCaller,
+            base::Bind(cb),
+            &waiter));
+    waiter.Wait();
+  }
 };
 
 
@@ -67,6 +95,19 @@ struct TrampolineHelper<void(A1)> {
       const base::Callback<void(A1)>& cb, A1 a1) {
     loop->PostTask(FROM_HERE, base::Bind(cb, internal::TrampolineForward(a1)));
   }
+  static void RunSync(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1)>& cb, A1 a1) {
+    DCHECK(!loop->BelongsToCurrentThread());
+    base::WaitableEvent waiter(false, false);
+    loop->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &TrampolineSyncCaller,
+            base::Bind(cb, internal::TrampolineForward(a1)),
+            &waiter));
+    waiter.Wait();
+  }
 };
 
 
@@ -78,6 +119,20 @@ struct TrampolineHelper<void(A1, A2)> {
     loop->PostTask(FROM_HERE, base::Bind(cb, internal::TrampolineForward(a1),
         internal::TrampolineForward(a2)));
   }
+  static void RunSync(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2)>& cb, A1 a1, A2 a2) {
+    DCHECK(!loop->BelongsToCurrentThread());
+    base::WaitableEvent waiter(false, false);
+    loop->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &TrampolineSyncCaller,
+            base::Bind(cb, internal::TrampolineForward(a1),
+                internal::TrampolineForward(a2)),
+            &waiter));
+    waiter.Wait();
+  }
 };
 
 
@@ -89,6 +144,21 @@ struct TrampolineHelper<void(A1, A2, A3)> {
     loop->PostTask(FROM_HERE, base::Bind(cb, internal::TrampolineForward(a1),
         internal::TrampolineForward(a2), internal::TrampolineForward(a3)));
   }
+  static void RunSync(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2, A3)>& cb, A1 a1, A2 a2, A3 a3) {
+    DCHECK(!loop->BelongsToCurrentThread());
+    base::WaitableEvent waiter(false, false);
+    loop->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &TrampolineSyncCaller,
+            base::Bind(cb, internal::TrampolineForward(a1),
+                internal::TrampolineForward(a2),
+                internal::TrampolineForward(a3)),
+            &waiter));
+    waiter.Wait();
+  }
 };
 
 
@@ -102,6 +172,23 @@ struct TrampolineHelper<void(A1, A2, A3, A4)> {
         internal::TrampolineForward(a2), internal::TrampolineForward(a3),
         internal::TrampolineForward(a4)));
   }
+  static void RunSync(
+      const scoped_refptr<base::MessageLoopProxy>& loop,
+      const base::Callback<void(A1, A2, A3, A4)>& cb, A1 a1, A2 a2, A3 a3,
+          A4 a4) {
+    DCHECK(!loop->BelongsToCurrentThread());
+    base::WaitableEvent waiter(false, false);
+    loop->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &TrampolineSyncCaller,
+            base::Bind(cb, internal::TrampolineForward(a1),
+                internal::TrampolineForward(a2),
+                internal::TrampolineForward(a3),
+                internal::TrampolineForward(a4)),
+            &waiter));
+    waiter.Wait();
+  }
 };
 
 
@@ -115,6 +202,24 @@ struct TrampolineHelper<void(A1, A2, A3, A4, A5)> {
         internal::TrampolineForward(a2), internal::TrampolineForward(a3),
         internal::TrampolineForward(a4), internal::TrampolineForward(a5)));
   }
+  static void RunSync(
+      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) {
+    DCHECK(!loop->BelongsToCurrentThread());
+    base::WaitableEvent waiter(false, false);
+    loop->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &TrampolineSyncCaller,
+            base::Bind(cb, internal::TrampolineForward(a1),
+                internal::TrampolineForward(a2),
+                internal::TrampolineForward(a3),
+                internal::TrampolineForward(a4),
+                internal::TrampolineForward(a5)),
+            &waiter));
+    waiter.Wait();
+  }
 };
 
 
@@ -130,6 +235,25 @@ struct TrampolineHelper<void(A1, A2, A3, A4, A5, A6)> {
         internal::TrampolineForward(a4), internal::TrampolineForward(a5),
         internal::TrampolineForward(a6)));
   }
+  static void RunSync(
+      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) {
+    DCHECK(!loop->BelongsToCurrentThread());
+    base::WaitableEvent waiter(false, false);
+    loop->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &TrampolineSyncCaller,
+            base::Bind(cb, internal::TrampolineForward(a1),
+                internal::TrampolineForward(a2),
+                internal::TrampolineForward(a3),
+                internal::TrampolineForward(a4),
+                internal::TrampolineForward(a5),
+                internal::TrampolineForward(a6)),
+            &waiter));
+    waiter.Wait();
+  }
 };
 
 
@@ -145,6 +269,26 @@ struct TrampolineHelper<void(A1, A2, A3, A4, A5, A6, A7)> {
         internal::TrampolineForward(a4), internal::TrampolineForward(a5),
         internal::TrampolineForward(a6), internal::TrampolineForward(a7)));
   }
+  static void RunSync(
+      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) {
+    DCHECK(!loop->BelongsToCurrentThread());
+    base::WaitableEvent waiter(false, false);
+    loop->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &TrampolineSyncCaller,
+            base::Bind(cb, internal::TrampolineForward(a1),
+                internal::TrampolineForward(a2),
+                internal::TrampolineForward(a3),
+                internal::TrampolineForward(a4),
+                internal::TrampolineForward(a5),
+                internal::TrampolineForward(a6),
+                internal::TrampolineForward(a7)),
+            &waiter));
+    waiter.Wait();
+  }
 };
 
 
@@ -158,6 +302,13 @@ static base::Callback<T> BindToLoop(
 }
 
 template<typename T>
+static base::Callback<T> BindToLoopSync(

[jam, 2013/10/28 20:11:08]

I'm concerned about this. This makes it easy for code to block one message loop
on another, which is an anti-pattern. I guess if a background thread is blocked
on another one, that may be ok. But what guarantee do we have that people won't
use this on the UI or IO threads (in the browser) or on the main renderer
thread?

I'm also concerned that we have this parallel base utilities for bind stuff.
Having multiple places to look for base-y things seems confusing and will lead
to duplication.

[sheu, 2013/10/28 20:32:58]

There's no guarantee, other than a general exhortation not to.  Is this danger
sufficient to make BindToLoopSync() something that Should Not Be Done?
That depends on the discussion previously on whether this has the approvals to
be put in base/.  It probably could, but I think that should be a separate
change where BindToLoop() goes into base/ simultaneously.

[jam, 2013/10/28 20:43:25]

If there's one thing I've learnt on Chrome, it's introducing a way to shoot
yourself in the foot will be abused much faster than could ever be expected.

So this is why I'm opposed to have a generic functionality to block one thread
on another. Or, at the very least, we should only do this after we have a
mechanism to enable it only for specific (background) threads, and to have a way
that ensures that this never gets allowed for main/IO threads.

+    const scoped_refptr<base::MessageLoopProxy>& loop,
+    const base::Callback<T>& cb) {
+  return base::Bind(&internal::TrampolineHelper<T>::RunSync, loop, cb);
+}
+
+template<typename T>
 static base::Callback<T> BindToCurrentLoop(
     const base::Callback<T>& cb) {
   return BindToLoop(base::MessageLoopProxy::current(), cb);