Cleanups in WebrtcTransport.

remoting/protocol/webrtc_transport.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/protocol/webrtc_transport.h"
 
 #include "base/callback_helpers.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/task_runner_util.h"
+#include "base/thread_task_runner_handle.h"
 #include "jingle/glue/thread_wrapper.h"
 #include "third_party/libjingle/source/talk/app/webrtc/test/fakeconstraints.h"
 #include "third_party/webrtc/libjingle/xmllite/xmlelement.h"
 #include "third_party/webrtc/modules/audio_device/include/fake_audio_device.h"
 
 using buzz::QName;
 using buzz::XmlElement;
 
 namespace remoting {
 namespace protocol {
 
 namespace {
 
 // Delay after candidate creation before sending transport-info message to
 // accumulate multiple candidates. This is an optimization to reduce number of
 // transport-info messages.
 const int kTransportInfoSendDelayMs = 20;
 
 // XML namespace for the transport elements.
 const char kTransportNamespace[] = "google:remoting:webrtc";
 
-rtc::Thread* InitAndGetRtcThread() {
-  jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
-
-  // TODO(sergeyu): Investigate if it's possible to avoid Send().
-  jingle_glue::JingleThreadWrapper::current()->set_send_allowed(true);
-
-  return jingle_glue::JingleThreadWrapper::current();
-}
-
 // A webrtc::CreateSessionDescriptionObserver implementation used to receive the
 // results of creating descriptions for this end of the PeerConnection.
 class CreateSessionDescriptionObserver
     : public webrtc::CreateSessionDescriptionObserver {
  public:
   typedef base::Callback<void(
       scoped_ptr<webrtc::SessionDescriptionInterface> description,
       const std::string& error)> ResultCallback;
 
   static CreateSessionDescriptionObserver* Create(
@@ skipping 50 matching lines @@
 
  private:
   ResultCallback result_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(SetSessionDescriptionObserver);
 };
 
 }  // namespace
 
 WebrtcTransport::WebrtcTransport(
+    rtc::Thread* worker_thread,
     rtc::scoped_refptr<webrtc::PortAllocatorFactoryInterface>
         port_allocator_factory,
-    TransportRole role,
-    scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner)
+    TransportRole role)
     : port_allocator_factory_(port_allocator_factory),
       role_(role),
-      worker_task_runner_(worker_task_runner),
+      worker_thread_(worker_thread),
       weak_factory_(this) {}
 
 WebrtcTransport::~WebrtcTransport() {}
 
 void WebrtcTransport::Start(EventHandler* event_handler,
                             Authenticator* authenticator) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   event_handler_ = event_handler;
 
   // TODO(sergeyu): Use the |authenticator| to authenticate PeerConnection.
+  jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
 
-  base::PostTaskAndReplyWithResult(
-      worker_task_runner_.get(), FROM_HERE, base::Bind(&InitAndGetRtcThread),
-      base::Bind(&WebrtcTransport::DoStart, weak_factory_.GetWeakPtr()));
+  // TODO(sergeyu): Investigate if it's possible to avoid Send().
+  jingle_glue::JingleThreadWrapper::current()->set_send_allowed(true);
+
+  fake_audio_device_module_.reset(new webrtc::FakeAudioDeviceModule());
+
+  peer_connection_factory_ = webrtc::CreatePeerConnectionFactory(
+      worker_thread_, rtc::Thread::Current(),
+      fake_audio_device_module_.get(), nullptr, nullptr);
+
+  webrtc::PeerConnectionInterface::IceServer stun_server;
+  stun_server.urls.push_back("stun:stun.l.google.com:19302");
+  webrtc::PeerConnectionInterface::RTCConfiguration rtc_config;
+  rtc_config.servers.push_back(stun_server);
+
+  webrtc::FakeConstraints constraints;

[Jamie, 2015/12/09 19:42:22]

Unrelated to this CL, but why are we using FakeConstraints in non-test code?

[Sergey Ulanov, 2015/12/09 19:57:06]

PeerConnection gets MediaConstraintsInterface. FakeConstraints is the only
implementation of that interface that we have available. Examples use
FakeConstraints. I think it should be just renamed to something else. There is
nothing fake about it. It just stores the constraints in memory. I'm not sure
why we even need abstract interface for constraints.

+  constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
+                           webrtc::MediaConstraintsInterface::kValueTrue);
+
+  peer_connection_ = peer_connection_factory_->CreatePeerConnection(
+      rtc_config, &constraints, port_allocator_factory_, nullptr, this);
+
+  data_stream_adapter_.Initialize(peer_connection_,
+                                  role_ == TransportRole::SERVER);
+
+  if (role_ == TransportRole::SERVER)
+    RequestNegotiation();
 }
 
 bool WebrtcTransport::ProcessTransportInfo(XmlElement* transport_info) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (transport_info->Name() != QName(kTransportNamespace, "transport"))
     return false;
 
   if (!peer_connection_)
     return false;
 
   XmlElement* session_description = transport_info->FirstNamed(
       QName(kTransportNamespace, "session-description"));
   if (session_description) {
     webrtc::PeerConnectionInterface::SignalingState expected_state =
-        role_ == TransportRole::SERVER
+        role_ == TransportRole::CLIENT
             ? webrtc::PeerConnectionInterface::kStable
             : webrtc::PeerConnectionInterface::kHaveLocalOffer;
     if (peer_connection_->signaling_state() != expected_state) {
       LOG(ERROR) << "Received unexpected WebRTC session_description. ";
       return false;
     }
 
     std::string type = session_description->Attr(QName(std::string(), "type"));
     std::string sdp = session_description->BodyText();
     if (type.empty() || sdp.empty()) {
-      LOG(ERROR) << "Incorrect session_description format.";
+      LOG(ERROR) << "Incorrect session description format.";
       return false;
     }
 
     webrtc::SdpParseError error;
     scoped_ptr<webrtc::SessionDescriptionInterface> session_description(
         webrtc::CreateSessionDescription(type, sdp, &error));
     if (!session_description) {
-      LOG(ERROR) << "Failed to parse the offer: " << error.description
-                 << " line: " << error.line;
+      LOG(ERROR) << "Failed to parse the session description: "
+                 << error.description << " line: " << error.line;
       return false;
     }
 
     peer_connection_->SetRemoteDescription(
         SetSessionDescriptionObserver::Create(
             base::Bind(&WebrtcTransport::OnRemoteDescriptionSet,
-                       weak_factory_.GetWeakPtr())),
+                       weak_factory_.GetWeakPtr(),
+                       type == webrtc::SessionDescriptionInterface::kOffer)),
         session_description.release());
   }
 
   XmlElement* candidate_element;
   QName candidate_qname(kTransportNamespace, "candidate");
   for (candidate_element = transport_info->FirstNamed(candidate_qname);
        candidate_element;
        candidate_element = candidate_element->NextNamed(candidate_qname)) {
     std::string candidate_str = candidate_element->BodyText();
     std::string sdp_mid =
@@ skipping 34 matching lines @@
 StreamChannelFactory* WebrtcTransport::GetStreamChannelFactory() {
   DCHECK(thread_checker_.CalledOnValidThread());
   return &data_stream_adapter_;
 }
 
 StreamChannelFactory* WebrtcTransport::GetMultiplexedChannelFactory() {
   DCHECK(thread_checker_.CalledOnValidThread());
   return GetStreamChannelFactory();
 }
 
-void WebrtcTransport::DoStart(rtc::Thread* worker_thread) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-
-  jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
-
-  // TODO(sergeyu): Investigate if it's possible to avoid Send().
-  jingle_glue::JingleThreadWrapper::current()->set_send_allowed(true);
-
-  fake_audio_device_module_.reset(new webrtc::FakeAudioDeviceModule());
-
-  peer_connection_factory_ = webrtc::CreatePeerConnectionFactory(
-      worker_thread, rtc::Thread::Current(),
-      fake_audio_device_module_.get(), nullptr, nullptr);
-
-  webrtc::PeerConnectionInterface::IceServer stun_server;
-  stun_server.urls.push_back("stun:stun.l.google.com:19302");
-  webrtc::PeerConnectionInterface::RTCConfiguration rtc_config;
-  rtc_config.servers.push_back(stun_server);
-
-  webrtc::FakeConstraints constraints;
-  constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
-                           webrtc::MediaConstraintsInterface::kValueTrue);
-
-  peer_connection_ = peer_connection_factory_->CreatePeerConnection(
-      rtc_config, &constraints, port_allocator_factory_, nullptr, this);
-
-  data_stream_adapter_.Initialize(peer_connection_,
-                                  role_ == TransportRole::SERVER);
-
-  if (role_ == TransportRole::CLIENT) {
-    webrtc::FakeConstraints offer_config;
-    offer_config.AddMandatory(
-        webrtc::MediaConstraintsInterface::kOfferToReceiveVideo,
-        webrtc::MediaConstraintsInterface::kValueTrue);
-    offer_config.AddMandatory(
-        webrtc::MediaConstraintsInterface::kOfferToReceiveAudio,
-        webrtc::MediaConstraintsInterface::kValueFalse);
-    offer_config.AddMandatory(
-        webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
-        webrtc::MediaConstraintsInterface::kValueTrue);
-    peer_connection_->CreateOffer(
-        CreateSessionDescriptionObserver::Create(
-            base::Bind(&WebrtcTransport::OnLocalSessionDescriptionCreated,
-                       weak_factory_.GetWeakPtr())),
-        &offer_config);
-  }
-}
-
 void WebrtcTransport::OnLocalSessionDescriptionCreated(
     scoped_ptr<webrtc::SessionDescriptionInterface> description,
     const std::string& error) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!peer_connection_)
     return;
 
   if (!description) {
     LOG(ERROR) << "PeerConnection offer creation failed: " << error;
@@ skipping 34 matching lines @@
 
   if (!success) {
     LOG(ERROR) << "Failed to set local description: " << error;
     Close(CHANNEL_CONNECTION_ERROR);
     return;
   }
 
   AddPendingCandidatesIfPossible();
 }
 
