Cast: deliver video frames on the IO thread

content/renderer/media/video_frame_deliverer.cc

Index: content/renderer/media/video_frame_deliverer.cc
diff --git a/content/renderer/media/video_frame_deliverer.cc b/content/renderer/media/video_frame_deliverer.cc
index 67b11c42ecdc4482f8b23c733c6c1a48cb0e5703..c291d0a7f1b8ec827f95b4e3cdfcd879bc946831 100644
--- a/content/renderer/media/video_frame_deliverer.cc
+++ b/content/renderer/media/video_frame_deliverer.cc
@@ -8,6 +8,11 @@
 #include "base/location.h"
 
 namespace content {
+namespace {
+void ResetCallback(VideoCaptureDeliverFrameCB callback) {
+  callback.Reset();
+}
+}  // namespace
 
 VideoFrameDeliverer::VideoFrameDeliverer(
     const scoped_refptr<base::MessageLoopProxy>& io_message_loop)
@@ -41,15 +46,18 @@ void VideoFrameDeliverer::RemoveCallback(void* id) {
   io_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&VideoFrameDeliverer::RemoveCallbackOnIO,
-                 this, id));
+                 this, id, base::MessageLoopProxy::current()));
 }
 
-void VideoFrameDeliverer::RemoveCallbackOnIO(void* id) {
+void VideoFrameDeliverer::RemoveCallbackOnIO(
+    void* id, const scoped_refptr<base::MessageLoopProxy>& message_loop) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   std::vector<VideoIdCallbackPair>::iterator it = callbacks_.begin();
   for (; it != callbacks_.end(); ++it) {
     if (it->first == id) {
+      VideoCaptureDeliverFrameCB callback = it->second;
       callbacks_.erase(it);
+      message_loop->PostTask(FROM_HERE, base::Bind(&ResetCallback, callback));

[piman, 2014/05/09 03:44:23]

So, this is racy.
If the target message loop is free, then ResetCallback can be called immediately
(releasing its reference), but then the local variable |callback| still is in
scope and has a reference to the callback storage, so the last one would be
removed on the IO thread.
(it also shows the difficulty of writing a full test for this :( )

Maybe something along the lines of this would work:

scoped_ptr<VideoCaptureDeliverFrameCB> callback;
{
  callback.reset(new VideoCaptureDeliverFrameCB(it->second));
  callbacks_.erase(it);
}
message_loop->PostTask(FROM_HERE, base::Bind(&ResetCallback, Passed(callback)));

(with big warnings about the subtlety of this).
You really can't have any temporary or anything still have a copy of the
callback, by the time you post the task.


Note that this will only work if message_loop outlives the current thread
(fortunately it is the case here).

       return;
     }
   }