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

webrtc/base/virtualsocket_unittest.cc

 /*
  *  Copyright 2006 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <math.h>
 #include <time.h>
 #if defined(WEBRTC_POSIX)
 #include <netinet/in.h>
 #endif
 
 #include <memory>
 
 #include "webrtc/base/arraysize.h"
+#include "webrtc/base/gunit.h"
 #include "webrtc/base/logging.h"
-#include "webrtc/base/gunit.h"
+#include "webrtc/base/ptr_util.h"
 #include "webrtc/base/testclient.h"
 #include "webrtc/base/testutils.h"
 #include "webrtc/base/thread.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/base/virtualsocketserver.h"
 
 using namespace rtc;
 
 using webrtc::testing::SSE_CLOSE;
 using webrtc::testing::SSE_ERROR;
@@ skipping 141 matching lines @@
   // to the default route.
   void TestDefaultRoute(const IPAddress& default_route) {
     ss_->SetDefaultRoute(default_route);
 
     // Create client1 bound to the any address.
     AsyncSocket* socket =
         ss_->CreateAsyncSocket(default_route.family(), SOCK_DGRAM);
     socket->Bind(EmptySocketAddressWithFamily(default_route.family()));
     SocketAddress client1_any_addr = socket->GetLocalAddress();
     EXPECT_TRUE(client1_any_addr.IsAnyIP());
-    TestClient* client1 = new TestClient(new AsyncUDPSocket(socket));
+    std::unique_ptr<TestClient> client1(
+        new TestClient(MakeUnique<AsyncUDPSocket>(socket)));

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

Why not use MakeUnique for TestClient too?

  auto client1 = MakeUnique<TestClient>(MakeUnique<AsyncUDPSocket>(socket));

(You have a bunch of these in this file.)

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

Done. I take it you don't want to see any use of the unique_ptr<T>(T*)
constructor? And in this case, I think auto is ok, since MakeUnique<T> provides
strong local type info.  

[kwiberg-webrtc, 2017/05/05 12:40:31]

No; as I said in an earlier comment, I think consistently using WrapUnique
instead makes more sense, since owning raw pointers are  easy enough to make
mistakes with that loudly calling out their use is a good choice.
Yes. With MakeUnique, there's never any reason to avoid auto.

 
     // Create client2 bound to the default route.
     AsyncSocket* socket2 =
         ss_->CreateAsyncSocket(default_route.family(), SOCK_DGRAM);
     socket2->Bind(SocketAddress(default_route, 0));
     SocketAddress client2_addr = socket2->GetLocalAddress();
     EXPECT_FALSE(client2_addr.IsAnyIP());
-    TestClient* client2 = new TestClient(new AsyncUDPSocket(socket2));
+    std::unique_ptr<TestClient> client2(
+        new TestClient(MakeUnique<AsyncUDPSocket>(socket2)));
 
     // Client1 sends to client2, client2 should see the default route as
     // client1's address.
     SocketAddress client1_addr;
     EXPECT_EQ(6, client1->SendTo("bizbaz", 6, client2_addr));
     EXPECT_TRUE(client2->CheckNextPacket("bizbaz", 6, &client1_addr));
     EXPECT_EQ(client1_addr,
               SocketAddress(default_route, client1_any_addr.port()));
 
     // Client2 can send back to client1's default route address.
     EXPECT_EQ(3, client2->SendTo("foo", 3, client1_addr));
     EXPECT_TRUE(client1->CheckNextPacket("foo", 3, &client2_addr));
   }
 
   void BasicTest(const SocketAddress& initial_addr) {
     AsyncSocket* socket = ss_->CreateAsyncSocket(initial_addr.family(),
                                                  SOCK_DGRAM);
     socket->Bind(initial_addr);
     SocketAddress server_addr = socket->GetLocalAddress();
     // Make sure VSS didn't switch families on us.
     EXPECT_EQ(server_addr.family(), initial_addr.family());
 
-    TestClient* client1 = new TestClient(new AsyncUDPSocket(socket));
+    std::unique_ptr<TestClient> client1(
+        new TestClient(MakeUnique<AsyncUDPSocket>(socket)));
     AsyncSocket* socket2 =
         ss_->CreateAsyncSocket(initial_addr.family(), SOCK_DGRAM);
-    TestClient* client2 = new TestClient(new AsyncUDPSocket(socket2));
+    std::unique_ptr<TestClient> client2(
+        new TestClient(MakeUnique<AsyncUDPSocket>(socket2)));
 
     SocketAddress client2_addr;
     EXPECT_EQ(3, client2->SendTo("foo", 3, server_addr));
     EXPECT_TRUE(client1->CheckNextPacket("foo", 3, &client2_addr));
 
     SocketAddress client1_addr;
     EXPECT_EQ(6, client1->SendTo("bizbaz", 6, client2_addr));
     EXPECT_TRUE(client2->CheckNextPacket("bizbaz", 6, &client1_addr));
     EXPECT_EQ(client1_addr, server_addr);
 
     SocketAddress empty = EmptySocketAddressWithFamily(initial_addr.family());
     for (int i = 0; i < 10; i++) {
-      client2 = new TestClient(AsyncUDPSocket::Create(ss_, empty));
+      client2.reset(
+          new TestClient(WrapUnique(AsyncUDPSocket::Create(ss_, empty))));
 
       SocketAddress next_client2_addr;
       EXPECT_EQ(3, client2->SendTo("foo", 3, server_addr));
       EXPECT_TRUE(client1->CheckNextPacket("foo", 3, &next_client2_addr));
       CheckPortIncrementalization(next_client2_addr, client2_addr);
       // EXPECT_EQ(next_client2_addr.port(), client2_addr.port() + 1);
 
       SocketAddress server_addr2;
       EXPECT_EQ(6, client1->SendTo("bizbaz", 6, next_client2_addr));
       EXPECT_TRUE(client2->CheckNextPacket("bizbaz", 6, &server_addr2));
@@ skipping 553 matching lines @@
 
   // Test cross-family datagram sending between a client bound to client_addr
   // and a server bound to server_addr. shouldSucceed indicates if sending is
   // expected to succeed or not.
   void CrossFamilyDatagramTest(const SocketAddress& client_addr,
                                const SocketAddress& server_addr,
                                bool shouldSucceed) {
     AsyncSocket* socket = ss_->CreateAsyncSocket(SOCK_DGRAM);
     socket->Bind(server_addr);
     SocketAddress bound_server_addr = socket->GetLocalAddress();
-    TestClient* client1 = new TestClient(new AsyncUDPSocket(socket));
+    std::unique_ptr<TestClient> client1(
+        new TestClient(MakeUnique<AsyncUDPSocket>(socket)));
 
     AsyncSocket* socket2 = ss_->CreateAsyncSocket(SOCK_DGRAM);
     socket2->Bind(client_addr);
-    TestClient* client2 = new TestClient(new AsyncUDPSocket(socket2));
+    std::unique_ptr<TestClient> client2(
+        new TestClient(MakeUnique<AsyncUDPSocket>(socket2)));
     SocketAddress client2_addr;
 
     if (shouldSucceed) {
       EXPECT_EQ(3, client2->SendTo("foo", 3, bound_server_addr));
       EXPECT_TRUE(client1->CheckNextPacket("foo", 3, &client2_addr));
       SocketAddress client1_addr;
       EXPECT_EQ(6, client1->SendTo("bizbaz", 6, client2_addr));
       EXPECT_TRUE(client2->CheckNextPacket("bizbaz", 6, &client1_addr));
       EXPECT_EQ(client1_addr, bound_server_addr);
     } else {
@@ skipping 204 matching lines @@
                           true);
 }
 
 TEST_F(VirtualSocketServerTest, SetSendingBlockedWithUdpSocket) {
   AsyncSocket* socket1 =
       ss_->CreateAsyncSocket(kIPv4AnyAddress.family(), SOCK_DGRAM);
   AsyncSocket* socket2 =
       ss_->CreateAsyncSocket(kIPv4AnyAddress.family(), SOCK_DGRAM);
   socket1->Bind(kIPv4AnyAddress);
   socket2->Bind(kIPv4AnyAddress);
-  TestClient* client1 = new TestClient(new AsyncUDPSocket(socket1));
+  std::unique_ptr<TestClient> client1(
+      new TestClient(MakeUnique<AsyncUDPSocket>(socket1)));
 
   ss_->SetSendingBlocked(true);
   EXPECT_EQ(-1, client1->SendTo("foo", 3, socket2->GetLocalAddress()));
   EXPECT_TRUE(socket1->IsBlocking());
   EXPECT_EQ(0, client1->ready_to_send_count());
 
   ss_->SetSendingBlocked(false);
   EXPECT_EQ(1, client1->ready_to_send_count());
   EXPECT_EQ(3, client1->SendTo("foo", 3, socket2->GetLocalAddress()));
 }
@@ skipping 76 matching lines @@
           << " N=" << kTestSamples[sidx];
         EXPECT_NEAR(kStdDev, stddev, 0.1 * kStdDev)
           << "M=" << kTestMean[midx]
           << " SD=" << kStdDev
           << " N=" << kTestSamples[sidx];
         delete f;
       }
     }
   }
 }