Add an explicit way of forcing the MDnsClient to listen on the network

Add an object called MDnsListenRef, whose existence forces the MDnsClient to
continue listening on the network, but who has no other function.

Rationale: Since a sandboxed process using an MDnsClient may not be able
to create new sockets on demand, it makes sense to force the MDnsClient to
be awake for the lifetime of the process. Add the ability to force the
MDnsClient to stay awake.

Also useful for abstractions over MDnsClient where you know that it makes sense
not to immediately shut down the client even if we shut down all listeners and
transactions.

BUG=233821

Review URL: https://chromiumcodereview.appspot.com/17922002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209812 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 25 insertions, 71 deletions

diff --git a/net/dns/mdns_client.h b/net/dns/mdns_client.h
index 9e3bdb7..21022c8 100644
--- a/net/dns/mdns_client.h
+++ b/net/dns/mdns_client.h
@@ -143,6 +143,13 @@ class NET_EXPORT MDnsClient {
       int flags,
       const MDnsTransaction::ResultCallback& callback) = 0;
 
+  virtual bool StartListening() = 0;
+
+  // Do not call this inside callbacks from related MDnsListener and
+  // MDnsTransaction objects.
+  virtual void StopListening() = 0;
+  virtual bool IsListening() const = 0;
+
   // Lazily create and return static instance for MDnsClient.
   static MDnsClient* GetInstance();
 