-void WebrtcTransport::OnRemoteDescriptionSet(bool success,
+void WebrtcTransport::OnRemoteDescriptionSet(bool send_answer,
+                                             bool success,
                                              const std::string& error) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!peer_connection_)
     return;
 
   if (!success) {
     LOG(ERROR) << "Failed to set local description: " << error;
     Close(CHANNEL_CONNECTION_ERROR);
     return;
   }
 
   // Create and send answer on the server.
-  if (role_ == TransportRole::SERVER) {
+  if (send_answer) {
     peer_connection_->CreateAnswer(
         CreateSessionDescriptionObserver::Create(
             base::Bind(&WebrtcTransport::OnLocalSessionDescriptionCreated,
                        weak_factory_.GetWeakPtr())),
         nullptr);
   }
 
   AddPendingCandidatesIfPossible();
 }
 
@@ skipping 25 matching lines @@
 }
 
 void WebrtcTransport::OnDataChannel(
     webrtc::DataChannelInterface* data_channel) {
   DCHECK(thread_checker_.CalledOnValidThread());
   data_stream_adapter_.OnIncomingDataChannel(data_channel);
 }
 
 void WebrtcTransport::OnRenegotiationNeeded() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // TODO(sergeyu): Figure out what needs to happen here.
+
+  if (role_ == TransportRole::SERVER) {
+    RequestNegotiation();
+  } else {
+    // TODO(sergeyu): Is it necessary to support renegotiation initiated by the
+    // client?
+    NOTIMPLEMENTED();
+  }
+}
+
+void WebrtcTransport::RequestNegotiation() {
+  DCHECK(role_ == TransportRole::SERVER);
+
+  if (!negotiation_pending_) {
+    negotiation_pending_ = true;
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&WebrtcTransport::SendOffer, weak_factory_.GetWeakPtr()));
+  }
 }
 
 void WebrtcTransport::OnIceConnectionChange(
     webrtc::PeerConnectionInterface::IceConnectionState new_state) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (new_state == webrtc::PeerConnectionInterface::kIceConnectionConnected)
     event_handler_->OnTransportConnected();
 }
 
