Fix asan test failure in P2PSocketClient::Init. Make sure P2PSocketClient::Init only access |deleg…

content/renderer/p2p/socket_client.cc

Index: content/renderer/p2p/socket_client.cc
diff --git a/content/renderer/p2p/socket_client.cc b/content/renderer/p2p/socket_client.cc
index 76357cd2b45fdc0bc08f1d654e74118fbd38eb10..687ba2d1eaa49a4147aaa9176ea654490f2e02ee 100644
--- a/content/renderer/p2p/socket_client.cc
+++ b/content/renderer/p2p/socket_client.cc
@@ -28,6 +28,10 @@ void P2PSocketClient::Init(
     const net::IPEndPoint& local_address,
     const net::IPEndPoint& remote_address,
     P2PSocketClient::Delegate* delegate) {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

if the delegate and ipc threads are not always the same, then this DCHECK will
fire because you post a task to this function from the ipc thread below.

+  // |delegate_| is only accessesed on |delegate_message_loop_|.
+  delegate_ = delegate;

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

...and since you post that task from the ipc thread, you end up touching this
variable from both threads always. Hence, I don't think you've fixed the
problem.

[perkj_chrome, 2012/09/20 12:22:32]

Humm- Stupid me. Sorry - should not have sent out for review before testintg and
reviewing myself.this is still the reason.. 

+
   if (!ipc_message_loop_->BelongsToCurrentThread()) {

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

btw, maybe you could add a note here (assuming that it's true), that
ipc_message_loop_ and delegate_message_loop_ can be the same loop?

[perkj_chrome, 2012/09/20 12:22:32]

In chrome now, they are not the same. 
But I don't think this class should have to know that or care. I remove the if
statement. 


On 2012/09/20 11:48:44, tommi wrote:

     ipc_message_loop_->PostTask(
         FROM_HERE, base::Bind(&P2PSocketClient::Init, this, type, local_address,
@@ -37,7 +41,6 @@ void P2PSocketClient::Init(
 
   DCHECK_EQ(state_, STATE_UNINITIALIZED);

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

and then here, you can DCHECK that delegate_ is not NULL :)

[perkj_chrome, 2012/09/20 12:22:32]

Done.

   state_ = STATE_OPENING;
-  delegate_ = delegate;
   socket_id_ = dispatcher_->RegisterClient(this);
   dispatcher_->SendP2PMessage(new P2PHostMsg_CreateSocket(
       type, socket_id_, local_address, remote_address));
@@ -68,6 +71,7 @@ void P2PSocketClient::Close() {
 }
 
 void P2PSocketClient::DoClose() {
+  DCHECK(ipc_message_loop_->BelongsToCurrentThread());
   if (dispatcher_) {
     if (state_ == STATE_OPEN || state_ == STATE_OPENING ||
         state_ == STATE_ERROR) {
@@ -95,6 +99,7 @@ void P2PSocketClient::OnSocketCreated(const net::IPEndPoint& address) {
 }
 
 void P2PSocketClient::DeliverOnSocketCreated(const net::IPEndPoint& address) {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (delegate_)
     delegate_->OnOpen(address);
 }
@@ -118,6 +123,7 @@ void P2PSocketClient::OnIncomingTcpConnection(const net::IPEndPoint& address) {
 
 void P2PSocketClient::DeliverOnIncomingTcpConnection(
     const net::IPEndPoint& address, scoped_refptr<P2PSocketClient> new_client) {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (delegate_)
     delegate_->OnIncomingTcpConnection(address, new_client);
 }
@@ -131,6 +137,7 @@ void P2PSocketClient::OnError() {
 }
 
 void P2PSocketClient::DeliverOnError() {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (delegate_)
     delegate_->OnError();
 }
@@ -146,6 +153,7 @@ void P2PSocketClient::OnDataReceived(const net::IPEndPoint& address,
 
 void P2PSocketClient::DeliverOnDataReceived(const net::IPEndPoint& address,
                                             const std::vector<char>& data) {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (delegate_)
     delegate_->OnDataReceived(address, data);
 }