Refactor VideoStub interface to accept ownership of video packets.

remoting/protocol/message_reader.cc

 // Copyright (c) 2011 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 "remoting/protocol/message_reader.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
 #include "net/base/io_buffer.h"
@@ skipping 63 matching lines @@
       LOG(ERROR) << "Read() returned error " << result;
     }
   }
 }
 
 void MessageReader::OnDataReceived(net::IOBuffer* data, int data_size) {
   message_decoder_.AddData(data, data_size);
 
   // Get list of all new messages first, and then call the callback
   // for all of them.
   std::vector<CompoundBuffer*> new_messages;

[Alpha Left Google, 2012/03/26 20:29:18]

The ownership of CompoundBuffer is unclear here. I think your intention is to
let OnNewData() to delete the message by scoped_ptr.

Since we are transferring ownership of CompoundBuffer to
|message_received_callback_| anyway I would prefer to use
vector<scoped_ptr<CompoundBuffer> > and then .Pass below to be explicit about
the intention.

[Sergey Ulanov, 2012/03/26 20:42:13]

Yes this code gets ownership of these messages and then passes to
message_received_callback_. 
Can't use scoped_ptr<> here because it's not compatible with STL containers
(it's not copyable) :-(.

   while (true) {
     CompoundBuffer* buffer = message_decoder_.GetNextMessage();
     if (!buffer)
       break;
     new_messages.push_back(buffer);
   }
 
   pending_messages_ += new_messages.size();
 
   // TODO(lambroslambrou): MessageLoopProxy::current() will not work from the
   // plugin thread if this code is compiled into a separate binary.  Fix this.
   for (std::vector<CompoundBuffer*>::iterator it = new_messages.begin();
        it != new_messages.end(); ++it) {
-    message_received_callback_.Run(*it, base::Bind(
-        &MessageReader::OnMessageDone, this,
-        *it, base::MessageLoopProxy::current()));
+    message_received_callback_.Run(
+        scoped_ptr<CompoundBuffer>(*it),
+        base::Bind(&MessageReader::OnMessageDone, this,
+                   base::MessageLoopProxy::current()));
   }
 }
 
 void MessageReader::OnMessageDone(
-    CompoundBuffer* message,
     scoped_refptr<base::MessageLoopProxy> message_loop) {
   if (!message_loop->BelongsToCurrentThread()) {
     message_loop->PostTask(
         FROM_HERE,
-        base::Bind(&MessageReader::OnMessageDone, this, message, message_loop));
+        base::Bind(&MessageReader::OnMessageDone, this, message_loop));
     return;
   }
-  delete message;
   ProcessDoneEvent();
 }
 
 void MessageReader::ProcessDoneEvent() {
   pending_messages_--;
   DCHECK_GE(pending_messages_, 0);
 
   DoRead(); // Start next read if neccessary.
 }
 
 }  // namespace protocol
 }  // namespace remoting