Refactor TestClient to use std::unique_ptr, and fix VirtualSocketServerTest leaks.

webrtc/base/testclient.cc

Index: webrtc/base/testclient.cc
diff --git a/webrtc/base/testclient.cc b/webrtc/base/testclient.cc
index e797c6fa5947cc261dc1c14d06465e76624e661a..7636dd9063a4258208652e48180b66939b3d2f2d 100644
--- a/webrtc/base/testclient.cc
+++ b/webrtc/base/testclient.cc
@@ -9,6 +9,7 @@
  */
 
 #include "webrtc/base/testclient.h"
+#include "webrtc/base/ptr_util.h"
 #include "webrtc/base/thread.h"
 #include "webrtc/base/timeutils.h"
 
@@ -18,19 +19,13 @@ namespace rtc {
 //         Callers can retrieve received packets from any thread by calling
 //         NextPacket.
 
-TestClient::TestClient(AsyncPacketSocket* socket)
-    : socket_(socket), prev_packet_timestamp_(-1) {
-  packets_ = new std::vector<Packet*>();
+TestClient::TestClient(std::unique_ptr<AsyncPacketSocket> socket)
+    : socket_(std::move(socket)), prev_packet_timestamp_(-1) {
   socket_->SignalReadPacket.connect(this, &TestClient::OnPacket);
   socket_->SignalReadyToSend.connect(this, &TestClient::OnReadyToSend);
 }
 
-TestClient::~TestClient() {
-  delete socket_;
-  for (unsigned i = 0; i < packets_->size(); i++)
-    delete (*packets_)[i];
-  delete packets_;
-}
+TestClient::~TestClient() {}
 
 bool TestClient::CheckConnState(AsyncPacketSocket::State state) {
   // Wait for our timeout value until the socket reaches the desired state.
@@ -52,7 +47,7 @@ int TestClient::SendTo(const char* buf, size_t size,
   return socket_->SendTo(buf, size, dest, options);
 }
 
-TestClient::Packet* TestClient::NextPacket(int timeout_ms) {
+std::unique_ptr<TestClient::Packet> TestClient::NextPacket(int timeout_ms) {
   // If no packets are currently available, we go into a get/dispatch loop for
   // at most timeout_ms.  If, during the loop, a packet arrives, then we can
   // stop early and return it.
@@ -68,7 +63,7 @@ TestClient::Packet* TestClient::NextPacket(int timeout_ms) {
   while (TimeUntil(end) > 0) {
     {
       CritScope cs(&crit_);
-      if (packets_->size() != 0) {
+      if (packets_.size() != 0) {
         break;
       }
     }
@@ -76,11 +71,11 @@ TestClient::Packet* TestClient::NextPacket(int timeout_ms) {
   }
 
   // Return the first packet placed in the queue.
-  Packet* packet = nullptr;
+  std::unique_ptr<Packet> packet;
   CritScope cs(&crit_);
-  if (packets_->size() > 0) {
-    packet = packets_->front();
-    packets_->erase(packets_->begin());
+  if (packets_.size() > 0) {
+    packet = std::move(packets_.front());
+    packets_.erase(packets_.begin());
   }
 
   return packet;
@@ -89,13 +84,12 @@ TestClient::Packet* TestClient::NextPacket(int timeout_ms) {
 bool TestClient::CheckNextPacket(const char* buf, size_t size,
                                  SocketAddress* addr) {
   bool res = false;
-  Packet* packet = NextPacket(kTimeoutMs);
+  std::unique_ptr<Packet> packet = NextPacket(kTimeoutMs);
   if (packet) {
     res = (packet->size == size && memcmp(packet->buf, buf, size) == 0 &&
            CheckTimestamp(packet->packet_time.timestamp));
     if (addr)
       *addr = packet->addr;
-    delete packet;
   }
   return res;
 }
@@ -116,9 +110,8 @@ bool TestClient::CheckTimestamp(int64_t packet_timestamp) {
 
 bool TestClient::CheckNoPacket() {
   bool res;
-  Packet* packet = NextPacket(kNoPacketTimeoutMs);
+  std::unique_ptr<Packet> packet = NextPacket(kNoPacketTimeoutMs);
   res = (packet == nullptr);
-  delete packet;
   return res;

[kwiberg-webrtc, 2017/05/05 11:04:42]

Combine the two statements on lines 114 and 115, now that they don't have to
have a delete in between them? (Well, strictly speaking they didn't *have* to
before either...)

Hmm. In fact, the whole function body collapses to something like

  return NextPacket(kNoPacketTimeoutMs) == nullptr;

[nisse-webrtc, 2017/05/05 12:12:56]

Nice. Done.

 }
 
@@ -134,7 +127,7 @@ void TestClient::OnPacket(AsyncPacketSocket* socket, const char* buf,
                           size_t size, const SocketAddress& remote_addr,
                           const PacketTime& packet_time) {
   CritScope cs(&crit_);
-  packets_->push_back(new Packet(remote_addr, buf, size, packet_time));
+  packets_.push_back(MakeUnique<Packet>(remote_addr, buf, size, packet_time));
 }
 
 void TestClient::OnReadyToSend(AsyncPacketSocket* socket) {