Refactor VideoStub interface to accept ownership of video packets.

remoting/client/chromoting_client.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 "remoting/client/chromoting_client.h"
 
 #include "base/bind.h"
 #include "remoting/client/chromoting_view.h"
 #include "remoting/client/client_context.h"
 #include "remoting/client/rectangle_update_decoder.h"
 #include "remoting/protocol/authentication_method.h"
 #include "remoting/protocol/connection_to_host.h"
 #include "remoting/protocol/negotiating_authenticator.h"
 #include "remoting/protocol/v1_authenticator.h"
 #include "remoting/protocol/session_config.h"
 
 namespace remoting {
 
 using protocol::AuthenticationMethod;
 
 ChromotingClient::QueuedVideoPacket::QueuedVideoPacket(
-    const VideoPacket* packet, const base::Closure& done)
-    : packet(packet), done(done) {
+    scoped_ptr<VideoPacket> packet, const base::Closure& done)
+    : packet(packet.release()), done(done) {

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

This line seems odd to me, can we use different variable names for parameter and
member? Also is .release() necessary here? It seems complicated to me since we
first call .Pass() then .release(), is there a simpler way?

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

done(done) was here before. 

We need to release() here instead of Pass() because this struct stores pointer
instead of scoped_ptr<>. Can't use scoped_ptr<> because the struct needs to be
copyable so that it can be stored in STL containers.

 }
 
 ChromotingClient::QueuedVideoPacket::~QueuedVideoPacket() {
 }
 
 ChromotingClient::ChromotingClient(const ClientConfig& config,
                                    ClientContext* context,
                                    protocol::ConnectionToHost* connection,
                                    ChromotingView* view,
                                    RectangleUpdateDecoder* rectangle_decoder,
@@ skipping 62 matching lines @@
   if (!client_done_.is_null()) {
     message_loop()->PostTask(FROM_HERE, client_done_);
     client_done_.Reset();
   }
 }
 
 ChromotingStats* ChromotingClient::GetStats() {
   return &stats_;
 }
 
-void ChromotingClient::ProcessVideoPacket(const VideoPacket* packet,
+void ChromotingClient::ProcessVideoPacket(scoped_ptr<VideoPacket> packet,
                                           const base::Closure& done) {
   DCHECK(message_loop()->BelongsToCurrentThread());
 
   // If the video packet is empty then drop it. Empty packets are used to
   // maintain activity on the network.
   if (!packet->has_data() || packet->data().size() == 0) {
     done.Run();
     return;
   }
 
   // Add one frame to the counter.
   stats_.video_frame_rate()->Record(1);
 
   // Record other statistics received from host.
   stats_.video_bandwidth()->Record(packet->data().size());
   if (packet->has_capture_time_ms())
     stats_.video_capture_ms()->Record(packet->capture_time_ms());
   if (packet->has_encode_time_ms())
     stats_.video_encode_ms()->Record(packet->encode_time_ms());
   if (packet->has_client_sequence_number() &&
       packet->client_sequence_number() > last_sequence_number_) {
     last_sequence_number_ = packet->client_sequence_number();
     base::TimeDelta round_trip_latency =
         base::Time::Now() -
         base::Time::FromInternalValue(packet->client_sequence_number());
     stats_.round_trip_ms()->Record(round_trip_latency.InMilliseconds());
   }
 
-  received_packets_.push_back(QueuedVideoPacket(packet, done));
+  received_packets_.push_back(QueuedVideoPacket(packet.Pass(), done));
   if (!packet_being_processed_)
     DispatchPacket();
 }
 
 int ChromotingClient::GetPendingPackets() {
   return received_packets_.size();
 }
 
 void ChromotingClient::DispatchPacket() {
   DCHECK(message_loop()->BelongsToCurrentThread());
@@ skipping 62 matching lines @@
     thread_proxy_.PostTask(FROM_HERE, base::Bind(
         &ChromotingClient::Initialize, base::Unretained(this)));
     return;
   }
 
   // Initialize the decoder.
   rectangle_decoder_->Initialize(connection_->config());
 }
 
 }  // namespace remoting