Fix MDnsClient's cache entry cleanup logic.

net/dns/mdns_client_impl.cc

Index: net/dns/mdns_client_impl.cc
diff --git a/net/dns/mdns_client_impl.cc b/net/dns/mdns_client_impl.cc
index 66873abc36bca278a33e0194f0dd85ae52f91740..4441f445aa1e0e735473e17a34bbe19d71a7ab0b 100644
--- a/net/dns/mdns_client_impl.cc
+++ b/net/dns/mdns_client_impl.cc
@@ -4,13 +4,16 @@
 
 #include "net/dns/mdns_client_impl.h"
 
+#include <algorithm>
 #include <queue>
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/stl_util.h"
+#include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "net/base/dns_util.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
@@ -195,7 +198,10 @@ void MDnsConnection::OnDatagramReceived(
   delegate_->HandlePacket(response, bytes_read);
 }
 
-MDnsClientImpl::Core::Core() : connection_(new MDnsConnection(this)) {
+MDnsClientImpl::Core::Core()
+    : clock_(new base::DefaultClock),
+      cleanup_timer_(new base::Timer(false, false)),
+      connection_(new MDnsConnection(this)) {
 }
 
 MDnsClientImpl::Core::~Core() {
@@ -241,8 +247,8 @@ void MDnsClientImpl::Core::HandlePacket(DnsResponse* response,
 
   for (unsigned i = 0; i < answer_count; i++) {
     offset = parser.GetOffset();
-    scoped_ptr<const RecordParsed> record = RecordParsed::CreateFrom(
-        &parser, base::Time::Now());
+    scoped_ptr<const RecordParsed> record =
+        RecordParsed::CreateFrom(&parser, clock_->Now());
 
     if (!record) {
       DVLOG(1) << "Could not understand an mDNS record.";
@@ -295,8 +301,7 @@ void MDnsClientImpl::Core::NotifyNsecRecord(const RecordParsed* record) {
   // Remove all cached records matching the nonexistent RR types.
   std::vector<const RecordParsed*> records_to_remove;
 
-  cache_.FindDnsRecords(0, record->name(), &records_to_remove,
-                        base::Time::Now());
+  cache_.FindDnsRecords(0, record->name(), &records_to_remove, clock_->Now());
 
   for (std::vector<const RecordParsed*>::iterator i = records_to_remove.begin();
        i != records_to_remove.end(); i++) {
@@ -323,6 +328,15 @@ void MDnsClientImpl::Core::OnConnectionError(int error) {
   // TODO(noamsml): On connection error, recreate connection and flush cache.
 }
 
+void MDnsClientImpl::Core::set_cleanup_timer_for_test(
+    scoped_ptr<base::Timer> timer) {
+  cleanup_timer_ = timer.Pass();
+}
+
+void MDnsClientImpl::Core::set_clock_for_test(scoped_ptr<base::Clock> clock) {
+  clock_ = clock.Pass();
+}
+
 void MDnsClientImpl::Core::AlertListeners(
     MDnsCache::UpdateType update_type,
     const ListenerKey& key,
@@ -380,25 +394,26 @@ void MDnsClientImpl::Core::CleanupObserverList(const ListenerKey& key) {
 
 void MDnsClientImpl::Core::ScheduleCleanup(base::Time cleanup) {
   // Cleanup is already scheduled, no need to do anything.
-  if (cleanup == scheduled_cleanup_) return;
+  if (cleanup == scheduled_cleanup_) {
+    return;
+  }
   scheduled_cleanup_ = cleanup;
 
   // This cancels the previously scheduled cleanup.
-  cleanup_callback_.Reset(base::Bind(
-      &MDnsClientImpl::Core::DoCleanup, base::Unretained(this)));
+  cleanup_timer_->Stop();
 
   // If |cleanup| is empty, then no cleanup necessary.
   if (cleanup != base::Time()) {
-    base::MessageLoop::current()->PostDelayedTask(
-        FROM_HERE,
-        cleanup_callback_.callback(),
-        cleanup - base::Time::Now());
+    cleanup_timer_->Start(
+        FROM_HERE, std::max(base::TimeDelta(), cleanup - clock_->Now()),
+        base::Bind(&MDnsClientImpl::Core::DoCleanup, base::Unretained(this)));
   }
 }
 
 void MDnsClientImpl::Core::DoCleanup() {
-  cache_.CleanupRecords(base::Time::Now(), base::Bind(
-      &MDnsClientImpl::Core::OnRecordRemoved, base::Unretained(this)));
+  cache_.CleanupRecords(clock_->Now(),
+                        base::Bind(&MDnsClientImpl::Core::OnRecordRemoved,

[mark a. foltz, 2015/02/25 21:44:07]

What cancels this callback when MDnsClientImpl is deleted?

[Kevin M, 2015/02/26 01:04:40]

DoCleanup() can't be called if the MdnsClientImpl is deleted, because
MDnsCleanup::cleanup_timer_ will be deleted and unable to execute the DoCleanup
callback.

[mark a. foltz, 2015/02/26 01:18:03]

Acknowledged.

+                                   base::Unretained(this)));
 
   ScheduleCleanup(cache_.next_expiration());
 }
@@ -412,7 +427,7 @@ void MDnsClientImpl::Core::OnRecordRemoved(
 void MDnsClientImpl::Core::QueryCache(
     uint16 rrtype, const std::string& name,
     std::vector<const RecordParsed*>* records) const {
-  cache_.FindDnsRecords(rrtype, name, records, base::Time::Now());
+  cache_.FindDnsRecords(rrtype, name, records, clock_->Now());
 }
 
 MDnsClientImpl::MDnsClientImpl() {