diff --git a/net/dns/mdns_client_impl.cc b/net/dns/mdns_client_impl.cc
index 7388e75..089a51a 100644
--- a/net/dns/mdns_client_impl.cc
+++ b/net/dns/mdns_client_impl.cc
@@ -434,40 +434,27 @@ void MDnsClientImpl::Core::QueryCache(
 
 MDnsClientImpl::MDnsClientImpl(
     scoped_ptr<MDnsConnection::SocketFactory> socket_factory)
-    : listen_refs_(0), socket_factory_(socket_factory.Pass()) {
+    : socket_factory_(socket_factory.Pass()) {
 }
 
 MDnsClientImpl::~MDnsClientImpl() {
 }
 
-bool MDnsClientImpl::AddListenRef() {
-  if (!core_.get()) {
-    core_.reset(new Core(this, socket_factory_.get()));
-    if (!core_->Init()) {
-      core_.reset();
-      return false;
-    }
+bool MDnsClientImpl::StartListening() {
+  DCHECK(!core_.get());
+  core_.reset(new Core(this, socket_factory_.get()));
+  if (!core_->Init()) {
+    core_.reset();
+    return false;
   }
-  listen_refs_++;
   return true;
 }
 
-void MDnsClientImpl::SubtractListenRef() {
-  listen_refs_--;
-  if (listen_refs_ == 0) {
-    base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
-        &MDnsClientImpl::Shutdown, base::Unretained(this)));
-  }
-}
-
-void MDnsClientImpl::Shutdown() {
-  // We need to check that new listeners haven't been created.
-  if (listen_refs_ == 0) {
-    core_.reset();
-  }
+void MDnsClientImpl::StopListening() {
+  core_.reset();
 }
 
-bool MDnsClientImpl::IsListeningForTests() {
+bool MDnsClientImpl::IsListening() const {
   return core_.get() != NULL;
 }
 
@@ -500,7 +487,6 @@ MDnsListenerImpl::MDnsListenerImpl(
 bool MDnsListenerImpl::Start() {
   DCHECK(!started_);
 
-  if (!client_->AddListenRef()) return false;
   started_ = true;
 
   DCHECK(client_->core());
@@ -513,7 +499,6 @@ MDnsListenerImpl::~MDnsListenerImpl() {
   if (started_) {
     DCHECK(client_->core());
     client_->core()->RemoveListener(this);
-    client_->SubtractListenRef();
   }
 }
 
diff --git a/net/dns/mdns_client_impl.h b/net/dns/mdns_client_impl.h
index 64aedf2..9fe3f99 100644
--- a/net/dns/mdns_client_impl.h
+++ b/net/dns/mdns_client_impl.h
@@ -181,24 +181,14 @@ class NET_EXPORT_PRIVATE MDnsClientImpl : public MDnsClient {
       int flags,
       const MDnsTransaction::ResultCallback& callback) OVERRIDE;
 
-  // Returns true when the client is listening for network packets.
-  bool IsListeningForTests();
-
-  bool AddListenRef();
-  void SubtractListenRef();
+  virtual bool StartListening() OVERRIDE;
+  virtual void StopListening() OVERRIDE;
+  virtual bool IsListening() const OVERRIDE;
 
   Core* core() { return core_.get(); }
 
  private:
-  // This method causes the client to stop listening for packets. The
-  // call for it is deferred through the message loop after the last
-  // listener is removed. If another listener is added after a
-  // shutdown is scheduled but before it actually runs, the shutdown
-  // will be canceled.
-  void Shutdown();
-
   scoped_ptr<Core> core_;
-  int listen_refs_;
 
   scoped_ptr<MDnsConnection::SocketFactory> socket_factory_;
 
diff --git a/net/dns/mdns_client_unittest.cc b/net/dns/mdns_client_unittest.cc
index df393ec..b19d63e 100644
--- a/net/dns/mdns_client_unittest.cc
+++ b/net/dns/mdns_client_unittest.cc
@@ -347,6 +347,7 @@ class MDnsTest : public ::testing::Test {
  public:
   MDnsTest();
   virtual ~MDnsTest();
+  virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
   void DeleteTransaction();
   void DeleteBothListeners();
@@ -393,12 +394,11 @@ MDnsTest::MDnsTest() {
 MDnsTest::~MDnsTest() {
 }
 
-void MDnsTest::TearDown() {
-  base::MessageLoop::current()->RunUntilIdle();
-
-  ASSERT_FALSE(test_client_->IsListeningForTests());
+void MDnsTest::SetUp() {
+  test_client_->StartListening();
+}
 
-  base::MessageLoop::current()->AssertIdle();
+void MDnsTest::TearDown() {
 }
 
 void MDnsTest::SimulatePacketReceive(const char* packet, unsigned size) {
@@ -448,8 +448,6 @@ TEST_F(MDnsTest, PassiveListeners) {
   ASSERT_TRUE(listener_privet->Start());
   ASSERT_TRUE(listener_printer->Start());
 
-  ASSERT_TRUE(test_client_->IsListeningForTests());
-
   // Send the same packet twice to ensure no records are double-counted.
 
   EXPECT_CALL(delegate_privet, OnRecordUpdate(MDnsListener::RECORD_ADDED, _))
@@ -476,8 +474,6 @@ TEST_F(MDnsTest, PassiveListeners) {
 
   listener_privet.reset();
   listener_printer.reset();
-
-  ASSERT_TRUE(test_client_->IsListeningForTests());
 }
 
 TEST_F(MDnsTest, PassiveListenersCacheCleanup) {
@@ -491,8 +487,6 @@ TEST_F(MDnsTest, PassiveListenersCacheCleanup) {
 
   ASSERT_TRUE(listener_privet->Start());
 
-  ASSERT_TRUE(test_client_->IsListeningForTests());
-
   EXPECT_CALL(delegate_privet, OnRecordUpdate(MDnsListener::RECORD_ADDED, _))
       .Times(Exactly(1))
       .WillOnce(Invoke(
@@ -527,8 +521,6 @@ TEST_F(MDnsTest, MalformedPacket) {
 
   ASSERT_TRUE(listener_printer->Start());
 
-  ASSERT_TRUE(test_client_->IsListeningForTests());
-
   EXPECT_CALL(delegate_printer, OnRecordUpdate(MDnsListener::RECORD_ADDED, _))
       .Times(Exactly(1))
       .WillOnce(Invoke(
@@ -565,8 +557,6 @@ TEST_F(MDnsTest, TransactionWithEmptyCache) {
 
   ASSERT_TRUE(transaction_privet->Start());
 
-  EXPECT_TRUE(test_client_->IsListeningForTests());
-
   PtrRecordCopyContainer record_privet;
 
   EXPECT_CALL(*this, MockableRecordCallback(MDnsTransaction::RESULT_RECORD, _))
@@ -594,8 +584,6 @@ TEST_F(MDnsTest, TransactionCacheOnlyNoResult) {
       .Times(Exactly(1));
 
   ASSERT_TRUE(transaction_privet->Start());
-
-  EXPECT_FALSE(test_client_->IsListeningForTests());
 }
 
 TEST_F(MDnsTest, TransactionWithCache) {
@@ -607,8 +595,6 @@ TEST_F(MDnsTest, TransactionWithCache) {
 
   ASSERT_TRUE(listener_irrelevant->Start());
 
-  EXPECT_TRUE(test_client_->IsListeningForTests());
-
   SimulatePacketReceive(kSamplePacket1, sizeof(kSamplePacket1));
 
 
@@ -644,8 +630,6 @@ TEST_F(MDnsTest, AdditionalRecords) {
 
   ASSERT_TRUE(listener_privet->Start());
 
-  ASSERT_TRUE(test_client_->IsListeningForTests());
-
   EXPECT_CALL(delegate_privet, OnRecordUpdate(MDnsListener::RECORD_ADDED, _))
       .Times(Exactly(1))
       .WillOnce(Invoke(
@@ -673,8 +657,6 @@ TEST_F(MDnsTest, TransactionTimeout) {
 
   ASSERT_TRUE(transaction_privet->Start());
 
-  EXPECT_TRUE(test_client_->IsListeningForTests());
-
   EXPECT_CALL(*this,
               MockableRecordCallback(MDnsTransaction::RESULT_NO_RESULTS, NULL))
       .Times(Exactly(1))
@@ -696,8 +678,6 @@ TEST_F(MDnsTest, TransactionMultipleRecords) {
 
   ASSERT_TRUE(transaction_privet->Start());
 
-  EXPECT_TRUE(test_client_->IsListeningForTests());
-
   PtrRecordCopyContainer record_privet;
   PtrRecordCopyContainer record_privet2;
 
@@ -736,8 +716,6 @@ TEST_F(MDnsTest, TransactionReentrantDelete) {
 
   ASSERT_TRUE(transaction_->Start());
 
-  EXPECT_TRUE(test_client_->IsListeningForTests());
-
   EXPECT_CALL(*this, MockableRecordCallback(MDnsTransaction::RESULT_NO_RESULTS,
                                             NULL))
       .Times(Exactly(1))
@@ -756,8 +734,6 @@ TEST_F(MDnsTest, TransactionReentrantDeleteFromCache) {
       &delegate_irrelevant);
   ASSERT_TRUE(listener_irrelevant->Start());
 
-  ASSERT_TRUE(test_client_->IsListeningForTests());
-
   SimulatePacketReceive(kSamplePacket1, sizeof(kSamplePacket1));
 
   transaction_ = test_client_->CreateTransaction(
@@ -806,8 +782,6 @@ TEST_F(MDnsTest, TransactionReentrantCacheLookupStart) {
 
   ASSERT_TRUE(transaction1->Start());
 
-  EXPECT_TRUE(test_client_->IsListeningForTests());
-
   SimulatePacketReceive(kSamplePacket1, sizeof(kSamplePacket1));
 }
 
@@ -834,8 +808,6 @@ TEST_F(MDnsTest, ListenerReentrantDelete) {
       .Times(Exactly(1))
       .WillOnce(InvokeWithoutArgs(this, &MDnsTest::DeleteBothListeners));
 
-  EXPECT_TRUE(test_client_->IsListeningForTests());
-
   SimulatePacketReceive(kSamplePacket1, sizeof(kSamplePacket1));
 
   EXPECT_EQ(NULL, listener1_.get());