Moving sigslot out of content::P2PAsyncAddressResolver as it's a

RefCountedThreadSafe class.

Sigslot objects must be destroyed from the same thread, otherwise
clients must call disconnect before exiting. The addition new
wrapper around P2PAsyncAddressResolver will take care of sigslot
object is created/deleted on same thread.

BUG=339392
R=sergeyu@chromium.org

Review URL: https://codereview.chromium.org/171233010

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

3 files changed, 110 insertions, 49 deletions

diff --git a/content/renderer/p2p/host_address_request.cc b/content/renderer/p2p/host_address_request.cc
index 2d1a92a9..4b5467e 100644
--- a/content/renderer/p2p/host_address_request.cc
+++ b/content/renderer/p2p/host_address_request.cc
@@ -5,6 +5,7 @@
 #include "content/renderer/p2p/host_address_request.h"
 
 #include "base/bind.h"
+#include "base/callback_helpers.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "content/common/p2p_messages.h"
 #include "content/renderer/p2p/socket_dispatcher.h"
@@ -28,39 +29,18 @@ P2PAsyncAddressResolver::~P2PAsyncAddressResolver() {
   DCHECK(!registered_);
 }
 
-void P2PAsyncAddressResolver::Start(const talk_base::SocketAddress& host_name) {
+void P2PAsyncAddressResolver::Start(const talk_base::SocketAddress& host_name,
+                                    const DoneCallback& done_callback) {
   DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(STATE_CREATED, state_);
 
   state_ = STATE_SENT;
   addr_ = host_name;
   ipc_message_loop_->PostTask(FROM_HERE, base::Bind(
-      &P2PAsyncAddressResolver::DoSendRequest, this, host_name));
+      &P2PAsyncAddressResolver::DoSendRequest, this, host_name, done_callback));
 }
 
