Use VideoStub interface on the host side.

BUG=None
TEST=None

Review URL: http://codereview.chromium.org/5232004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66841 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 89 insertions, 85 deletions

diff --git a/remoting/client/chromoting_client.cc b/remoting/client/chromoting_client.cc
index 1a0192c..cd358c4 100644
--- a/remoting/client/chromoting_client.cc
+++ b/remoting/client/chromoting_client.cc
@@ -110,6 +110,10 @@ void ChromotingClient::ProcessVideoPacket(const VideoPacket* packet,
     DispatchPacket();
 }
 
+int ChromotingClient::GetPendingPackets() {
+  return received_packets_.size();
+}
+
 void ChromotingClient::DispatchPacket() {
   DCHECK_EQ(message_loop(), MessageLoop::current());
   CHECK(!packet_being_processed_);
diff --git a/remoting/client/chromoting_client.h b/remoting/client/chromoting_client.h
index 7bbe306..e6e7d7d 100644
--- a/remoting/client/chromoting_client.h
+++ b/remoting/client/chromoting_client.h
@@ -59,6 +59,7 @@ class ChromotingClient : public protocol::ConnectionToHost::HostEventCallback,
 
   // VideoStub implementation.
   virtual void ProcessVideoPacket(const VideoPacket* packet, Task* done);
+  virtual int GetPendingPackets();
 
  private:
   struct QueuedVideoPacket {
diff --git a/remoting/host/session_manager.cc b/remoting/host/session_manager.cc
index ebd1061..219c982 100644
--- a/remoting/host/session_manager.cc
+++ b/remoting/host/session_manager.cc
@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/scoped_ptr.h"
 #include "base/stl_util-inl.h"
+#include "base/task.h"
 #include "remoting/base/capture_data.h"
 #include "remoting/base/tracer.h"
 #include "remoting/proto/control.pb.h"
@@ -305,7 +306,7 @@ void SessionManager::DoRateControl() {
   for (size_t i = 0; i < connections_.size(); ++i) {
     max_pending_update_streams =
         std::max(max_pending_update_streams,
-                 connections_[i]->GetPendingUpdateStreamMessages());
+                 connections_[i]->video_stub()->GetPendingPackets());
   }
 
   // If |slow_down| equals zero, we have no slow down.
@@ -336,9 +337,9 @@ void SessionManager::DoSendVideoPacket(VideoPacket* packet) {
 
   for (ConnectionToClientList::const_iterator i = connections_.begin();
        i < connections_.end(); ++i) {
-    (*i)->SendVideoPacket(*packet);
+    (*i)->video_stub()->ProcessVideoPacket(
+        packet, new DeleteTask<VideoPacket>(packet));
   }
-  delete packet;
 
   TraceContext::tracer()->PrintString("DoSendUpdate done");
 }
diff --git a/remoting/host/session_manager_unittest.cc b/remoting/host/session_manager_unittest.cc
index 5c853b28..967d778 100644
--- a/remoting/host/session_manager_unittest.cc
+++ b/remoting/host/session_manager_unittest.cc
@@ -13,6 +13,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::remoting::protocol::MockConnectionToClient;
+using ::remoting::protocol::MockVideoStub;
 
 using ::testing::_;
 using ::testing::AtLeast;
@@ -101,9 +102,13 @@ TEST_F(SessionManagerTest, DISABLED_OneRecordCycle) {
   EXPECT_CALL(*encoder_, Encode(data, false, NotNull()))
       .WillOnce(FinishEncode(packet));
 
+  MockVideoStub video_stub;
+  EXPECT_CALL(*connection_, video_stub())
+      .WillRepeatedly(Return(&video_stub));
+
   // Expect the client be notified.
-  EXPECT_CALL(*connection_, SendVideoPacket(_));
-  EXPECT_CALL(*connection_, GetPendingUpdateStreamMessages())
+  EXPECT_CALL(video_stub, ProcessVideoPacket(_, _));
+  EXPECT_CALL(video_stub, GetPendingPackets())
       .Times(AtLeast(0))
       .WillRepeatedly(Return(0));
 
diff --git a/remoting/protocol/connection_to_client.cc b/remoting/protocol/connection_to_client.cc
index 0661be3..7d9fb17 100644
--- a/remoting/protocol/connection_to_client.cc
+++ b/remoting/protocol/connection_to_client.cc
@@ -48,21 +48,6 @@ protocol::Session* ConnectionToClient::session() {
   return session_;
 }
 
-void ConnectionToClient::SendVideoPacket(const VideoPacket& packet) {
-  DCHECK_EQ(loop_, MessageLoop::current());
-
-  // If we are disconnected then return.
-  if (!session_)
-    return;
-
-  video_writer_->SendPacket(packet);
-}
-
-int ConnectionToClient::GetPendingUpdateStreamMessages() {
-  DCHECK_EQ(loop_, MessageLoop::current());
-  return video_writer_->GetPendingPackets();
-}
-
 void ConnectionToClient::Disconnect() {
   DCHECK_EQ(loop_, MessageLoop::current());
 
@@ -73,10 +58,19 @@ void ConnectionToClient::Disconnect() {
   }
 }
 
-ConnectionToClient::ConnectionToClient() {}
+VideoStub* ConnectionToClient::video_stub() {
+  return video_writer_.get();
+}
+
+// Return pointer to ClientStub.
+ClientStub* ConnectionToClient::client_stub() {
+  return client_stub_.get();
+}
+
+ConnectionToClient::ConnectionToClient() {
+}
 
-void ConnectionToClient::OnSessionStateChange(
-    protocol::Session::State state) {
+void ConnectionToClient::OnSessionStateChange(protocol::Session::State state) {
   if (state == protocol::Session::CONNECTED) {
     client_stub_.reset(new ClientControlSender(session_->control_channel()));
     video_writer_.reset(VideoWriter::Create(session_->config()));
diff --git a/remoting/protocol/connection_to_client.h b/remoting/protocol/connection_to_client.h
index 546db76..5984528 100644
--- a/remoting/protocol/connection_to_client.h
+++ b/remoting/protocol/connection_to_client.h
@@ -61,23 +61,17 @@ class ConnectionToClient :
   // Returns the connection in use.
   virtual protocol::Session* session();
 
-  // Send encoded update stream data to the viewer.
-  virtual void SendVideoPacket(const VideoPacket& packet);
-
-  // Gets the number of update stream messages not yet transmitted.
-  // Note that the value returned is an estimate using average size of the
-  // most recent update streams.
-  // TODO(hclam): Report this number accurately.
-  virtual int GetPendingUpdateStreamMessages();
-
   // Disconnect the client connection. This method is allowed to be called
   // more than once and calls after the first one will be ignored.
   //
   // After this method is called all the send method calls will be ignored.
   virtual void Disconnect();
 
+  // Send encoded update stream data to the viewer.
+  virtual VideoStub* video_stub();
+
   // Return pointer to ClientStub.
-  virtual ClientStub* client_stub() { return client_stub_.get(); }
+  virtual ClientStub* client_stub();
 
  protected:
   // Protected constructor used by unit test.
diff --git a/remoting/protocol/connection_to_client_unittest.cc b/remoting/protocol/connection_to_client_unittest.cc
index eaea342..1420392 100644
--- a/remoting/protocol/connection_to_client_unittest.cc
+++ b/remoting/protocol/connection_to_client_unittest.cc
@@ -50,8 +50,9 @@ class ConnectionToClientTest : public testing::Test {
 
 TEST_F(ConnectionToClientTest, SendUpdateStream) {
   // Then send the actual data.
-  VideoPacket packet;
-  viewer_->SendVideoPacket(packet);
+  VideoPacket* packet = new VideoPacket();
+  viewer_->video_stub()->ProcessVideoPacket(
+      packet, new DeleteTask<VideoPacket>(packet));
 
   // And then close the connection to ConnectionToClient.
   viewer_->Disconnect();
@@ -73,20 +74,14 @@ TEST_F(ConnectionToClientTest, StateChange) {
   message_loop_.RunAllPending();
 }
 
-// Test that we can close client connection more than once and operations
-// after the connection is closed has no effect.
+// Test that we can close client connection more than once.
 TEST_F(ConnectionToClientTest, Close) {
   viewer_->Disconnect();
   message_loop_.RunAllPending();
   EXPECT_TRUE(session_->is_closed());
 
-  VideoPacket packet;
-  viewer_->SendVideoPacket(packet);
   viewer_->Disconnect();
   message_loop_.RunAllPending();
-
-  // Verify that nothing has been written.
-  EXPECT_EQ(session_->video_channel()->written_data().size(), 0u);
 }
 
 }  // namespace protocol
diff --git a/remoting/protocol/mock_objects.h b/remoting/protocol/mock_objects.h
index b10c139..587d415 100644
--- a/remoting/protocol/mock_objects.h
+++ b/remoting/protocol/mock_objects.h
@@ -9,6 +9,7 @@
 #include "remoting/protocol/connection_to_client.h"
 #include "remoting/protocol/host_stub.h"
 #include "remoting/protocol/input_stub.h"
+#include "remoting/protocol/video_stub.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace remoting {
@@ -22,8 +23,7 @@ class MockConnectionToClient : public ConnectionToClient {
 
   MOCK_METHOD1(Init, void(ChromotocolConnection* connection));
   MOCK_METHOD2(SendInitClientMessage, void(int width, int height));
-  MOCK_METHOD1(SendVideoPacket, void(const VideoPacket& packet));
-  MOCK_METHOD0(GetPendingUpdateStreamMessages, int());
+  MOCK_METHOD0(video_stub, VideoStub*());
   MOCK_METHOD0(Disconnect, void());
 
  private:
@@ -68,6 +68,18 @@ class MockHostStub : public HostStub {
   DISALLOW_COPY_AND_ASSIGN(MockHostStub);
 };
 
+class MockVideoStub : public VideoStub {
+ public:
+  MockVideoStub() {}
+
+  MOCK_METHOD2(ProcessVideoPacket, void(const VideoPacket* video_packet,
+                                        Task* done));
+  MOCK_METHOD0(GetPendingPackets, int());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockVideoStub);
+};
+
 }  // namespace protocol
 }  // namespace remoting
 
diff --git a/remoting/protocol/protobuf_video_writer.cc b/remoting/protocol/protobuf_video_writer.cc
index fdd3b9e..73a9b0d 100644
--- a/remoting/protocol/protobuf_video_writer.cc
+++ b/remoting/protocol/protobuf_video_writer.cc
@@ -4,6 +4,7 @@
 
 #include "remoting/protocol/protobuf_video_writer.h"
 
+#include "base/task.h"
 #include "remoting/proto/video.pb.h"
 #include "remoting/protocol/rtp_writer.h"
 #include "remoting/protocol/session.h"
@@ -22,18 +23,16 @@ void ProtobufVideoWriter::Init(protocol::Session* session) {
   buffered_writer_->Init(session->video_channel(), NULL);
 }
 
-void ProtobufVideoWriter::SendPacket(const VideoPacket& packet) {
-  buffered_writer_->Write(SerializeAndFrameMessage(packet));
+void ProtobufVideoWriter::ProcessVideoPacket(const VideoPacket* packet,
+                                             Task* done) {
+  buffered_writer_->Write(SerializeAndFrameMessage(*packet));
+  done->Run();
+  delete done;
 }
 
 int ProtobufVideoWriter::GetPendingPackets() {
   return buffered_writer_->GetBufferChunks();
 }
 
-
-void ProtobufVideoWriter::Close() {
-  buffered_writer_->Close();
-}
-
 }  // namespace protocol
 }  // namespace remoting
diff --git a/remoting/protocol/protobuf_video_writer.h b/remoting/protocol/protobuf_video_writer.h
index ba02584..e288de1 100644
--- a/remoting/protocol/protobuf_video_writer.h
+++ b/remoting/protocol/protobuf_video_writer.h
@@ -21,9 +21,10 @@ class ProtobufVideoWriter : public VideoWriter {
 
   // VideoWriter interface.
   virtual void Init(protocol::Session* session);
-  virtual void SendPacket(const VideoPacket& packet);
+
+  // VideoStub interface.
+  virtual void ProcessVideoPacket(const VideoPacket* packet, Task* done);
   virtual int GetPendingPackets();
-  virtual void Close();
 
  private:
   scoped_refptr<BufferedSocketWriter> buffered_writer_;
diff --git a/remoting/protocol/rtp_video_reader_unittest.cc b/remoting/protocol/rtp_video_reader_unittest.cc
index 81a8e17..60dba69c 100644
--- a/remoting/protocol/rtp_video_reader_unittest.cc
+++ b/remoting/protocol/rtp_video_reader_unittest.cc
@@ -22,6 +22,7 @@ namespace protocol {
 class RtpVideoReaderTest : public testing::Test,
                            public VideoStub {
  public:
+  // VideoStub interface.
   virtual void ProcessVideoPacket(const VideoPacket* video_packet,
                                   Task* done) {
     received_packets_.push_back(VideoPacket());
@@ -30,6 +31,10 @@ class RtpVideoReaderTest : public testing::Test,
     delete done;
   }
 
+  virtual int GetPendingPackets() {
+    return 0;
+  }
+
  protected:
   struct FragmentInfo {
     int sequence_number;
diff --git a/remoting/protocol/rtp_video_writer.cc b/remoting/protocol/rtp_video_writer.cc
index 146752c..f74d932 100644
--- a/remoting/protocol/rtp_video_writer.cc
+++ b/remoting/protocol/rtp_video_writer.cc
@@ -4,6 +4,7 @@
 
 #include "remoting/protocol/rtp_video_writer.h"
 
+#include "base/task.h"
 #include "net/base/io_buffer.h"
 #include "remoting/base/compound_buffer.h"
 #include "remoting/proto/video.pb.h"
@@ -25,14 +26,14 @@ void RtpVideoWriter::Init(protocol::Session* session) {
   rtp_writer_.Init(session->video_rtp_channel(), session->video_rtcp_channel());
 }
 
-void RtpVideoWriter::SendPacket(const VideoPacket& packet) {
-  CHECK(packet.format().encoding() == VideoPacketFormat::ENCODING_VP8)
+void RtpVideoWriter::ProcessVideoPacket(const VideoPacket* packet, Task* done) {
+  CHECK(packet->format().encoding() == VideoPacketFormat::ENCODING_VP8)
       << "Only VP8 is supported in RTP.";
 
   CompoundBuffer payload;
   // TODO(sergeyu): This copy would not be necessary CompoundBuffer was used
   // inside of VideoPacket.
-  payload.AppendCopyOf(packet.data().data(), packet.data().size());
+  payload.AppendCopyOf(packet->data().data(), packet->data().size());
 
   Vp8Descriptor vp8_desriptor;
   // TODO(sergeyu): Add a flag in VideoPacket that indicates whether this is a
@@ -47,11 +48,11 @@ void RtpVideoWriter::SendPacket(const VideoPacket& packet) {
     // Frame beginning flag is set only for the first packet in the first
     // partition.
     vp8_desriptor.frame_beginning =
-        (packet.flags() & VideoPacket::FIRST_PACKET) != 0 && position == 0;
+        (packet->flags() & VideoPacket::FIRST_PACKET) != 0 && position == 0;
 
     // Marker bit is set only for the last packet in the last partition.
     bool marker = (position + size) == payload.total_bytes() &&
-        (packet.flags() & VideoPacket::LAST_PACKET) != 0;
+        (packet->flags() & VideoPacket::LAST_PACKET) != 0;
 
     // Set fragmentation flag appropriately.
     if (position == 0) {
@@ -73,21 +74,19 @@ void RtpVideoWriter::SendPacket(const VideoPacket& packet) {
     chunk.CopyFrom(payload, position, position + size);
 
     // And send it.
-    rtp_writer_.SendPacket(packet.timestamp(), marker, vp8_desriptor, chunk);
+    rtp_writer_.SendPacket(packet->timestamp(), marker, vp8_desriptor, chunk);
 
     position += size;
   }
-
   DCHECK_EQ(position, payload.total_bytes());
+
+  done->Run();
+  delete done;
 }
 
 int RtpVideoWriter::GetPendingPackets() {
   return rtp_writer_.GetPendingPackets();
 }
 
-void RtpVideoWriter::Close() {
-  rtp_writer_.Close();
-}
-
 }  // namespace protocol
 }  // namespace remoting
diff --git a/remoting/protocol/rtp_video_writer.h b/remoting/protocol/rtp_video_writer.h
index 3d05925..809ac49 100644
--- a/remoting/protocol/rtp_video_writer.h
+++ b/remoting/protocol/rtp_video_writer.h
@@ -20,9 +20,10 @@ class RtpVideoWriter : public VideoWriter {
 
   // VideoWriter interface.
   virtual void Init(protocol::Session* session);
-  virtual void SendPacket(const VideoPacket& packet);
+
+  // VideoStub interface.
+  virtual void ProcessVideoPacket(const VideoPacket* packet, Task* done);
   virtual int GetPendingPackets();
-  virtual void Close();
 
  private:
   RtpWriter rtp_writer_;
diff --git a/remoting/protocol/rtp_video_writer_unittest.cc b/remoting/protocol/rtp_video_writer_unittest.cc
index ca8a8f4..be8d6c6 100644
--- a/remoting/protocol/rtp_video_writer_unittest.cc
+++ b/remoting/protocol/rtp_video_writer_unittest.cc
@@ -70,7 +70,7 @@ class RtpVideoWriterTest : public testing::Test {
   void InitPacket(int size, bool first, bool last) {
     InitData(size);
 
-    packet_.reset(new VideoPacket());
+    packet_ = new VideoPacket();
     packet_->mutable_format()->set_encoding(VideoPacketFormat::ENCODING_VP8);
     if (first)
       packet_->set_flags(packet_->flags() | VideoPacket::FIRST_PACKET);
@@ -112,12 +112,12 @@ class RtpVideoWriterTest : public testing::Test {
 
   MessageLoop message_loop_;
   vector<char> data_;
-  scoped_ptr<VideoPacket> packet_;
+  VideoPacket* packet_;
 };
 
 TEST_F(RtpVideoWriterTest, NotFragmented_FirstPacket) {
   InitPacket(1024, true, false);
-  writer_.SendPacket(*packet_);
+  writer_.ProcessVideoPacket(packet_, new DeleteTask<VideoPacket>(packet_));
   message_loop_.RunAllPending();
 
   ExpectedPacket expected[] = {
@@ -128,7 +128,7 @@ TEST_F(RtpVideoWriterTest, NotFragmented_FirstPacket) {
 
 TEST_F(RtpVideoWriterTest, NotFragmented_LastPackes) {
   InitPacket(1024, false, true);
-  writer_.SendPacket(*packet_);
+  writer_.ProcessVideoPacket(packet_, new DeleteTask<VideoPacket>(packet_));
   message_loop_.RunAllPending();
 
   ExpectedPacket expected[] = {
@@ -139,7 +139,7 @@ TEST_F(RtpVideoWriterTest, NotFragmented_LastPackes) {
 
 TEST_F(RtpVideoWriterTest, TwoFragments_FirstPacket) {
   InitPacket(2000, true, false);
-  writer_.SendPacket(*packet_);
+  writer_.ProcessVideoPacket(packet_, new DeleteTask<VideoPacket>(packet_));
   message_loop_.RunAllPending();
 
   ExpectedPacket expected[] = {
@@ -151,7 +151,7 @@ TEST_F(RtpVideoWriterTest, TwoFragments_FirstPacket) {
 
 TEST_F(RtpVideoWriterTest, TwoFragments_LastPacket) {
   InitPacket(2000, false, true);
-  writer_.SendPacket(*packet_);
+  writer_.ProcessVideoPacket(packet_, new DeleteTask<VideoPacket>(packet_));
   message_loop_.RunAllPending();
 
   ExpectedPacket expected[] = {
@@ -163,7 +163,7 @@ TEST_F(RtpVideoWriterTest, TwoFragments_LastPacket) {
 
 TEST_F(RtpVideoWriterTest, ThreeFragments) {
   InitPacket(3000, true, true);
-  writer_.SendPacket(*packet_);
+  writer_.ProcessVideoPacket(packet_, new DeleteTask<VideoPacket>(packet_));
   message_loop_.RunAllPending();
 
   ExpectedPacket expected[] = {
diff --git a/remoting/protocol/video_stub.h b/remoting/protocol/video_stub.h
index c1b6b11..d9c3714 100644
--- a/remoting/protocol/video_stub.h
+++ b/remoting/protocol/video_stub.h
@@ -23,6 +23,9 @@ class VideoStub {
   virtual void ProcessVideoPacket(const VideoPacket* video_packet,
                                   Task* done) = 0;
 
+  // Returns number of packets currently pending in the buffer.
+  virtual int GetPendingPackets() = 0;
+
  protected:
   VideoStub() { }
 
diff --git a/remoting/protocol/video_writer.h b/remoting/protocol/video_writer.h
index eafa2b4..b9216c0 100644
--- a/remoting/protocol/video_writer.h
+++ b/remoting/protocol/video_writer.h
@@ -11,18 +11,16 @@
 #define REMOTING_PROTOCOL_VIDEO_WRITER_H_
 
 #include "base/basictypes.h"
+#include "remoting/protocol/video_stub.h"
 
 namespace remoting {
-
-class VideoPacket;
-
 namespace protocol {
 
 class Session;
 class SessionConfig;
 
 // TODO(sergeyu): VideoWriter should implement VideoStub interface.
-class VideoWriter {
+class VideoWriter : public VideoStub {
  public:
   virtual ~VideoWriter();
 
@@ -31,14 +29,6 @@ class VideoWriter {
   // Initializes the writer.
   virtual void Init(Session* session) = 0;
 
-  // Sends the |packet|.
-  virtual void SendPacket(const VideoPacket& packet) = 0;
-
-  // Returns number of packets currently pending in the buffer.
-  virtual int GetPendingPackets() = 0;
-
-  virtual void Close() = 0;
-
  protected:
   VideoWriter() { }