DnsTransaction::RecordLostPacketsIfAny shouldn't crash if some attempts failed to connect and thus …

net/dns/dns_transaction_unittest.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 "net/dns/dns_transaction.h"
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/rand_util.h"
@@ skipping 167 matching lines @@
 class TestSocketFactory : public MockClientSocketFactory {
  public:
   TestSocketFactory() : create_failing_sockets_(false) {}
   virtual ~TestSocketFactory() {}
 
   virtual DatagramClientSocket* CreateDatagramClientSocket(
       DatagramSocket::BindType bind_type,
       const RandIntCallback& rand_int_cb,
       net::NetLog* net_log,
       const net::NetLog::Source& source) OVERRIDE {
-    if (create_failing_sockets_)
+    if (create_failing_sockets_) {

[szym, 2013/05/29 17:09:42]

Suggest renaming this field to |fail_next_socket_| since after creating one
socket the behavior reverts to normal.

[mef, 2013/05/29 17:24:04]

Done.

+      create_failing_sockets_ = false;
       return new FailingUDPClientSocket(&empty_data_, net_log);
+    }
     SocketDataProvider* data_provider = mock_data().GetNext();
     TestUDPClientSocket* socket = new TestUDPClientSocket(this,
                                                           data_provider,
                                                           net_log);
     data_provider->set_socket(socket);
     return socket;
   }
 
   void OnConnect(const IPEndPoint& endpoint) {
     remote_endpoints_.push_back(endpoint);
@@ skipping 658 matching lines @@
 }
 
 TEST_F(DnsTransactionTest, ConnectFailure) {
   socket_factory_->create_failing_sockets_ = true;
   transaction_ids_.push_back(0);  // Needed to make a DnsUDPAttempt.
   TransactionHelper helper0("www.chromium.org", dns_protocol::kTypeA,
                             ERR_CONNECTION_REFUSED);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
+TEST_F(DnsTransactionTest, ConnectFailureDoesntCrashInRecordLostPacketsIfAny) {

[szym, 2013/05/29 17:09:42]

suggest just calling the test "ConnectFailureFollowedBySuccess"

[mef, 2013/05/29 17:24:04]

Done.

+  // Fallback on server failure.

[szym, 2013/05/29 17:09:42]

suggest: "// Retry after failure."

[mef, 2013/05/29 17:24:04]

Done.

+  config_.attempts = 2;
+  ConfigureFactory();
+  // First server connection attempt fails.
+  transaction_ids_.push_back(0);  // Needed to make a DnsUDPAttempt.
+  socket_factory_->create_failing_sockets_ = true;
+  // Second DNS query succeeds.
+  AddAsyncQueryAndResponse(0 /* id */, kT0HostName, kT0Qtype,
+                           kT0ResponseDatagram, arraysize(kT0ResponseDatagram));
+  TransactionHelper helper0(kT0HostName, kT0Qtype, kT0RecordCount);
+  // Expect no crash here!

[szym, 2013/05/29 17:09:42]

unnecessary comment

[mef, 2013/05/29 17:24:04]

Done.

+  EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
+}
+
 TEST_F(DnsTransactionTest, TCPLookup) {
   AddAsyncQueryAndRcode(kT0HostName, kT0Qtype,
                         dns_protocol::kRcodeNOERROR | dns_protocol::kFlagTC);
   AddQueryAndResponse(0 /* id */, kT0HostName, kT0Qtype,
                       kT0ResponseDatagram, arraysize(kT0ResponseDatagram),
                       ASYNC, true /* use_tcp */);
 
   TransactionHelper helper0(kT0HostName, kT0Qtype, kT0RecordCount);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
@@ skipping 42 matching lines @@
   config_.timeout = TestTimeouts::tiny_timeout();
   ConfigureFactory();
 
   TransactionHelper helper0(".", dns_protocol::kTypeA, ERR_INVALID_ARGUMENT);
   EXPECT_TRUE(helper0.Run(transaction_factory_.get()));
 }
 
 }  // namespace
 
 }  // namespace net