@@ skipping 36 matching lines @@
         new XmlElement(QName(kTransportNamespace, "transport"), true));
 
     // Delay sending the new candidates in case we get more candidates
     // that we can send in one message.
     transport_info_timer_.Start(
         FROM_HERE, base::TimeDelta::FromMilliseconds(kTransportInfoSendDelayMs),
         this, &WebrtcTransport::SendTransportInfo);
   }
 }
 
+void WebrtcTransport::SendOffer() {
+  DCHECK(role_ == TransportRole::SERVER);
+
+  DCHECK(negotiation_pending_);
+  negotiation_pending_ = false;
+
+  webrtc::FakeConstraints offer_config;
+  offer_config.AddMandatory(
+      webrtc::MediaConstraintsInterface::kOfferToReceiveVideo,
+      webrtc::MediaConstraintsInterface::kValueTrue);
+  offer_config.AddMandatory(
+      webrtc::MediaConstraintsInterface::kOfferToReceiveAudio,
+      webrtc::MediaConstraintsInterface::kValueFalse);
+  offer_config.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
+                            webrtc::MediaConstraintsInterface::kValueTrue);
+  peer_connection_->CreateOffer(
+      CreateSessionDescriptionObserver::Create(
+          base::Bind(&WebrtcTransport::OnLocalSessionDescriptionCreated,
+                     weak_factory_.GetWeakPtr())),
+      &offer_config);
+}
+
 void WebrtcTransport::SendTransportInfo() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(pending_transport_info_message_);
 
   event_handler_->OnOutgoingTransportInfo(
       pending_transport_info_message_.Pass());
   pending_transport_info_message_.reset();
 }
 
 void WebrtcTransport::AddPendingCandidatesIfPossible() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (peer_connection_->signaling_state() ==
       webrtc::PeerConnectionInterface::kStable) {
     for (auto candidate : pending_incoming_candidates_) {
       if (!peer_connection_->AddIceCandidate(candidate)) {
         LOG(ERROR) << "Failed to add incoming candidate";
         Close(INCOMPATIBLE_PROTOCOL);
         return;
       }
     }
     pending_incoming_candidates_.clear();
   }
 }
 
 WebrtcTransportFactory::WebrtcTransportFactory(
+    rtc::Thread* worker_thread,
     SignalStrategy* signal_strategy,
     rtc::scoped_refptr<webrtc::PortAllocatorFactoryInterface>
         port_allocator_factory,
     TransportRole role)
-    : signal_strategy_(signal_strategy),
+    : worker_thread_(worker_thread),
+      signal_strategy_(signal_strategy),
       port_allocator_factory_(port_allocator_factory),
-      role_(role),
-      worker_thread_("ChromotingWebrtcWorkerThread") {
-  worker_thread_.StartWithOptions(
-      base::Thread::Options(base::MessageLoop::TYPE_IO, 0));
-}
+      role_(role) {}
 
 WebrtcTransportFactory::~WebrtcTransportFactory() {}
 
 scoped_ptr<Transport> WebrtcTransportFactory::CreateTransport() {
-  return make_scoped_ptr(new WebrtcTransport(port_allocator_factory_, role_,
-                                             worker_thread_.task_runner()));
+  return make_scoped_ptr(
+      new WebrtcTransport(worker_thread_, port_allocator_factory_, role_));
 }
 
 }  // namespace protocol
 }  // namespace remoting