Cast: deliver video frames on the IO thread

content/renderer/media/video_frame_deliverer.cc

 // Copyright 2014 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 "content/renderer/media/video_frame_deliverer.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 
 namespace content {
+namespace {
+void ResetCallback(VideoCaptureDeliverFrameCB callback) {
+  callback.Reset();
+}
+}  // namespace
 
 VideoFrameDeliverer::VideoFrameDeliverer(
     const scoped_refptr<base::MessageLoopProxy>& io_message_loop)
     : io_message_loop_(io_message_loop) {
   DCHECK(io_message_loop_);
 }
 
 VideoFrameDeliverer::~VideoFrameDeliverer() {
   DCHECK(callbacks_.empty());
 }
@@ skipping 13 matching lines @@
     const VideoCaptureDeliverFrameCB& callback) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   callbacks_.push_back(std::make_pair(id, callback));
 }
 
 void VideoFrameDeliverer::RemoveCallback(void* id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   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;
     }
   }
 }
 
 void VideoFrameDeliverer::DeliverFrameOnIO(
     const scoped_refptr<media::VideoFrame>& frame,
     const media::VideoCaptureFormat& format) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   for (std::vector<VideoIdCallbackPair>::iterator it = callbacks_.begin();
        it != callbacks_.end(); ++it) {
     it->second.Run(frame, format);
   }
 }
 
 }  // namespace content