From 245989b22ac6580d5a42346d7aab7b676dbc457e Mon Sep 17 00:00:00 2001
From: Tommi <tommi@webrtc.org>
Date: Tue, 24 Mar 2015 17:56:21 +0100
Subject: [PATCH] Address comments from cr 43769004. - Remove unnecessary hop
 to worker from OnChannelRequestSignaling_s. - Remove now-not-needed component
 param. - Update documentation.

R=juberti@webrtc.org
BUG=4444

Review URL: https://webrtc-codereview.appspot.com/42839004

Cr-Commit-Position: refs/heads/master@{#8852}
---
 webrtc/p2p/base/transport.cc | 37 ++++++++++++++----------------------
 webrtc/p2p/base/transport.h  |  4 ++--
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/webrtc/p2p/base/transport.cc b/webrtc/p2p/base/transport.cc
index b5c6b05c4e..10a069683c 100644
--- a/webrtc/p2p/base/transport.cc
+++ b/webrtc/p2p/base/transport.cc
@@ -563,24 +563,17 @@ TransportState Transport::GetTransportState_s(bool read) {
 
 void Transport::OnChannelRequestSignaling(TransportChannelImpl* channel) {
   ASSERT(worker_thread()->IsCurrent());
-  ChannelParams* params = new ChannelParams(channel->component());
-  signaling_thread()->Post(this, MSG_REQUESTSIGNALING, params);
-}
-
-void Transport::OnChannelRequestSignaling_s(int component) {
-  ASSERT(signaling_thread()->IsCurrent());
-  LOG(LS_INFO) << "Transport: " << content_name_ << ", allocating candidates";
   // Resetting ICE state for the channel.
-  worker_thread_->Invoke<void>(
-      Bind(&Transport::OnChannelRequestSignaling_w, this, component));
-  SignalRequestSignaling(this);
-}
-
-void Transport::OnChannelRequestSignaling_w(int component) {
-  ASSERT(worker_thread()->IsCurrent());
-  ChannelMap::iterator iter = channels_.find(component);
+  ChannelMap::iterator iter = channels_.find(channel->component());
   if (iter != channels_.end())
     iter->second.set_candidates_allocated(false);
+  signaling_thread()->Post(this, MSG_REQUESTSIGNALING, nullptr);
+}
+
+void Transport::OnChannelRequestSignaling_s() {
+  ASSERT(signaling_thread()->IsCurrent());
+  LOG(LS_INFO) << "Transport: " << content_name_ << ", allocating candidates";
+  SignalRequestSignaling(this);
 }
 
 void Transport::OnChannelCandidateReady(TransportChannelImpl* channel,
@@ -742,9 +735,10 @@ bool Transport::SetLocalTransportDescription_w(
   // point. |local_description_| seems to always be modified on the worker
   // thread, so we should be able to use it here without grabbing the lock.
   // However, we _might_ need it before the call to reset() below?
-  // Actually, if we ever give out a pointer to the local description to
-  // another thread, won't we run into trouble? (see local_description() in
-  // the header file - no thread check, so I'm not sure from where it's called).
+  // Raw access to |local_description_| is granted to derived transports outside
+  // of locking (see local_description() in the header file).
+  // The contract is that the derived implementations must be aware of when the
+  // description might change and do appropriate synchronization.
   rtc::CritScope cs(&crit_);
   if (local_description_ && IceCredentialsChanged(*local_description_, desc)) {
     IceRole new_ice_role = (action == CA_OFFER) ? ICEROLE_CONTROLLING
@@ -914,11 +908,8 @@ void Transport::OnMessage(rtc::Message* msg) {
     case MSG_WRITESTATE:
       OnChannelWritableState_s();
       break;
-    case MSG_REQUESTSIGNALING: {
-        ChannelParams* params = static_cast<ChannelParams*>(msg->pdata);
-        OnChannelRequestSignaling_s(params->component);
-        delete params;
-      }
+    case MSG_REQUESTSIGNALING:
+      OnChannelRequestSignaling_s();
       break;
     case MSG_CANDIDATEREADY:
       OnChannelCandidateReady_s();
diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h
index 016c9af7ba..eda112bb7b 100644
--- a/webrtc/p2p/base/transport.h
+++ b/webrtc/p2p/base/transport.h
@@ -386,8 +386,7 @@ class Transport : public rtc::MessageHandler,
   void OnRemoteCandidate_w(const Candidate& candidate);
   void OnChannelReadableState_s();
   void OnChannelWritableState_s();
-  void OnChannelRequestSignaling_s(int component);
-  void OnChannelRequestSignaling_w(int component);
+  void OnChannelRequestSignaling_s();
   void OnConnecting_s();
   void OnChannelRouteChange_s(const TransportChannel* channel,
                               const Candidate& remote_candidate);
@@ -433,6 +432,7 @@ class Transport : public rtc::MessageHandler,
   rtc::scoped_ptr<TransportDescription> local_description_;
   rtc::scoped_ptr<TransportDescription> remote_description_;
 
+  // TODO(tommi): Make sure we only use this on the worker thread.
   ChannelMap channels_;
   // Buffers the ready_candidates so that SignalCanidatesReady can
   // provide them in multiples.