Multicast DNS implementation (initial)

net/dns/mdns_client_unittest.cc

Index: net/dns/mdns_client_unittest.cc
diff --git a/net/dns/mdns_client_unittest.cc b/net/dns/mdns_client_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3cadc129250d46c2b7707bfbb8639adba59978c8
--- /dev/null
+++ b/net/dns/mdns_client_unittest.cc
@@ -0,0 +1,1066 @@
+// Copyright (c) 2013 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 <queue>
+
+#include "base/memory/ref_counted.h"
+#include "base/message_loop.h"
+#include "net/base/rand_callback.h"
+#include "net/base/test_completion_callback.h"
+#include "net/dns/mdns_client_impl.h"
+#include "net/dns/record_rdata.h"
+#include "net/udp/udp_client_socket.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::Invoke;
+using ::testing::InvokeWithoutArgs;
+using ::testing::StrictMock;
+using ::testing::NiceMock;
+using ::testing::Exactly;
+using ::testing::Return;
+using ::testing::SaveArg;
+using ::testing::_;
+
+namespace net {
+
+namespace {
+
+const char kSamplePacket1[] = {
+    // Header
+    0x00, 0x00,               // ID is zeroed out
+    0x81, 0x80,               // Standard query response, RA, no error
+    0x00, 0x00,               // No questions (for simplicity)
+    0x00, 0x02,               // 2 RRs (answers)
+    0x00, 0x00,               // 0 authority RRs
+    0x00, 0x00,               // 0 additional RRs
+
+    // Answer 1
+    0x07, '_', 'p', 'r', 'i', 'v', 'e', 't',
+    0x04, '_', 't', 'c', 'p',
+    0x05, 'l', 'o', 'c', 'a', 'l',
+    0x00,
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x00,        // TTL (4 bytes) is 1 second;
+    0x00, 0x01,
+    0x00, 0x08,        // RDLENGTH is 8 bytes.
+    0x05, 'h', 'e', 'l', 'l', 'o',
+    0xc0, 0x0c,
+
+    // Answer 2
+    0x08, '_', 'p', 'r', 'i', 'n', 't', 'e', 'r',
+    0xc0, 0x14,         // Pointer to "._tcp.local"
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x01,        // TTL (4 bytes) is 20 hours, 47 minutes, 49 seconds.
+    0x24, 0x75,
+    0x00, 0x08,        // RDLENGTH is 8 bytes.
+    0x05, 'h', 'e', 'l', 'l', 'o',
+    0xc0, 0x32
+  };
+
+const char kCorruptedPacketBadQuestion[] = {
+    // Header
+    0x00, 0x00,               // ID is zeroed out
+    0x81, 0x80,               // Standard query response, RA, no error
+    0x00, 0x01,               // One question
+    0x00, 0x02,               // 2 RRs (answers)
+    0x00, 0x00,               // 0 authority RRs
+    0x00, 0x00,               // 0 additional RRs
+
+    // Question is corrupted and cannot be read.
+    0x99, 'h', 'e', 'l', 'l', 'o',
+    0x00,
+    0x00, 0x00,
+    0x00, 0x00,
+
+    // Answer 1
+    0x07, '_', 'p', 'r', 'i', 'v', 'e', 't',
+    0x04, '_', 't', 'c', 'p',
+    0x05, 'l', 'o', 'c', 'a', 'l',
+    0x00,
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x01,        // TTL (4 bytes) is 20 hours, 47 minutes, 48 seconds.
+    0x24, 0x74,
+    0x00, 0x99,        // RDLENGTH is impossible
+    0x05, 'h', 'e', 'l', 'l', 'o',
+    0xc0, 0x0c,
+
+    // Answer 2
+    0x08, '_', 'p', 'r',  // Useless trailing data.
+};
+
+const char kCorruptedPacketUnsalvagable[] = {
+    // Header
+    0x00, 0x00,               // ID is zeroed out
+    0x81, 0x80,               // Standard query response, RA, no error
+    0x00, 0x00,               // No questions (for simplicity)
+    0x00, 0x02,               // 2 RRs (answers)
+    0x00, 0x00,               // 0 authority RRs
+    0x00, 0x00,               // 0 additional RRs
+
+    // Answer 1
+    0x07, '_', 'p', 'r', 'i', 'v', 'e', 't',
+    0x04, '_', 't', 'c', 'p',
+    0x05, 'l', 'o', 'c', 'a', 'l',
+    0x00,
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x01,        // TTL (4 bytes) is 20 hours, 47 minutes, 48 seconds.
+    0x24, 0x74,
+    0x00, 0x99,        // RDLENGTH is impossible
+    0x05, 'h', 'e', 'l', 'l', 'o',
+    0xc0, 0x0c,
+
+    // Answer 2
+    0x08, '_', 'p', 'r',  // Useless trailing data.
+};
+
+const char kCorruptedPacketSalvagable[] = {
+      // Header
+    0x00, 0x00,               // ID is zeroed out
+    0x81, 0x80,               // Standard query response, RA, no error
+    0x00, 0x00,               // No questions (for simplicity)
+    0x00, 0x02,               // 2 RRs (answers)
+    0x00, 0x00,               // 0 authority RRs
+    0x00, 0x00,               // 0 additional RRs
+
+    // Answer 1
+    0x07, '_', 'p', 'r', 'i', 'v', 'e', 't',
+    0x04, '_', 't', 'c', 'p',
+    0x05, 'l', 'o', 'c', 'a', 'l',
+    0x00,
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x01,        // TTL (4 bytes) is 20 hours, 47 minutes, 48 seconds.
+    0x24, 0x74,
+    0x00, 0x08,        // RDLENGTH is 8 bytes.
+    0x99, 'h', 'e', 'l', 'l', 'o',   // Bad RDATA format.
+    0xc0, 0x0c,
+
+    // Answer 2
+    0x08, '_', 'p', 'r', 'i', 'n', 't', 'e', 'r',
+    0xc0, 0x14,         // Pointer to "._tcp.local"
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x01,        // TTL (4 bytes) is 20 hours, 47 minutes, 49 seconds.
+    0x24, 0x75,
+    0x00, 0x08,        // RDLENGTH is 8 bytes.
+    0x05, 'h', 'e', 'l', 'l', 'o',
+    0xc0, 0x32
+};
+
+const char kSamplePacket2[] = {
+    // Header
+    0x00, 0x00,               // ID is zeroed out
+    0x81, 0x80,               // Standard query response, RA, no error
+    0x00, 0x00,               // No questions (for simplicity)
+    0x00, 0x02,               // 2 RRs (answers)
+    0x00, 0x00,               // 0 authority RRs
+    0x00, 0x00,               // 0 additional RRs
+
+    // Answer 1
+    0x07, '_', 'p', 'r', 'i', 'v', 'e', 't',
+    0x04, '_', 't', 'c', 'p',
+    0x05, 'l', 'o', 'c', 'a', 'l',
+    0x00,
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x01,        // TTL (4 bytes) is 20 hours, 47 minutes, 48 seconds.
+    0x24, 0x74,
+    0x00, 0x08,        // RDLENGTH is 8 bytes.
+    0x05, 'z', 'z', 'z', 'z', 'z',
+    0xc0, 0x0c,
+
+    // Answer 2
+    0x08, '_', 'p', 'r', 'i', 'n', 't', 'e', 'r',
+    0xc0, 0x14,         // Pointer to "._tcp.local"
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x01,        // TTL (4 bytes) is 20 hours, 47 minutes, 48 seconds.
+    0x24, 0x74,
+    0x00, 0x08,        // RDLENGTH is 8 bytes.
+    0x05, 'z', 'z', 'z', 'z', 'z',
+    0xc0, 0x32
+  };
+
+const char kQueryPacketPrivet[] = {
+  // Header
+  0x00, 0x00,               // ID is zeroed out
+  0x00, 0x00,               // No flags.
+  0x00, 0x01,               // One question.
+  0x00, 0x00,               // 0 RRs (answers)
+  0x00, 0x00,               // 0 authority RRs
+  0x00, 0x00,               // 0 additional RRs
+
+  // Question
+  // This part is echoed back from the respective query.
+  0x07, '_', 'p', 'r', 'i', 'v', 'e', 't',
+  0x04, '_', 't', 'c', 'p',
+  0x05, 'l', 'o', 'c', 'a', 'l',
+  0x00,
+  0x00, 0x0c,        // TYPE is PTR.
+  0x00, 0x01,        // CLASS is IN.
+};
+
+const char kSamplePacketAdditionalOnly[] = {
+    // Header
+    0x00, 0x00,               // ID is zeroed out
+    0x81, 0x80,               // Standard query response, RA, no error
+    0x00, 0x00,               // No questions (for simplicity)
+    0x00, 0x00,               // 2 RRs (answers)
+    0x00, 0x00,               // 0 authority RRs
+    0x00, 0x01,               // 0 additional RRs
+
+    // Answer 1
+    0x07, '_', 'p', 'r', 'i', 'v', 'e', 't',
+    0x04, '_', 't', 'c', 'p',
+    0x05, 'l', 'o', 'c', 'a', 'l',
+    0x00,
+    0x00, 0x0c,        // TYPE is PTR.
+    0x00, 0x01,        // CLASS is IN.
+    0x00, 0x01,        // TTL (4 bytes) is 20 hours, 47 minutes, 48 seconds.
+    0x24, 0x74,
+    0x00, 0x08,        // RDLENGTH is 8 bytes.
+    0x05, 'h', 'e', 'l', 'l', 'o',
+    0xc0, 0x0c,
+};
+
+class MockDatagramServerSocket : public DatagramServerSocket {
+ public:
+  int Listen(const IPEndPoint& address) {

[szym, 2013/06/12 21:35:41]

// DatagramServerSocket implementation

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+    return ListenInternal(address.ToString());
+  }
+
+  MOCK_METHOD1(ListenInternal, int(const std::string& address));
+
+  void SetResponsePacket(std::string response_packet) {

[szym, 2013/06/12 21:35:41]

These are not part of DatagramServerSocket so move them from here.

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+    response_packet_ = response_packet;
+  }
+
+  int RespondImmediately(IOBuffer* buffer, int size, IPEndPoint* address,

[szym, 2013/06/12 21:35:41]

The name is confusing. How about HandleRecv? HandleRecvNow? HandleRecvLater?

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+                         const CompletionCallback& callback) {
+    int to_copy = std::min(response_packet_.size(), (size_t)size);

[szym, 2013/06/12 21:35:41]

size_returned would be more clear than to_copy

static_cast<size_t>

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+    memcpy(buffer->data(), response_packet_.data(), to_copy);
+    return to_copy;
+  }
+
+  int RespondDelayed(IOBuffer* buffer, int size, IPEndPoint* address,
+                     const CompletionCallback& callback) {
+    int rv = RespondImmediately(buffer, size, address, callback);
+    base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, rv));
+    return ERR_IO_PENDING;
+  }
+
+  MOCK_METHOD4(RecvFrom, int(IOBuffer* buffer, int size,
+                             IPEndPoint* address,
+                             const CompletionCallback& callback));
+
+  int SendTo(IOBuffer* buf, int buf_len, const IPEndPoint& address,
+             const CompletionCallback& callback) {
+    return SendToInternal(std::string(buf->data(), buf_len), address.ToString(),
+                          callback);
+  }
+
+  MOCK_METHOD3(SendToInternal, int(const std::string& packet,
+                                   const std::string address,
+                                   const CompletionCallback& callback));
+
+  MOCK_METHOD1(SetReceiveBufferSize, bool(int32 size));
+  MOCK_METHOD1(SetSendBufferSize, bool(int32 size));
+
+  MOCK_METHOD0(Close, void());
+
+  MOCK_CONST_METHOD1(GetPeerAddress, int(IPEndPoint* address));
+  MOCK_CONST_METHOD1(GetLocalAddress, int(IPEndPoint* address));
+  MOCK_CONST_METHOD0(NetLog, const BoundNetLog&());
+
+  MOCK_METHOD0(AllowAddressReuse, void());
+  MOCK_METHOD0(AllowBroadcast, void());
+
+  int JoinGroup(const IPAddressNumber& group_address) const {
+    return JoinGroupInternal(IPAddressToString(group_address));
+  }
+
+  MOCK_CONST_METHOD1(JoinGroupInternal, int(const std::string& group));
+
+  int LeaveGroup(const IPAddressNumber& group_address) const {
+    return LeaveGroupInternal(IPAddressToString(group_address));
+  }
+
+  MOCK_CONST_METHOD1(LeaveGroupInternal, int(const std::string& group));
+
+  MOCK_METHOD1(SetMulticastTimeToLive, int(int ttl));
+
+  MOCK_METHOD1(SetMulticastLoopbackMode, int(bool loopback));
+
+ private:
+  std::string response_packet_;
+};
+
+class MockDatagramServerSocketFactory
+    : public MDnsDatagramServerSocketFactory {
+ public:
+  MockDatagramServerSocketFactory() {
+  }
+
+  virtual ~MockDatagramServerSocketFactory() {
+  }
+
+  virtual scoped_ptr<DatagramServerSocket> CreateSocket() OVERRIDE {
+    scoped_ptr<MockDatagramServerSocket> new_socket(
+        new NiceMock<MockDatagramServerSocket>);
+
+    ON_CALL(*new_socket, SendToInternal(_, _, _))
+        .WillByDefault(Invoke(
+            this,
+            &MockDatagramServerSocketFactory::SendToInternal));
+
+    ON_CALL(*new_socket, RecvFrom(_, _, _, _))
+        .WillByDefault(Invoke(
+            this,
+            &MockDatagramServerSocketFactory::RecvFromInternal));
+
+    return new_socket.PassAs<DatagramServerSocket>();
+  }
+
+  int SendToInternal(const std::string& packet, const std::string& address,

[szym, 2013/06/12 21:35:41]

Does it need to be public?

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+                     const CompletionCallback& callback) {
+    OnSendTo(packet);
+    return packet.size();
+  }
+
+  // The latest recieve callback is always saved, since the MDnsConnection

[szym, 2013/06/12 21:35:41]

nit: receive

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+  // does not care which socket a packet is received on.
+  int RecvFromInternal(IOBuffer* buffer, int size,
+                       IPEndPoint* address,
+                       const CompletionCallback& callback) {
+    recv_buffer_ = buffer;
+    recv_buffer_size_ = size;
+    recv_callback_ = callback;
+    return ERR_IO_PENDING;
+  }
+
+  void SimulateReceive(const char* packet, int size) {
+    DCHECK(recv_buffer_size_ >= size);
+    DCHECK(recv_buffer_.get());
+    DCHECK(!recv_callback_.is_null());
+
+    memcpy(recv_buffer_->data(), packet, size);
+    CompletionCallback recv_callback = recv_callback_;
+    recv_callback_.Reset();
+    recv_callback.Run(size);
+  }
+
+  MOCK_METHOD1(OnSendTo, void(const std::string&));
+
+ private:
+  scoped_refptr<IOBuffer> recv_buffer_;
+  int recv_buffer_size_;
+  CompletionCallback recv_callback_;
+};
+
+class PtrRecordCopyContainer {
+ public:
+  PtrRecordCopyContainer() {}
+  ~PtrRecordCopyContainer() {}
+
+  bool is_set() const { return set_; }
+
+  void SaveWithDummyArg(int unused, const RecordParsed* value) {
+    Save(value);
+  }
+
+  void Save(const RecordParsed* value) {
+    set_ = true;
+    name_ = value->name();
+    ptrdomain_ = value->rdata<PtrRecordRdata>()->ptrdomain();
+    ttl_ = value->ttl();
+  }
+
+  bool IsRecordWith(std::string name, std::string ptrdomain) {
+    return set_ && name_ == name && ptrdomain_ == ptrdomain;
+  }
+
+  const std::string& name() { return name_; }
+  const std::string& ptrdomain() { return ptrdomain_; }
+  int ttl() { return ttl_; }
+
+ private:
+  bool set_;
+  std::string name_;
+  std::string ptrdomain_;
+  int ttl_;
+};
+
+class MDnsTest : public ::testing::Test {
+ public:
+  MDnsTest();
+  virtual ~MDnsTest();
+  void DeleteTransaction();
+  void DeleteBothListeners();
+  void RunUntilIdle();
+  void RunFor(base::TimeDelta time_period);
+  void Stop();
+
+  MOCK_METHOD2(MockableRecordCallback, void(MDnsTransactionResult result,
+                                            const RecordParsed* record));
+
+ protected:
+  void ExpectPacket(const char* packet, unsigned size);
+  void SimulatePacketRecieve(const char* packet, unsigned size);

[szym, 2013/06/12 21:35:41]

nit: Receive

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+
+  base::MessageLoop* message_loop_;
+
+  scoped_ptr<MDnsClientImpl> test_client_;
+  IPEndPoint mdns_ipv4_endpoint_;
+  StrictMock<MockDatagramServerSocketFactory>* socket_factory_;
+
+  // Transactions and listeners that can be deleted by class methods for
+  // reentrancy tests.
+  scoped_ptr<MDnsTransaction> transaction_;
+  scoped_ptr<MDnsListener> listener1_;
+  scoped_ptr<MDnsListener> listener2_;
+};
+
+class MockListenerDelegate : public MDnsListener::Delegate {
+ public:
+  MOCK_METHOD2(OnRecordUpdate,
+               void(MDnsUpdateType update, const RecordParsed* records));
+  MOCK_METHOD2(OnNsecRecord, void(const std::string&, unsigned));
+  MOCK_METHOD0(OnCachePurged, void());
+};
+
+MDnsTest::MDnsTest()
+    : message_loop_(base::MessageLoop::current()) {
+  socket_factory_ = new StrictMock<MockDatagramServerSocketFactory>();
+  test_client_.reset(new MDnsClientImpl(socket_factory_));
+}
+
+MDnsTest::~MDnsTest() {
+}
+
+void MDnsTest::SimulatePacketRecieve(const char* packet, unsigned size) {
+  socket_factory_->SimulateReceive(packet, size);
+}
+
+void MDnsTest::ExpectPacket(
+    const char* packet,
+    unsigned size) {
+  EXPECT_CALL(*socket_factory_, OnSendTo(std::string(packet, size)))
+      .Times(2);
+}
+
+void MDnsTest::DeleteTransaction() {
+  transaction_.reset();
+}
+
+void MDnsTest::DeleteBothListeners() {
+  listener1_.reset();
+  listener2_.reset();
+}
+
+void MDnsTest::RunUntilIdle() {
+  base::MessageLoop::current()->RunUntilIdle();
+}
+
+void MDnsTest::RunFor(base::TimeDelta time_period) {
+  base::CancelableCallback<void()> callback(base::Bind(&MDnsTest::Stop,
+                                                       base::Unretained(this)));
+  base::MessageLoop::current()->PostDelayedTask(
+      FROM_HERE, callback.callback(), time_period);
+
+  base::MessageLoop::current()->Run();
+  callback.Cancel();
+}
+
+void MDnsTest::Stop() {
+  base::MessageLoop::current()->Quit();
+}
+
+TEST_F(MDnsTest, PassiveListeners) {
+  StrictMock<MockListenerDelegate> delegate_privet;
+  StrictMock<MockListenerDelegate> delegate_printer;
+  StrictMock<MockListenerDelegate> delegate_ptr;
+
+  PtrRecordCopyContainer record_privet;
+  PtrRecordCopyContainer record_printer;
+
+  scoped_ptr<MDnsListener> listener_privet = test_client_->CreateListener(
+      dns_protocol::kTypePTR, "_privet._tcp.local", &delegate_privet);
+  scoped_ptr<MDnsListener> listener_printer = test_client_->CreateListener(
+      dns_protocol::kTypePTR, "_printer._tcp.local", &delegate_printer);
+  scoped_ptr<MDnsListener> listener_ptr = test_client_->CreateListener(
+      dns_protocol::kTypePTR, "", &delegate_ptr);
+
+  listener_privet->Start();

[szym, 2013/06/12 21:35:41]

Should you ASSERT_TRUE all Start() calls (both listeners and transactions)?

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+  listener_printer->Start();
+  listener_ptr->Start();
+
+  ASSERT_TRUE(test_client_->IsListeningForTests());
+
+  // Send the same packet twice to ensure no records are double-counted.
+
+  EXPECT_CALL(delegate_privet, OnRecordUpdate(kMDnsRecordAdded, _))
+      .Times(Exactly(1))
+      .WillOnce(Invoke(
+          &record_privet,
+          &PtrRecordCopyContainer::SaveWithDummyArg));
+
+  EXPECT_CALL(delegate_printer, OnRecordUpdate(kMDnsRecordAdded, _))
+      .Times(Exactly(1))
+      .WillOnce(Invoke(
+          &record_printer,
+          &PtrRecordCopyContainer::SaveWithDummyArg));
+
+  EXPECT_CALL(delegate_ptr, OnRecordUpdate(kMDnsRecordAdded, _))
+      .Times(Exactly(2));
+
+  SimulatePacketRecieve(kSamplePacket1, sizeof(kSamplePacket1));
+  SimulatePacketRecieve(kSamplePacket1, sizeof(kSamplePacket1));
+
+  RunUntilIdle();

[szym, 2013/06/12 21:35:41]

I don't think you need to run the loop here. It should already be idle.

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+
+  EXPECT_TRUE(record_privet.IsRecordWith("_privet._tcp.local",
+                                         "hello._privet._tcp.local"));
+
+  EXPECT_TRUE(record_printer.IsRecordWith("_printer._tcp.local",
+                                          "hello._printer._tcp.local"));
+
+  listener_privet.reset();
+  listener_printer.reset();
+
+  ASSERT_TRUE(test_client_->IsListeningForTests());
+
+  EXPECT_CALL(delegate_ptr, OnRecordUpdate(kMDnsRecordAdded, _))
+      .Times(Exactly(2));
+
+  SimulatePacketRecieve(kSamplePacket2, sizeof(kSamplePacket2));
+
+  RunUntilIdle();
+
+  // Test to make sure mdns listener is not active with no listeners present.
+  listener_ptr.reset();
+
+  RunUntilIdle();
+
+  ASSERT_FALSE(test_client_->IsListeningForTests());
+}
+
+TEST_F(MDnsTest, PassiveListenersCleanup) {

[szym, 2013/06/12 21:35:41]

I suggest PassiveListenersCacheCleanup.

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+  StrictMock<MockListenerDelegate> delegate_privet;
+
+  PtrRecordCopyContainer record_privet;
+  PtrRecordCopyContainer record_privet2;
+
+  scoped_ptr<MDnsListener> listener_privet = test_client_->CreateListener(
+      dns_protocol::kTypePTR, "_privet._tcp.local", &delegate_privet);
+
+  listener_privet->Start();
+
+  ASSERT_TRUE(test_client_->IsListeningForTests());
+
+  EXPECT_CALL(delegate_privet, OnRecordUpdate(kMDnsRecordAdded, _))
+      .Times(Exactly(1))
+      .WillOnce(Invoke(
+          &record_privet,
+          &PtrRecordCopyContainer::SaveWithDummyArg));
+
+  SimulatePacketRecieve(kSamplePacket1, sizeof(kSamplePacket1));
+
+  RunUntilIdle();
+
+  EXPECT_TRUE(record_privet.IsRecordWith("_privet._tcp.local",
+                                         "hello._privet._tcp.local"));
+
+  EXPECT_CALL(delegate_privet, OnRecordUpdate(kMDnsRecordRemoved, _))

[szym, 2013/06/12 21:35:41]

Add a comment "Expect record is removed when its TTL expires."

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+      .Times(Exactly(1))
+      .WillOnce(DoAll(InvokeWithoutArgs(this, &MDnsTest::Stop),
+                      Invoke(&record_privet2,
+                             &PtrRecordCopyContainer::SaveWithDummyArg)));
+
+  RunFor(base::TimeDelta::FromSeconds(record_privet.ttl() + 1));
+
+  RunUntilIdle();
+
+  EXPECT_TRUE(record_privet2.IsRecordWith("_privet._tcp.local",
+                                          "hello._privet._tcp.local"));
+}
+
+TEST_F(MDnsTest, MalformedPacket) {
+  StrictMock<MockListenerDelegate> delegate_printer;
+
+  PtrRecordCopyContainer record_printer;
+
+  scoped_ptr<MDnsListener> listener_printer = test_client_->CreateListener(
+      dns_protocol::kTypePTR, "_printer._tcp.local", &delegate_printer);
+
+  listener_printer->Start();
+
+  ASSERT_TRUE(test_client_->IsListeningForTests());
+
+  EXPECT_CALL(delegate_printer, OnRecordUpdate(kMDnsRecordAdded, _))
+      .Times(Exactly(1))
+      .WillOnce(Invoke(
+          &record_printer,
+          &PtrRecordCopyContainer::SaveWithDummyArg));
+
+  // First, send unsalvagable packet to ensure we can deal with it.
+  SimulatePacketRecieve(kCorruptedPacketUnsalvagable,
+             sizeof(kCorruptedPacketUnsalvagable));
+
+  // Regression test: send a packet where the question cannot be read.
+  SimulatePacketRecieve(kCorruptedPacketBadQuestion,
+             sizeof(kCorruptedPacketBadQuestion));
+
+  // Then send salvagable packet to ensure we can extract useful records.
+  SimulatePacketRecieve(kCorruptedPacketSalvagable,
+             sizeof(kCorruptedPacketSalvagable));
+
+  RunUntilIdle();
+
+  EXPECT_TRUE(record_printer.IsRecordWith("_printer._tcp.local",
+                                          "hello._printer._tcp.local"));
+}
+
+TEST_F(MDnsTest, TransactionNoCache) {

[szym, 2013/06/12 21:35:41]

I suggest TransactionWithEmptyCache

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+  ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
+
+  scoped_ptr<MDnsTransaction> transaction_privet =
+      test_client_->CreateTransaction(
+          dns_protocol::kTypePTR, "_privet._tcp.local",
+          kMDnsTransactionQueryNetwork |
+          kMDnsTransactionQueryCache |
+          kMDnsTransactionSingleResult,
+          base::Bind(&MDnsTest::MockableRecordCallback,
+                     base::Unretained(this)));
+
+  transaction_privet->Start();
+
+  EXPECT_TRUE(test_client_->IsListeningForTests());
+
+  PtrRecordCopyContainer record_privet;
+
+  EXPECT_CALL(*this, MockableRecordCallback(kMDnsTransactionRecord, _))
+      .Times(Exactly(1))
+      .WillOnce(Invoke(&record_privet,
+                       &PtrRecordCopyContainer::SaveWithDummyArg));
+
+  SimulatePacketRecieve(kSamplePacket1, sizeof(kSamplePacket1));
+
+  RunUntilIdle();
+
+  EXPECT_TRUE(record_privet.IsRecordWith("_privet._tcp.local",
+                                         "hello._privet._tcp.local"));
+
+  EXPECT_FALSE(test_client_->IsListeningForTests());
+}
+
+TEST_F(MDnsTest, TransactionCacheOnlyNoResult) {
+  scoped_ptr<MDnsTransaction> transaction_privet =
+      test_client_->CreateTransaction(
+          dns_protocol::kTypePTR, "_privet._tcp.local",
+          kMDnsTransactionQueryCache |
+          kMDnsTransactionSingleResult,
+          base::Bind(&MDnsTest::MockableRecordCallback,
+                     base::Unretained(this)));
+
+  EXPECT_CALL(*this, MockableRecordCallback(kMDnsTransactionNoResults, _))
+      .Times(Exactly(1));
+
+  transaction_privet->Start();
+
+  EXPECT_FALSE(test_client_->IsListeningForTests());
+
+  RunUntilIdle();
+}
+
+TEST_F(MDnsTest, TransactionWithCache) {
+  // Listener to force the client to listen
+  StrictMock<MockListenerDelegate> delegate_irrelevant;
+  scoped_ptr<MDnsListener> listener_irrelevant = test_client_->CreateListener(
+      dns_protocol::kTypeA, "codereview.chromium.local",
+      &delegate_irrelevant);
+
+  listener_irrelevant->Start();
+
+  EXPECT_TRUE(test_client_->IsListeningForTests());
+
+  SimulatePacketRecieve(kSamplePacket1, sizeof(kSamplePacket1));
+
+  RunUntilIdle();
+
+  PtrRecordCopyContainer record_privet;
+
+  EXPECT_CALL(*this, MockableRecordCallback(kMDnsTransactionRecord, _))
+      .WillOnce(Invoke(&record_privet,
+                       &PtrRecordCopyContainer::SaveWithDummyArg));
+
+  scoped_ptr<MDnsTransaction> transaction_privet =
+      test_client_->CreateTransaction(
+          dns_protocol::kTypePTR, "_privet._tcp.local",
+          kMDnsTransactionQueryNetwork |
+          kMDnsTransactionQueryCache |
+          kMDnsTransactionSingleResult,
+          base::Bind(&MDnsTest::MockableRecordCallback,
+                     base::Unretained(this)));
+
+  transaction_privet->Start();
+
+  RunUntilIdle();
+
+  EXPECT_TRUE(record_privet.IsRecordWith("_privet._tcp.local",
+                                         "hello._privet._tcp.local"));
+}
+
+TEST_F(MDnsTest, AdditionalRecords) {
+  StrictMock<MockListenerDelegate> delegate_privet;
+
+  PtrRecordCopyContainer record_privet;
+
+  scoped_ptr<MDnsListener> listener_privet = test_client_->CreateListener(
+      dns_protocol::kTypePTR, "_privet._tcp.local",
+      &delegate_privet);
+
+  listener_privet->Start();
+
+  ASSERT_TRUE(test_client_->IsListeningForTests());
+
+  EXPECT_CALL(delegate_privet, OnRecordUpdate(kMDnsRecordAdded, _))
+      .Times(Exactly(1))
+      .WillOnce(Invoke(
+          &record_privet,
+          &PtrRecordCopyContainer::SaveWithDummyArg));
+
+  SimulatePacketRecieve(kSamplePacketAdditionalOnly, sizeof(kSamplePacket1));
+
+  RunUntilIdle();
+
+  EXPECT_TRUE(record_privet.IsRecordWith("_privet._tcp.local",
+                                         "hello._privet._tcp.local"));
+}
+
+TEST_F(MDnsTest, TransactionTimeout) {
+  ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
+
+  scoped_ptr<MDnsTransaction> transaction_privet =
+      test_client_->CreateTransaction(
+          dns_protocol::kTypePTR, "_privet._tcp.local",
+          kMDnsTransactionQueryNetwork |
+          kMDnsTransactionQueryCache |
+          kMDnsTransactionSingleResult,
+          base::Bind(&MDnsTest::MockableRecordCallback,
+                     base::Unretained(this)));
+
+  transaction_privet->Start();
+
+  EXPECT_TRUE(test_client_->IsListeningForTests());
+
+  EXPECT_CALL(*this, MockableRecordCallback(kMDnsTransactionNoResults, NULL))
+      .Times(Exactly(1))
+      .WillOnce(InvokeWithoutArgs(this, &MDnsTest::Stop));
+
+  RunFor(base::TimeDelta::FromSeconds(4));
+
+  EXPECT_FALSE(test_client_->IsListeningForTests());
+}
+
+TEST_F(MDnsTest, TransactionMultipleRecords) {
+  ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
+
+  scoped_ptr<MDnsTransaction> transaction_privet =
+      test_client_->CreateTransaction(
+          dns_protocol::kTypePTR, "_privet._tcp.local",
+          kMDnsTransactionQueryNetwork |
+          kMDnsTransactionQueryCache ,
+          base::Bind(&MDnsTest::MockableRecordCallback,
+                     base::Unretained(this)));
+
+  transaction_privet->Start();
+
+  EXPECT_TRUE(test_client_->IsListeningForTests());
+
+  PtrRecordCopyContainer record_privet;
+  PtrRecordCopyContainer record_privet2;
+
+  EXPECT_CALL(*this, MockableRecordCallback(kMDnsTransactionRecord, _))
+      .Times(Exactly(2))
+      .WillOnce(Invoke(&record_privet,
+                       &PtrRecordCopyContainer::SaveWithDummyArg))
+      .WillOnce(Invoke(&record_privet2,
+                       &PtrRecordCopyContainer::SaveWithDummyArg));
+
+  SimulatePacketRecieve(kSamplePacket1, sizeof(kSamplePacket1));
+  SimulatePacketRecieve(kSamplePacket2, sizeof(kSamplePacket2));
+
+  RunUntilIdle();
+
+  EXPECT_TRUE(record_privet.IsRecordWith("_privet._tcp.local",
+                                         "hello._privet._tcp.local"));
+
+  EXPECT_TRUE(record_privet2.IsRecordWith("_privet._tcp.local",
+                                          "zzzzz._privet._tcp.local"));
+
+  EXPECT_CALL(*this, MockableRecordCallback(kMDnsTransactionDone, NULL))
+      .WillOnce(InvokeWithoutArgs(this, &MDnsTest::Stop));
+
+  RunFor(base::TimeDelta::FromSeconds(4));
+
+  EXPECT_FALSE(test_client_->IsListeningForTests());
+}
+
+TEST_F(MDnsTest, TransactionReentrantDelete) {
+  ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
+
+  transaction_ = test_client_->CreateTransaction(
+      dns_protocol::kTypePTR, "_privet._tcp.local",
+      kMDnsTransactionQueryNetwork |
+      kMDnsTransactionQueryCache |
+      kMDnsTransactionSingleResult,
+      base::Bind(&MDnsTest::MockableRecordCallback,
+                 base::Unretained(this)));
+
+  transaction_->Start();
+
+  EXPECT_TRUE(test_client_->IsListeningForTests());
+
+  EXPECT_CALL(*this, MockableRecordCallback(kMDnsTransactionNoResults, NULL))
+      .Times(Exactly(1))
+      .WillOnce(DoAll(InvokeWithoutArgs(this, &MDnsTest::DeleteTransaction),
+                      InvokeWithoutArgs(this, &MDnsTest::Stop)));
+
+  RunFor(base::TimeDelta::FromSeconds(4));
+
+  EXPECT_EQ(NULL, transaction_.get());
+
+  EXPECT_FALSE(test_client_->IsListeningForTests());
+}
+
+TEST_F(MDnsTest, TransactionReentrantDeleteFromCache) {
+  StrictMock<MockListenerDelegate> delegate_irrelevant;
+  scoped_ptr<MDnsListener> listener_irrelevant = test_client_->CreateListener(
+      dns_protocol::kTypeA, "codereview.chromium.local",
+      &delegate_irrelevant);
+  listener_irrelevant->Start();
+
+  ASSERT_TRUE(test_client_->IsListeningForTests());
+
+  SimulatePacketRecieve(kSamplePacket1, sizeof(kSamplePacket1));
+
+  transaction_ = test_client_->CreateTransaction(
+      dns_protocol::kTypePTR, "_privet._tcp.local",
+      kMDnsTransactionQueryNetwork |
+      kMDnsTransactionQueryCache,
+      base::Bind(&MDnsTest::MockableRecordCallback,
+                 base::Unretained(this)));
+
+  EXPECT_CALL(*this, MockableRecordCallback(kMDnsTransactionRecord, _))
+      .Times(Exactly(1))
+      .WillOnce(InvokeWithoutArgs(this, &MDnsTest::DeleteTransaction));
+
+  transaction_->Start();
+
+  RunUntilIdle();
+
+  EXPECT_EQ(NULL, transaction_.get());
+}
+
+// In order to reliably test reentrant listener deletes, we create two listeners
+// and have each of them delete both, so we're guaranteed to try and deliver a
+// callback to at least one deleted listener.
+
+TEST_F(MDnsTest, ListenerReentrantDelete) {
+  StrictMock<MockListenerDelegate> delegate_privet;
+
+  listener1_ = test_client_->CreateListener(
+      dns_protocol::kTypePTR, "_privet._tcp.local",
+      &delegate_privet);
+
+  listener2_ = test_client_->CreateListener(
+      dns_protocol::kTypePTR, "_privet._tcp.local",
+      &delegate_privet);
+
+  listener1_->Start();
+
+  listener2_->Start();
+
+  EXPECT_CALL(delegate_privet, OnRecordUpdate(kMDnsRecordAdded, _))
+      .Times(Exactly(1))
+      .WillOnce(InvokeWithoutArgs(this, &MDnsTest::DeleteBothListeners));
+
+  EXPECT_TRUE(test_client_->IsListeningForTests());
+
+  SimulatePacketRecieve(kSamplePacket1, sizeof(kSamplePacket1));
+
+  RunUntilIdle();
+
+  EXPECT_EQ(NULL, listener1_.get());
+  EXPECT_EQ(NULL, listener2_.get());
+
+  EXPECT_FALSE(test_client_->IsListeningForTests());
+}
+
+// Note: These tests assume that the ipv4 socket will always be created first.
+// This is a simplifying assumption based on the way the code works now.
+
+class SimpleMockDatagramServerSocketFactory
+    : public MDnsDatagramServerSocketFactory {
+ public:
+  SimpleMockDatagramServerSocketFactory() {
+  }
+  virtual ~SimpleMockDatagramServerSocketFactory() {
+  }
+
+  virtual scoped_ptr<DatagramServerSocket> CreateSocket() OVERRIDE {
+    scoped_ptr<MockDatagramServerSocket> socket(
+        new StrictMock<MockDatagramServerSocket>);
+    sockets_.push(socket.get());
+    return socket.PassAs<DatagramServerSocket>();
+  }
+
+  MockDatagramServerSocket* PopFirstSocket() {
+    MockDatagramServerSocket* socket = sockets_.front();

[szym, 2013/06/12 21:35:41]

It'd be nice if you could ensure test failure without crash in case
MDnsConnection only created one socket. (EXPECT_EQ will not abort the test, so
you will still crash.) But given how simple MDnsConnection is, I'm not too
concerned about this ever becoming an issue.

[Noam Samuel, 2013/06/13 01:08:40]

Replaced with assert. Verified no crash.

+    sockets_.pop();
+    return socket;
+  }
+
+  size_t num_sockets() {
+    return sockets_.size();
+  }
+
+ private:
+  std::queue<MockDatagramServerSocket*> sockets_;
+};
+
+class MockMDnsConnectionDelegate : public MDnsConnection::Delegate {
+ public:
+  virtual void HandlePacket(DnsResponse* response, int size) {
+    HandlePacketInternal(std::string(response->io_buffer()->data(), size));
+  }
+
+  MOCK_METHOD1(HandlePacketInternal, void(std::string packet));
+
+  MOCK_METHOD1(OnConnectionError, void(int error));
+};
+
+class MDnsConnectionTest : public ::testing::Test {
+ public:
+  MDnsConnectionTest() : connection_(&factory_, &delegate_) {
+  }
+
+ protected:
+  // Follow successful connection initialization

[szym, 2013/06/12 21:35:41]

nit: end with "."

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+  virtual void SetUp() OVERRIDE {
+    EXPECT_EQ(2u, factory_.num_sockets());
+
+    socket_ipv4_ = factory_.PopFirstSocket();
+    socket_ipv6_ = factory_.PopFirstSocket();
+  }
+
+  void InitConnection() {
+    EXPECT_CALL(*socket_ipv4_, AllowAddressReuse());
+    EXPECT_CALL(*socket_ipv6_, AllowAddressReuse());
+
+    EXPECT_CALL(*socket_ipv4_, SetMulticastLoopbackMode(false));
+    EXPECT_CALL(*socket_ipv6_, SetMulticastLoopbackMode(false));
+
+    EXPECT_CALL(*socket_ipv4_, ListenInternal("0.0.0.0:5353"))
+        .WillOnce(Return(OK));
+    EXPECT_CALL(*socket_ipv6_, ListenInternal("[::]:5353"))
+        .WillOnce(Return(OK));
+
+    EXPECT_CALL(*socket_ipv4_, JoinGroupInternal("224.0.0.251"))
+        .WillOnce(Return(OK));
+    EXPECT_CALL(*socket_ipv6_, JoinGroupInternal("ff02::fb"))
+        .WillOnce(Return(OK));
+
+    connection_.Init();

[szym, 2013/06/12 21:35:41]

I suggest you return the value or (return connection_.Init() == OK) and ASSERT
it in the test.

[Noam Samuel, 2013/06/13 01:08:40]

Done.

+  }
+
+  StrictMock<MockMDnsConnectionDelegate> delegate_;
+
+  MockDatagramServerSocket* socket_ipv4_;
+  MockDatagramServerSocket* socket_ipv6_;
+  SimpleMockDatagramServerSocketFactory factory_;
+  MDnsConnection connection_;
+  TestCompletionCallback callback_;
+};
+
+TEST_F(MDnsConnectionTest, ReceiveSynchronous) {
+  std::string sample_packet =
+      std::string(kSamplePacket1, sizeof(kSamplePacket1));
+
+  socket_ipv6_->SetResponsePacket(sample_packet);
+  EXPECT_CALL(*socket_ipv4_, RecvFrom(_, _, _, _))
+      .WillOnce(Return(ERR_IO_PENDING));
+  EXPECT_CALL(*socket_ipv6_, RecvFrom(_, _, _, _))
+      .WillOnce(
+          Invoke(socket_ipv6_, &MockDatagramServerSocket::RespondImmediately))
+      .WillOnce(Return(ERR_IO_PENDING));
+
+  EXPECT_CALL(delegate_, HandlePacketInternal(sample_packet));
+
+  InitConnection();
+}
+
+TEST_F(MDnsConnectionTest, ReceiveAsynchronous) {
+  std::string sample_packet =
+      std::string(kSamplePacket1, sizeof(kSamplePacket1));
+  socket_ipv6_->SetResponsePacket(sample_packet);
+  EXPECT_CALL(*socket_ipv4_, RecvFrom(_, _, _, _))
+      .WillOnce(Return(ERR_IO_PENDING));
+  EXPECT_CALL(*socket_ipv6_, RecvFrom(_, _, _, _))
+      .WillOnce(
+          Invoke(socket_ipv6_, &MockDatagramServerSocket::RespondDelayed))
+      .WillOnce(Return(ERR_IO_PENDING));
+
+  InitConnection();
+
+  EXPECT_CALL(delegate_, HandlePacketInternal(sample_packet));
+
+  base::MessageLoop::current()->RunUntilIdle();
+}
+
+TEST_F(MDnsConnectionTest, Send) {
+  std::string sample_packet =
+      std::string(kSamplePacket1, sizeof(kSamplePacket1));
+
+  scoped_refptr<IOBufferWithSize> buf(
+      new IOBufferWithSize(sizeof kSamplePacket1));
+  memcpy(buf->data(), kSamplePacket1, sizeof(kSamplePacket1));
+
+  EXPECT_CALL(*socket_ipv4_, RecvFrom(_, _, _, _))
+      .WillOnce(Return(ERR_IO_PENDING));
+  EXPECT_CALL(*socket_ipv6_, RecvFrom(_, _, _, _))
+      .WillOnce(Return(ERR_IO_PENDING));
+
+  InitConnection();
+
+  EXPECT_CALL(*socket_ipv4_,
+              SendToInternal(sample_packet, "224.0.0.251:5353", _));
+  EXPECT_CALL(*socket_ipv6_,
+              SendToInternal(sample_packet, "[ff02::fb]:5353", _));
+
+  connection_.Send(buf, buf->size());
+}
+
+TEST_F(MDnsConnectionTest, Error) {
+  CompletionCallback callback;
+
+  EXPECT_CALL(*socket_ipv4_, RecvFrom(_, _, _, _))
+      .WillOnce(Return(ERR_IO_PENDING));
+  EXPECT_CALL(*socket_ipv6_, RecvFrom(_, _, _, _))
+      .WillOnce(DoAll(SaveArg<3>(&callback),  Return(ERR_IO_PENDING)));
+
+  InitConnection();
+
+  EXPECT_CALL(delegate_, OnConnectionError(ERR_SOCKET_NOT_CONNECTED));
+  callback.Run(ERR_SOCKET_NOT_CONNECTED);
+}
+
+}  // namespace
+
+}  // namespace net