-bool P2PAsyncAddressResolver::GetResolvedAddress(
-  int family, talk_base::SocketAddress* addr) const {
-  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
-  DCHECK_EQ(STATE_FINISHED, state_);
-
-  if (addresses_.empty())
-    return false;
-
-  *addr = addr_;
-  for (size_t i = 0; i < addresses_.size(); ++i) {
-    if (family == addresses_[i].family()) {
-      addr->SetIP(addresses_[i]);
-      return true;
-    }
-  }
-  return false;
-}
-
-int P2PAsyncAddressResolver::GetError() const {
-  return addresses_.empty() ? -1 : 0;
-}
-
-void P2PAsyncAddressResolver::Destroy(bool wait) {
+void P2PAsyncAddressResolver::Cancel() {
   DCHECK(delegate_message_loop_->BelongsToCurrentThread());
 
   if (state_ != STATE_FINISHED) {
@@ -68,13 +48,15 @@ void P2PAsyncAddressResolver::Destroy(bool wait) {
     ipc_message_loop_->PostTask(FROM_HERE, base::Bind(
         &P2PAsyncAddressResolver::DoUnregister, this));
   }
-  Release();
+  done_callback_.Reset();
 }
 
 void P2PAsyncAddressResolver::DoSendRequest(
-    const talk_base::SocketAddress& host_name) {
+    const talk_base::SocketAddress& host_name,
+    const DoneCallback& done_callback) {
   DCHECK(ipc_message_loop_->BelongsToCurrentThread());
 
+  done_callback_ = done_callback;
   request_id_ = dispatcher_->RegisterHostAddressRequest(this);
   registered_ = true;
   dispatcher_->SendP2PMessage(
@@ -104,16 +86,8 @@ void P2PAsyncAddressResolver::DeliverResponse(
     const net::IPAddressList& addresses) {
   DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (state_ == STATE_SENT) {
-    for (size_t i = 0; i < addresses.size(); ++i) {
-      talk_base::SocketAddress socket_address;
-      if (!jingle_glue::IPEndPointToSocketAddress(
-              net::IPEndPoint(addresses[i], 0), &socket_address)) {
-        NOTREACHED();
-      }
-      addresses_.push_back(socket_address.ipaddr());
-    }
     state_ = STATE_FINISHED;
-    SignalDone(this);
+    base::ResetAndReturn(&done_callback_).Run(addresses);
   }
 }
 
diff --git a/content/renderer/p2p/host_address_request.h b/content/renderer/p2p/host_address_request.h
index f0157ba..ea124ef 100644
--- a/content/renderer/p2p/host_address_request.h
+++ b/content/renderer/p2p/host_address_request.h
@@ -14,6 +14,7 @@
 #include "third_party/libjingle/source/talk/base/asyncresolverinterface.h"
 
 namespace base {
+class MessageLoop;
 class MessageLoopProxy;
 }  // namespace base
 
@@ -24,20 +25,16 @@ class P2PSocketDispatcher;
 // P2PAsyncAddressResolver performs DNS hostname resolution. It's used
 // to resolve addresses of STUN and relay servers.
 class P2PAsyncAddressResolver
-    : public base::RefCountedThreadSafe<P2PAsyncAddressResolver>,
-      public talk_base::AsyncResolverInterface {
+    : public base::RefCountedThreadSafe<P2PAsyncAddressResolver> {
  public:
-  P2PAsyncAddressResolver(P2PSocketDispatcher* dispatcher);
+  typedef base::Callback<void(const net::IPAddressList&)> DoneCallback;
 
+  P2PAsyncAddressResolver(P2PSocketDispatcher* dispatcher);
   // Start address resolve process.
-  virtual void Start(const talk_base::SocketAddress& addr) OVERRIDE;
-  // Returns top most resolved address of |family|
-  virtual bool GetResolvedAddress(
-      int family, talk_base::SocketAddress* addr) const OVERRIDE;
-  // Returns error from resolver.
-  virtual int GetError() const OVERRIDE;
-  // Delete the resolver.
-  virtual void Destroy(bool wait) OVERRIDE;
+  void Start(const talk_base::SocketAddress& addr,
+             const DoneCallback& done_callback);
+  // Clients must unregister before exiting for cleanup.
+  void Cancel();
 
  private:
   enum State {
@@ -52,7 +49,8 @@ class P2PAsyncAddressResolver
 
   virtual ~P2PAsyncAddressResolver();
 
-  void DoSendRequest(const talk_base::SocketAddress& host_name);
+  void DoSendRequest(const talk_base::SocketAddress& host_name,
+                     const DoneCallback& done_callback);
   void DoUnregister();
   void OnResponse(const net::IPAddressList& address);
   void DeliverResponse(const net::IPAddressList& address);
@@ -69,6 +67,7 @@ class P2PAsyncAddressResolver
   bool registered_;
   talk_base::SocketAddress addr_;
   std::vector<talk_base::IPAddress> addresses_;
+  DoneCallback done_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(P2PAsyncAddressResolver);
 };
diff --git a/content/renderer/p2p/ipc_socket_factory.cc b/content/renderer/p2p/ipc_socket_factory.cc
index cb4f436..784a491 100644
--- a/content/renderer/p2p/ipc_socket_factory.cc
+++ b/content/renderer/p2p/ipc_socket_factory.cc
@@ -11,6 +11,7 @@
 #include "base/debug/trace_event.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/threading/non_thread_safe.h"
 #include "content/renderer/p2p/host_address_request.h"
 #include "content/renderer/p2p/socket_client_delegate.h"
 #include "content/renderer/p2p/socket_client_impl.h"
@@ -159,6 +160,32 @@ class IpcPacketSocket : public talk_base::AsyncPacketSocket,
   DISALLOW_COPY_AND_ASSIGN(IpcPacketSocket);
 };
 
+// Simple wrapper around P2PAsyncAddressResolver. The main purpose of this
+// class is to send SignalDone, after OnDone callback from
+// P2PAsyncAddressResolver. Libjingle sig slots are not thread safe. In case
+// of MT sig slots clients must call disconnect. This class is to make sure
+// we destruct from the same thread on which is created.
+class AsyncAddressResolverImpl :  public base::NonThreadSafe,
+                                  public talk_base::AsyncResolverInterface {
+ public:
+  AsyncAddressResolverImpl(P2PSocketDispatcher* dispatcher);
+  virtual ~AsyncAddressResolverImpl();
+
+  // talk_base::AsyncResolverInterface interface.
+  virtual void Start(const talk_base::SocketAddress& addr) OVERRIDE;
+  virtual bool GetResolvedAddress(
+      int family, talk_base::SocketAddress* addr) const OVERRIDE;
+  virtual int GetError() const OVERRIDE;
+  virtual void Destroy(bool wait) OVERRIDE;
+
+ private:
+  virtual void OnAddressResolved(const net::IPAddressList& addresses);
+
+  scoped_refptr<P2PAsyncAddressResolver> resolver_;
+  talk_base::SocketAddress addr_;   // Hostname.
+  std::vector<talk_base::IPAddress> addresses_;  // Resolved addresses.
+};
+
 IpcPacketSocket::IpcPacketSocket()
     : type_(P2P_SOCKET_UDP),
       message_loop_(base::MessageLoop::current()),
@@ -465,6 +492,65 @@ void IpcPacketSocket::OnDataReceived(const net::IPEndPoint& address,
                    packet_time);
 }
 
+AsyncAddressResolverImpl::AsyncAddressResolverImpl(
+    P2PSocketDispatcher* dispatcher)
+    : resolver_(new P2PAsyncAddressResolver(dispatcher)) {
+}
+
+AsyncAddressResolverImpl::~AsyncAddressResolverImpl() {
+}
+
+void AsyncAddressResolverImpl::Start(const talk_base::SocketAddress& addr) {
+  DCHECK(CalledOnValidThread());
+  resolver_->Start(addr, base::Bind(
+      &AsyncAddressResolverImpl::OnAddressResolved,
+      base::Unretained(this)));
+}
+
+bool AsyncAddressResolverImpl::GetResolvedAddress(
+    int family, talk_base::SocketAddress* addr) const {
+  DCHECK(CalledOnValidThread());
+
+  if (addresses_.empty())
+   return false;
+
+  *addr = addr_;
+  for (size_t i = 0; i < addresses_.size(); ++i) {
+    if (family == addresses_[i].family()) {
+      addr->SetIP(addresses_[i]);
+      return true;
+    }
+  }
+  return false;
+}
+
+int AsyncAddressResolverImpl::GetError() const {
+  DCHECK(CalledOnValidThread());
+  return addresses_.empty() ? -1 : 0;
+}
+
+void AsyncAddressResolverImpl::Destroy(bool wait) {
+  DCHECK(CalledOnValidThread());
+  resolver_->Cancel();
+  // Libjingle doesn't need this object any more and it's not going to delete
+  // it explicitly.
+  delete this;
+}
+
+void AsyncAddressResolverImpl::OnAddressResolved(
+    const net::IPAddressList& addresses) {
+  DCHECK(CalledOnValidThread());
+  for (size_t i = 0; i < addresses.size(); ++i) {
+    talk_base::SocketAddress socket_address;
+    if (!jingle_glue::IPEndPointToSocketAddress(
+            net::IPEndPoint(addresses[i], 0), &socket_address)) {
+      NOTREACHED();
+    }
+    addresses_.push_back(socket_address.ipaddr());
+  }
+  SignalDone(this);
+}
+
 }  // namespace
 
 IpcPacketSocketFactory::IpcPacketSocketFactory(
@@ -535,7 +621,9 @@ talk_base::AsyncPacketSocket* IpcPacketSocketFactory::CreateClientTcpSocket(
 
 talk_base::AsyncResolverInterface*
 IpcPacketSocketFactory::CreateAsyncResolver() {
-  return new P2PAsyncAddressResolver(socket_dispatcher_);
+  scoped_ptr<AsyncAddressResolverImpl> resolver(
+    new AsyncAddressResolverImpl(socket_dispatcher_));
+  return resolver.release();
 }
 
 }  // namespace content