Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(969)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: net/dns/mdns_client_unittest.cc

Issue 937743003: Fix MDnsClient's cache entry cleanup logic. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Use base::Clock throughout MDnsClientImpl Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/dns/mdns_client_impl.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/dns/mdns_client_unittest.cc
diff --git a/net/dns/mdns_client_unittest.cc b/net/dns/mdns_client_unittest.cc
index ba0308201bb1558be136a002193d8befcb3fc491..eaacee3e1d55239e43232f01836cf5d3192ddb1f 100644
--- a/net/dns/mdns_client_unittest.cc
+++ b/net/dns/mdns_client_unittest.cc
@@ -6,6 +6,9 @@
#include "base/memory/ref_counted.h"
#include "base/message_loop/message_loop.h"
+#include "base/time/clock.h"
+#include "base/time/default_clock.h"
+#include "base/timer/mock_timer.h"
#include "net/base/rand_callback.h"
#include "net/base/test_completion_callback.h"
#include "net/dns/mdns_client_impl.h"
@@ -222,6 +225,34 @@ const uint8 kSamplePacket2[] = {
0xc0, 0x32
};
+const uint8 kSamplePacket3[] = {
+ // 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;
gene 2015/03/02 19:37:16 nit: can you move 0x00, 0x01 here from the line be
Kevin M 2015/03/02 20:23:46 Hmm, "git cl format" screwed this up. I'll put in
+ 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, 0x00, // TTL (4 bytes) is 3 seconds.
gene 2015/03/02 19:37:16 same here
Kevin M 2015/03/02 20:23:46 Done.
+ 0x00, 0x03, 0x00, 0x08, // RDLENGTH is 8 bytes.
+ 0x05, 'h', 'e', 'l', 'l', 'o', 0xc0, 0x32
+};
+
const uint8 kQueryPacketPrivet[] = {
// Header
0x00, 0x00, // ID is zeroed out
@@ -389,9 +420,45 @@ class PtrRecordCopyContainer {
int ttl_;
};
+class MockClock : public base::DefaultClock {
+ public:
+ MockClock() : base::DefaultClock() {}
+ virtual ~MockClock() {}
+
+ MOCK_METHOD0(Now, base::Time());
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockClock);
+};
+
+class MockTimer : public base::MockTimer {
+ public:
+ MockTimer() : base::MockTimer(false, false) {}
+ ~MockTimer() {}
+
+ void Start(const tracked_objects::Location& posted_from,
+ base::TimeDelta delay,
+ const base::Closure& user_task) {
+ StartObserver(posted_from, delay, user_task);
+ base::MockTimer::Start(posted_from, delay, user_task);
+ }
+
+ // StartObserver is invoked when MockTimer::Start() is called.
+ // Does not replace the behavior of MockTimer::Start().
+ MOCK_METHOD3(StartObserver,
+ void(const tracked_objects::Location& posted_from,
+ base::TimeDelta delay,
+ const base::Closure& user_task));
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockTimer);
+};
+
+} // namespace
+
class MDnsTest : public ::testing::Test {
public:
- virtual void SetUp() override;
+ void SetUp() override;
void DeleteTransaction();
void DeleteBothListeners();
void RunFor(base::TimeDelta time_period);
@@ -403,12 +470,11 @@ class MDnsTest : public ::testing::Test {
MOCK_METHOD2(MockableRecordCallback2, void(MDnsTransaction::Result result,
const RecordParsed* record));
-
protected:
void ExpectPacket(const uint8* packet, unsigned size);
void SimulatePacketReceive(const uint8* packet, unsigned size);
- MDnsClientImpl test_client_;
+ scoped_ptr<MDnsClientImpl> test_client_;
IPEndPoint mdns_ipv4_endpoint_;
StrictMock<MockMDnsSocketFactory> socket_factory_;
@@ -429,7 +495,8 @@ class MockListenerDelegate : public MDnsListener::Delegate {
};
void MDnsTest::SetUp() {
- test_client_.StartListening(&socket_factory_);
+ test_client_.reset(new MDnsClientImpl());
+ test_client_->StartListening(&socket_factory_);
}
void MDnsTest::SimulatePacketReceive(const uint8* packet, unsigned size) {
@@ -471,12 +538,10 @@ TEST_F(MDnsTest, PassiveListeners) {
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_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);
ASSERT_TRUE(listener_privet->Start());
ASSERT_TRUE(listener_printer->Start());
@@ -516,7 +581,7 @@ TEST_F(MDnsTest, PassiveListenersCacheCleanup) {
PtrRecordCopyContainer record_privet2;
scoped_ptr<MDnsListener> listener_privet =
- test_client_.CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
+ test_client_->CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
&delegate_privet);
ASSERT_TRUE(listener_privet->Start());
@@ -545,14 +610,75 @@ TEST_F(MDnsTest, PassiveListenersCacheCleanup) {
"hello._privet._tcp.local"));
}
+// Ensure that the cleanup task scheduler won't schedule cleanup tasks in the
+// past if the system clock creeps past the expiration time while in the
+// cleanup dispatcher.
+TEST_F(MDnsTest, CacheCleanupWithShortTTL) {
+ // Use a nonzero starting time as a base.
+ base::Time start_time = base::Time() + base::TimeDelta::FromSeconds(1);
+
+ MockClock* clock = new MockClock;
+ MockTimer* timer = new MockTimer;
+
+ test_client_.reset(
+ new MDnsClientImpl(make_scoped_ptr(clock), make_scoped_ptr(timer)));
+ test_client_->StartListening(&socket_factory_);
+
+ EXPECT_CALL(*timer, StartObserver(_, _, _)).Times(1);
+ EXPECT_CALL(*clock, Now())
+ .Times(3)
+ .WillRepeatedly(Return(start_time))
+ .RetiresOnSaturation();
+
+ // Receive two records with different TTL values.
+ // TTL(privet)=1.0s
+ // TTL(printer)=3.0s
+ StrictMock<MockListenerDelegate> delegate_privet;
+ StrictMock<MockListenerDelegate> delegate_printer;
+
+ 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);
+
+ ASSERT_TRUE(listener_privet->Start());
+ ASSERT_TRUE(listener_printer->Start());
+
+ EXPECT_CALL(delegate_privet, OnRecordUpdate(MDnsListener::RECORD_ADDED, _))
+ .Times(Exactly(1));
+ EXPECT_CALL(delegate_printer, OnRecordUpdate(MDnsListener::RECORD_ADDED, _))
+ .Times(Exactly(1));
+
+ SimulatePacketReceive(kSamplePacket3, sizeof(kSamplePacket3));
+
+ EXPECT_CALL(delegate_privet, OnRecordUpdate(MDnsListener::RECORD_REMOVED, _))
+ .Times(Exactly(1));
+
+ // Set the clock to 2.0s, which should clean up the 'privet' record, but not
+ // the printer. The mock clock will change Now() mid-execution from 2s to 4s.
+ // Note: expectations are FILO-ordered -- t+2 seconds is returned, then t+4.
+ EXPECT_CALL(*clock, Now())
+ .WillOnce(Return(start_time + base::TimeDelta::FromSeconds(4)))
+ .RetiresOnSaturation();
+ EXPECT_CALL(*clock, Now())
+ .WillOnce(Return(start_time + base::TimeDelta::FromSeconds(2)))
+ .RetiresOnSaturation();
+
+ EXPECT_CALL(*timer, StartObserver(_, base::TimeDelta(), _));
+
+ timer->Fire();
+}
+
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);
+ scoped_ptr<MDnsListener> listener_printer = test_client_->CreateListener(
+ dns_protocol::kTypePTR, "_printer._tcp.local", &delegate_printer);
ASSERT_TRUE(listener_printer->Start());
@@ -582,7 +708,7 @@ TEST_F(MDnsTest, TransactionWithEmptyCache) {
ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
scoped_ptr<MDnsTransaction> transaction_privet =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE |
@@ -607,7 +733,7 @@ TEST_F(MDnsTest, TransactionWithEmptyCache) {
TEST_F(MDnsTest, TransactionCacheOnlyNoResult) {
scoped_ptr<MDnsTransaction> transaction_privet =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_CACHE |
MDnsTransaction::SINGLE_RESULT,
@@ -625,7 +751,7 @@ 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,
+ test_client_->CreateListener(dns_protocol::kTypeA,
"codereview.chromium.local",
&delegate_irrelevant);
@@ -641,7 +767,7 @@ TEST_F(MDnsTest, TransactionWithCache) {
&PtrRecordCopyContainer::SaveWithDummyArg));
scoped_ptr<MDnsTransaction> transaction_privet =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE |
@@ -661,7 +787,7 @@ TEST_F(MDnsTest, AdditionalRecords) {
PtrRecordCopyContainer record_privet;
scoped_ptr<MDnsListener> listener_privet =
- test_client_.CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
+ test_client_->CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
&delegate_privet);
ASSERT_TRUE(listener_privet->Start());
@@ -683,7 +809,7 @@ TEST_F(MDnsTest, TransactionTimeout) {
ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
scoped_ptr<MDnsTransaction> transaction_privet =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE |
@@ -705,7 +831,7 @@ TEST_F(MDnsTest, TransactionMultipleRecords) {
ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
scoped_ptr<MDnsTransaction> transaction_privet =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE ,
@@ -742,7 +868,7 @@ TEST_F(MDnsTest, TransactionMultipleRecords) {
TEST_F(MDnsTest, TransactionReentrantDelete) {
ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
- transaction_ = test_client_.CreateTransaction(
+ transaction_ = test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE |
@@ -765,14 +891,14 @@ TEST_F(MDnsTest, TransactionReentrantDelete) {
TEST_F(MDnsTest, TransactionReentrantDeleteFromCache) {
StrictMock<MockListenerDelegate> delegate_irrelevant;
- scoped_ptr<MDnsListener> listener_irrelevant = test_client_.CreateListener(
+ scoped_ptr<MDnsListener> listener_irrelevant = test_client_->CreateListener(
dns_protocol::kTypeA, "codereview.chromium.local",
&delegate_irrelevant);
ASSERT_TRUE(listener_irrelevant->Start());
SimulatePacketReceive(kSamplePacket1, sizeof(kSamplePacket1));
- transaction_ = test_client_.CreateTransaction(
+ transaction_ = test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE,
@@ -792,7 +918,7 @@ TEST_F(MDnsTest, TransactionReentrantCacheLookupStart) {
ExpectPacket(kQueryPacketPrivet, sizeof(kQueryPacketPrivet));
scoped_ptr<MDnsTransaction> transaction1 =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE |
@@ -801,7 +927,7 @@ TEST_F(MDnsTest, TransactionReentrantCacheLookupStart) {
base::Unretained(this)));
scoped_ptr<MDnsTransaction> transaction2 =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_printer._tcp.local",
MDnsTransaction::QUERY_CACHE |
MDnsTransaction::SINGLE_RESULT,
@@ -826,7 +952,7 @@ TEST_F(MDnsTest, TransactionReentrantCacheLookupStart) {
TEST_F(MDnsTest, GoodbyePacketNotification) {
StrictMock<MockListenerDelegate> delegate_privet;
- scoped_ptr<MDnsListener> listener_privet = test_client_.CreateListener(
+ scoped_ptr<MDnsListener> listener_privet = test_client_->CreateListener(
dns_protocol::kTypePTR, "_privet._tcp.local", &delegate_privet);
ASSERT_TRUE(listener_privet->Start());
@@ -839,7 +965,7 @@ TEST_F(MDnsTest, GoodbyePacketRemoval) {
StrictMock<MockListenerDelegate> delegate_privet;
scoped_ptr<MDnsListener> listener_privet =
- test_client_.CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
+ test_client_->CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
&delegate_privet);
ASSERT_TRUE(listener_privet->Start());
@@ -863,11 +989,11 @@ TEST_F(MDnsTest, GoodbyePacketRemoval) {
TEST_F(MDnsTest, ListenerReentrantDelete) {
StrictMock<MockListenerDelegate> delegate_privet;
- listener1_ = test_client_.CreateListener(dns_protocol::kTypePTR,
+ listener1_ = test_client_->CreateListener(dns_protocol::kTypePTR,
"_privet._tcp.local",
&delegate_privet);
- listener2_ = test_client_.CreateListener(dns_protocol::kTypePTR,
+ listener2_ = test_client_->CreateListener(dns_protocol::kTypePTR,
"_privet._tcp.local",
&delegate_privet);
@@ -897,7 +1023,7 @@ TEST_F(MDnsTest, DoubleRecordDisagreeing) {
StrictMock<MockListenerDelegate> delegate_privet;
scoped_ptr<MDnsListener> listener_privet =
- test_client_.CreateListener(dns_protocol::kTypeA, "privet.local",
+ test_client_->CreateListener(dns_protocol::kTypeA, "privet.local",
&delegate_privet);
ASSERT_TRUE(listener_privet->Start());
@@ -915,14 +1041,14 @@ TEST_F(MDnsTest, DoubleRecordDisagreeing) {
TEST_F(MDnsTest, NsecWithListener) {
StrictMock<MockListenerDelegate> delegate_privet;
scoped_ptr<MDnsListener> listener_privet =
- test_client_.CreateListener(dns_protocol::kTypeA, "_privet._tcp.local",
+ test_client_->CreateListener(dns_protocol::kTypeA, "_privet._tcp.local",
&delegate_privet);
// Test to make sure nsec callback is NOT called for PTR
// (which is marked as existing).
StrictMock<MockListenerDelegate> delegate_privet2;
scoped_ptr<MDnsListener> listener_privet2 =
- test_client_.CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
+ test_client_->CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
&delegate_privet2);
ASSERT_TRUE(listener_privet->Start());
@@ -936,7 +1062,7 @@ TEST_F(MDnsTest, NsecWithListener) {
TEST_F(MDnsTest, NsecWithTransactionFromNetwork) {
scoped_ptr<MDnsTransaction> transaction_privet =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypeA, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE |
@@ -959,7 +1085,7 @@ TEST_F(MDnsTest, NsecWithTransactionFromCache) {
// Force mDNS to listen.
StrictMock<MockListenerDelegate> delegate_irrelevant;
scoped_ptr<MDnsListener> listener_irrelevant =
- test_client_.CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
+ test_client_->CreateListener(dns_protocol::kTypePTR, "_privet._tcp.local",
&delegate_irrelevant);
listener_irrelevant->Start();
@@ -970,7 +1096,7 @@ TEST_F(MDnsTest, NsecWithTransactionFromCache) {
MockableRecordCallback(MDnsTransaction::RESULT_NSEC, NULL));
scoped_ptr<MDnsTransaction> transaction_privet_a =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypeA, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE |
@@ -984,7 +1110,7 @@ TEST_F(MDnsTest, NsecWithTransactionFromCache) {
// valid answer to the query
scoped_ptr<MDnsTransaction> transaction_privet_ptr =
- test_client_.CreateTransaction(
+ test_client_->CreateTransaction(
dns_protocol::kTypePTR, "_privet._tcp.local",
MDnsTransaction::QUERY_NETWORK |
MDnsTransaction::QUERY_CACHE |
@@ -1000,7 +1126,7 @@ TEST_F(MDnsTest, NsecWithTransactionFromCache) {
TEST_F(MDnsTest, NsecConflictRemoval) {
StrictMock<MockListenerDelegate> delegate_privet;
scoped_ptr<MDnsListener> listener_privet =
- test_client_.CreateListener(dns_protocol::kTypeA, "_privet._tcp.local",
+ test_client_->CreateListener(dns_protocol::kTypeA, "_privet._tcp.local",
&delegate_privet);
ASSERT_TRUE(listener_privet->Start());
@@ -1030,7 +1156,7 @@ TEST_F(MDnsTest, NsecConflictRemoval) {
TEST_F(MDnsTest, RefreshQuery) {
StrictMock<MockListenerDelegate> delegate_privet;
scoped_ptr<MDnsListener> listener_privet =
- test_client_.CreateListener(dns_protocol::kTypeA, "_privet._tcp.local",
+ test_client_->CreateListener(dns_protocol::kTypeA, "_privet._tcp.local",
&delegate_privet);
listener_privet->SetActiveRefresh(true);
@@ -1087,7 +1213,7 @@ class MDnsConnectionTest : public ::testing::Test {
protected:
// Follow successful connection initialization.
- virtual void SetUp() override {
+ void SetUp() override {
socket_ipv4_ = new MockMDnsDatagramServerSocket(ADDRESS_FAMILY_IPV4);
socket_ipv6_ = new MockMDnsDatagramServerSocket(ADDRESS_FAMILY_IPV6);
factory_.PushSocket(socket_ipv6_);
@@ -1219,6 +1345,4 @@ TEST_F(MDnsConnectionSendTest, SendQueued) {
callback.Run(OK);
}
-} // namespace
-
} // namespace net
« no previous file with comments | « net/dns/mdns_client_impl.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698