Changes to WebRtcSession after Provider(s) interface addition.

Review URL: http://webrtc-codereview.appspot.com/201001

git-svn-id: http://webrtc.googlecode.com/svn/trunk@695 4adac7df-926f-26a2-2b94-8c16560cd09d

third_party_mods/libjingle/source/talk/app/webrtc_dev/webrtcsession.cc

@@ -34,6 +34,7 @@
 #include "talk/base/logging.h"
 #include "talk/session/phone/channel.h"
 #include "talk/session/phone/channelmanager.h"
+#include "talk/session/phone/mediasession.h"
 
 using cricket::MediaContentDescription;
 
@@ -59,13 +60,12 @@ WebRtcSession::WebRtcSession(cricket::ChannelManager* channel_manager,
                              talk_base::Thread* signaling_thread,
                              talk_base::Thread* worker_thread,
                              cricket::PortAllocator* port_allocator)
-    : observer_(NULL),
+    : cricket::BaseSession(signaling_thread, worker_thread, port_allocator,
-      cricket::BaseSession(signaling_thread, worker_thread, port_allocator,
           talk_base::ToString(talk_base::CreateRandomId()),
-                           cricket::NS_JINGLE_RTP, true) {
+          cricket::NS_JINGLE_RTP, true),
-  // TODO(mallinath) - Remove initiator flag from BaseSession. As it's being
+      channel_manager_(channel_manager),
-  // used only by cricket::Session.
+      observer_(NULL),
-  channel_manager_ = channel_manager;
+      session_desc_factory_(channel_manager) {
 }
 
 WebRtcSession::~WebRtcSession() {
@@ -85,30 +85,27 @@ void WebRtcSession::Terminate() {
   }
 }
 
-void WebRtcSession::OnSignalUpdateSessionDescription(
+bool WebRtcSession::CreateChannels() {
-    const cricket::SessionDescription* local_desc,
+  voice_channel_.reset(channel_manager_->CreateVoiceChannel(
-    const cricket::SessionDescription* remote_desc,
+      this, cricket::CN_AUDIO, true));
-    const cricket::Candidates& remote_candidates) {
+  if (!voice_channel_.get()) {
-  // Session updates are not supported yet. If session is in progress state
+    LOG(LS_ERROR) << "Failed to create voice channel";
-  // ignore this callback.
+    return false;
-  if (state() == STATE_INPROGRESS) {
-    ProcessSessionUpdate(local_desc, remote_desc);
-    if (remote_candidates.size() > 0) {
-      SetRemoteCandidates(remote_candidates);
   }
-    return;
+
+  video_channel_.reset(channel_manager_->CreateVideoChannel(
+      this, cricket::CN_VIDEO, true, voice_channel_.get()));
+  if (!video_channel_.get()) {
+    LOG(LS_ERROR) << "Failed to create video channel";
+    return false;
   }
-  // Local session and remote session descriptions are available before
+
-  // any state change. So for session it's doesn't matter it's initiator
+  // TransportProxies and TransportChannels will be created when
-  // or receiver of the call. Session will be treated as initiator.
+  // CreateVoiceChannel and CreateVideoChannel are called.
-  // Apply first local description
+  // Try connecting all transport channels. This is necessary to generate
-  set_local_description(local_desc);
+  // ICE candidates.
-  SetState(STATE_SENTINITIATE);
+  SpeculativelyConnectAllTransportChannels();
-  // Applying remote description on the session
+  return true;
-  set_remote_description(const_cast<cricket::SessionDescription*>(remote_desc));
-  SetState(STATE_RECEIVEDACCEPT);
-  // Set remote candidates
-  SetRemoteCandidates(remote_candidates);
 }
 
 void WebRtcSession::SetRemoteCandidates(
@@ -150,36 +147,13 @@ void WebRtcSession::SetRemoteCandidates(
       video_proxy->CompleteNegotiation();
       // TODO(mallinath) - Add a interface to TransportProxy to accept
       // remote candidate list.
-      video_proxy->impl()->OnRemoteCandidates(audio_candidates);
+      video_proxy->impl()->OnRemoteCandidates(video_candidates);
     } else {
       LOG(LS_INFO) << "No video TransportProxy exists";
     }
   }
 }
 
-bool WebRtcSession::CreateChannels() {
-  voice_channel_.reset(channel_manager_->CreateVoiceChannel(
-      this, cricket::CN_AUDIO, true));
-  if (!voice_channel_.get()) {
-    LOG(LS_ERROR) << "Failed to create voice channel";
-    return false;
-  }
-
-  video_channel_.reset(channel_manager_->CreateVideoChannel(
-      this, cricket::CN_VIDEO, true, voice_channel_.get()));
-  if (!video_channel_.get()) {
-    LOG(LS_ERROR) << "Failed to create video channel";
-    return false;
-  }
-
-  // TransportProxies and TransportChannels will be created when
-  // CreateVoiceChannel and CreateVideoChannel are called.
-  // Try connecting all transport channels. This is necessary to generate
-  // ICE candidates.
-  SpeculativelyConnectAllTransportChannels();
-  return true;
-}
-
 void WebRtcSession::OnTransportRequestSignaling(
     cricket::Transport* transport) {
   ASSERT(signaling_thread()->IsCurrent());
@@ -212,12 +186,8 @@ void WebRtcSession::OnTransportCandidatesReady(
   if (local_candidates_.size() == kAllowedCandidates)
     return;
   InsertTransportCandidates(candidates);
-  if (local_candidates_.size() == kAllowedCandidates) {
+  if (local_candidates_.size() == kAllowedCandidates && observer_) {
-    if(observer_)
     observer_->OnCandidatesReady(local_candidates_);
-    // TODO(mallinath) - Remove signal when a new interface added for
-    // PC signaling.
-    SignalCandidatesReady(this, local_candidates_);
   }
 }
 
@@ -262,56 +232,6 @@ bool WebRtcSession::CheckCandidate(const std::string& name) {
   return ret;
 }
 
-void WebRtcSession::ProcessSessionUpdate(
-    const cricket::SessionDescription* local_desc,
-    const cricket::SessionDescription* remote_desc) {
-
-  if (local_desc) {
-    ProcessLocalMediaChanges(local_desc);
-  }
-  if (remote_desc) {
-    ProcessRemoteMediaChanges(remote_desc);
-  }
-}
-
-bool WebRtcSession::GetAudioSourceParamInfo(
-    const cricket::SessionDescription* sdesc,
-    cricket::Sources* sources) {
-  bool ret = false;
-  const cricket::ContentInfo* content = GetFirstAudioContent(sdesc);
-  if (content) {
-    const MediaContentDescription* audio_desc =
-        static_cast<const MediaContentDescription*> (content->description);
-    *sources = audio_desc->sources();
-    ret = true;
-  }
-  return ret;
-}
-
-bool WebRtcSession::GetVideoSourceParamInfo(
-    const cricket::SessionDescription* sdesc,
-    cricket::Sources* sources) {
-  bool ret = false;
-  const cricket::ContentInfo* content = GetFirstVideoContent(sdesc);
-  if (content) {
-    const MediaContentDescription* video_desc =
-        static_cast<const MediaContentDescription*> (content->description);
-    *sources = video_desc->sources();
-    ret = true;
-  }
-  return ret;
-}
-
-void WebRtcSession::ProcessLocalMediaChanges(
-    const cricket::SessionDescription* sdesc) {
-  // TODO(mallinath) - Handling of local media stream changes in active session
-}
-
-void WebRtcSession::ProcessRemoteMediaChanges(
-    const cricket::SessionDescription* sdesc) {
-  // TODO(mallinath) - Handling of remote media stream changes in active session
-}
-
 void WebRtcSession::SetCaptureDevice(uint32 ssrc,
                                      VideoCaptureModule* camera) {
   // should be called from a signaling thread
@@ -331,4 +251,72 @@ void WebRtcSession::SetRemoteRenderer(uint32 ssrc,
   video_channel_->SetRenderer(ssrc, renderer);
 }
 
+const cricket::SessionDescription* WebRtcSession::ProvideOffer(
+    const cricket::MediaSessionOptions& options) {
+  // TODO(mallinath) - Sanity check for options.
+  cricket::SessionDescription* offer(
+      session_desc_factory_.CreateOffer(options));
+  set_local_description(offer);
+  return offer;
+}
+
+const cricket::SessionDescription* WebRtcSession::SetRemoteSessionDescription(
+    const cricket::SessionDescription* remote_offer,
+    const std::vector<cricket::Candidate>& remote_candidates) {
+  set_remote_description(
+      const_cast<cricket::SessionDescription*>(remote_offer));
+  SetRemoteCandidates(remote_candidates);
+  return remote_offer;
+}
+
+const cricket::SessionDescription* WebRtcSession::ProvideAnswer(
+    const cricket::MediaSessionOptions& options) {
+  cricket::SessionDescription* answer(
+      session_desc_factory_.CreateAnswer(remote_description(), options));
+  set_local_description(answer);
+  return answer;
+}
+
+void WebRtcSession::NegotiationDone() {
+  // No state change after state moved to progress state.
+  if (state() == STATE_INIT) {
+    SetState(STATE_SENTINITIATE);
+    SetState(STATE_RECEIVEDACCEPT);
+  }
+
+  // Enabling channels
+  voice_channel_->Enable(true);
+  video_channel_->Enable(true);
+
+  const cricket::ContentInfo* audio_info =
+      cricket::GetFirstAudioContent(local_description());
+  if (audio_info) {
+    const cricket::MediaContentDescription* audio_content =
+        static_cast<const cricket::MediaContentDescription*>(
+            audio_info->description);
+    // Since channels are currently not supporting multiple send streams,
+    // we can remove stream from a session by muting it.
+    // TODO(mallinath) - Change needed when multiple send streams support
+    // is available.
+    if (audio_content->sources().size() == 0) {
+      voice_channel_->Mute(true);
+    }
+  }
+
+  const cricket::ContentInfo* video_info =
+      cricket::GetFirstVideoContent(local_description());
+  if (video_info) {
+    const cricket::MediaContentDescription* video_content =
+        static_cast<const cricket::MediaContentDescription*>(
+            video_info->description);
+    // Since channels are currently not supporting multiple send streams,
+    // we can remove stream from a session by muting it.
+    // TODO(mallinath) - Change needed when multiple send streams support
+    // is available.
+    if (video_content->sources().size() == 0) {
+      video_channel_->Mute(true);
+    }
+  }
+}
+
 }  // namespace webrtc

third_party_mods/libjingle/source/talk/app/webrtc_dev/webrtcsession.h

@@ -31,13 +31,13 @@
 #include <string>
 #include <vector>
 
+#include "talk/app/webrtc_dev/mediastreamprovider.h"
+#include "talk/app/webrtc_dev/sessiondescriptionprovider.h"
+#include "talk/app/webrtc_dev/webrtcsessionobserver.h"
 #include "talk/base/sigslot.h"
 #include "talk/base/thread.h"
 #include "talk/p2p/base/session.h"
 #include "talk/session/phone/mediasession.h"
-#include "talk/app/webrtc_dev/mediastreamprovider.h"
-#include "talk/app/webrtc_dev/sessiondescriptionprovider.h"
-#include "talk/app/webrtc_dev/webrtcsessionobserver.h"
 
 namespace cricket {
 class ChannelManager;
@@ -47,10 +47,6 @@ class VoiceChannel;
 }
 
 namespace webrtc {
-class MediaStream;
-class PeerConnectionMessage;
-class PeerConnectionSignaling;
-class StreamCollection;
 
 class WebRtcSession : public cricket::BaseSession,
                       public MediaProviderInterface,
@@ -64,6 +60,10 @@ class WebRtcSession : public cricket::BaseSession,
 
   bool Initialize();
 
+  void RegisterObserver(WebRtcSessionObserver* observer) {
+    observer_ = observer;
+  }
+
   const cricket::VoiceChannel* voice_channel() const {
     return voice_channel_.get();
   }
@@ -74,15 +74,18 @@ class WebRtcSession : public cricket::BaseSession,
   // Generic error message callback from WebRtcSession.
   // TODO(mallinath) - It may be necessary to supply error code as well.
   sigslot::signal0<> SignalError;
-  // This signal added for testing. Shouldn't be registered by other
-  // objects.
-  sigslot::signal2<WebRtcSession*,
-                   cricket::Candidates&> SignalCandidatesReady;
-
-  void ProcessSessionUpdate(const cricket::SessionDescription* local_desc,
-                            const cricket::SessionDescription* remote_desc);
 
  private:
+  // Implements SessionDescriptionProvider
+  virtual const cricket::SessionDescription* ProvideOffer(
+      const cricket::MediaSessionOptions& options);
+  virtual const cricket::SessionDescription* SetRemoteSessionDescription(
+      const cricket::SessionDescription* remote_offer,
+      const std::vector<cricket::Candidate>& remote_candidates);
+  virtual const cricket::SessionDescription* ProvideAnswer(
+      const cricket::MediaSessionOptions& options);
+  virtual void NegotiationDone();
+
   // Implements MediaProviderInterface.
   virtual void SetCaptureDevice(uint32 ssrc, VideoCaptureModule* camera);
   virtual void SetLocalRenderer(uint32 ssrc,
@@ -90,23 +93,6 @@ class WebRtcSession : public cricket::BaseSession,
   virtual void SetRemoteRenderer(uint32 ssrc,
                                  cricket::VideoRenderer* renderer);
 
-  //TODO mallinath: remove.
-  void OnSignalUpdateSessionDescription(
-      const cricket::SessionDescription* local_desc,
-      const cricket::SessionDescription* remote_desc,
-      const cricket::Candidates& remote_candidates);
-
-  // Implements SessionDescriptionProvider
-  virtual const cricket::SessionDescription* ProvideOffer(
-      const cricket::MediaSessionOptions& options) {}
-  virtual const cricket::SessionDescription* SetRemoteSessionDescription(
-      const cricket::SessionDescription* remote_offer,
-      const cricket::Candidates& remote_candidates) {}
-  virtual const cricket::SessionDescription* ProvideAnswer(
-      const cricket::MediaSessionOptions& options) {}
-  virtual void NegotiationDone() {}
-
-
   // Transport related callbacks, override from cricket::BaseSession.
   virtual void OnTransportRequestSignaling(cricket::Transport* transport);
   virtual void OnTransportConnecting(cricket::Transport* transport);
@@ -125,21 +111,13 @@ class WebRtcSession : public cricket::BaseSession,
   bool CheckCandidate(const std::string& name);
   void SetRemoteCandidates(const cricket::Candidates& candidates);
 
-  // Helper methods to get handle to the MediaContentDescription sources param.
-  bool GetAudioSourceParamInfo(const cricket::SessionDescription* sdesc,
-                               cricket::Sources* sources);
-  bool GetVideoSourceParamInfo(const cricket::SessionDescription* sdesc,
-                               cricket::Sources* sources);
-
-  void ProcessLocalMediaChanges(const cricket::SessionDescription* sdesc);
-  void ProcessRemoteMediaChanges(const cricket::SessionDescription* sdesc);
-
  private:
-  WebRtcSessionObserver* observer_;
   talk_base::scoped_ptr<cricket::VoiceChannel> voice_channel_;
   talk_base::scoped_ptr<cricket::VideoChannel> video_channel_;
   cricket::ChannelManager* channel_manager_;
   cricket::Candidates local_candidates_;
+  WebRtcSessionObserver* observer_;
+  cricket::MediaSessionDescriptionFactory session_desc_factory_;
 };
 
 }  // namespace webrtc

third_party_mods/libjingle/source/talk/app/webrtc_dev/webrtcsession_unittest.cc

@@ -32,24 +32,28 @@
 #include "talk/session/phone/channelmanager.h"
 #include "talk/p2p/client/fakeportallocator.h"
 
-class WebRtcSessionTest : public testing::Test,
+class MockWebRtcSessionObserver : public webrtc::WebRtcSessionObserver {
-                          public sigslot::has_slots<> {
  public:
-  cricket::MediaSessionDescriptionFactory* media_factory_;
+  virtual void OnCandidatesReady(
-  WebRtcSessionTest() {
+      const std::vector<cricket::Candidate>& candidates) {
+    for (cricket::Candidates::const_iterator iter = candidates.begin();
+         iter != candidates.end(); ++iter) {
+      candidates_.push_back(*iter);
     }
-
-  ~WebRtcSessionTest() {
   }
+  std::vector<cricket::Candidate> candidates_;
+};
 
+class WebRtcSessionTest : public testing::Test {
+ protected:
   virtual void SetUp() {
     signaling_thread_ = talk_base::Thread::Current();
     worker_thread_ = talk_base::Thread::Current();
     channel_manager_.reset(new cricket::ChannelManager(worker_thread_));
     port_allocator_.reset(
         new cricket::FakePortAllocator(worker_thread_, NULL));
-    media_factory_ =
+    desc_factory_.reset(
-        new cricket::MediaSessionDescriptionFactory(channel_manager_.get());
+        new cricket::MediaSessionDescriptionFactory(channel_manager_.get()));
   }
 
   bool InitializeSession() {
@@ -75,43 +79,53 @@ class WebRtcSessionTest : public testing::Test,
     session_.reset(new webrtc::WebRtcSession(
         channel_manager_.get(), worker_thread_, signaling_thread_,
         port_allocator_.get()));
-    session_->SignalCandidatesReady.connect(
+    session_->RegisterObserver(&observer_);
-        this, &WebRtcSessionTest::OnCandidatesReady);
+    desc_provider_ = session_.get();
     EXPECT_TRUE(InitializeSession());
   }
-  void OnCandidatesReady(webrtc::WebRtcSession* session,
+
-                         cricket::Candidates& candidates) {
+  // Creates an offer with one source ssrc, if ssrc = 0 no source info
-    for (cricket::Candidates::iterator iter = candidates.begin();
+  // video ssrc + 1
-         iter != candidates.end(); ++iter) {
+  void CreateOffer(uint32 ssrc) {
-      local_candidates_.push_back(*iter);
-    }
-  }
-  cricket::Candidates& local_candidates() {
-    return local_candidates_;
-  }
-  cricket::SessionDescription* CreateOffer(bool video) {
     cricket::MediaSessionOptions options;
     options.is_video = true;
-    // Source params not set
+    if (ssrc != 0) {
-    cricket::SessionDescription* sdp = media_factory_->CreateOffer(options);
+      options.audio_sources.push_back(cricket::SourceParam(ssrc, "", ""));
-    return sdp;
+      ++ssrc;
+      options.video_sources.push_back(cricket::SourceParam(ssrc, "", ""));
     }
-  cricket::SessionDescription* CreateAnswer(
+    local_desc_ = desc_provider_->ProvideOffer(options);
-      cricket::SessionDescription* offer, bool video) {
+    ASSERT_TRUE(local_desc_ != NULL);
+  }
+  void CreateAnswer(uint32 ssrc) {
     cricket::MediaSessionOptions options;
-    options.is_video = video;
+    options.is_video = true;
-    cricket::SessionDescription* sdp =
+    if (ssrc != 0) {
-        media_factory_->CreateAnswer(offer, options);
+      options.audio_sources.push_back(cricket::SourceParam(ssrc, "", ""));
+      ++ssrc;
+      options.video_sources.push_back(cricket::SourceParam(ssrc, "", ""));
+    }
+    remote_desc_ = desc_factory_->CreateAnswer(local_desc_, options);
+    ASSERT_TRUE(remote_desc_ != NULL);
+  }
+  void SetRemoteContents() {
+    desc_provider_->SetRemoteSessionDescription(
+        remote_desc_, observer_.candidates_);
+  }
+  void NegotiationDone() {
+    desc_provider_->NegotiationDone();
   }
 
- private:
+  const cricket::SessionDescription* local_desc_;
-  cricket::Candidates local_candidates_;
+  const cricket::SessionDescription* remote_desc_;
-  cricket::Candidates remote_candidates_;
   talk_base::Thread* signaling_thread_;
   talk_base::Thread* worker_thread_;
   talk_base::scoped_ptr<cricket::PortAllocator> port_allocator_;
   talk_base::scoped_ptr<webrtc::WebRtcSession> session_;
+  webrtc::SessionDescriptionProvider* desc_provider_;
   talk_base::scoped_ptr<cricket::ChannelManager> channel_manager_;
+  talk_base::scoped_ptr<cricket::MediaSessionDescriptionFactory> desc_factory_;
+  MockWebRtcSessionObserver observer_;
 };
 
 TEST_F(WebRtcSessionTest, TestInitialize) {
@@ -119,6 +133,14 @@ TEST_F(WebRtcSessionTest, TestInitialize) {
   EXPECT_TRUE(CheckChannels());
   CheckTransportChannels();
   talk_base::Thread::Current()->ProcessMessages(1000);
-  EXPECT_EQ(4u, local_candidates().size());
+  EXPECT_EQ(4u, observer_.candidates_.size());
+}
+
+// TODO(mallinath) - Adding test cases for session.
+TEST_F(WebRtcSessionTest, DISABLE_TestOfferAnswer) {
+  WebRtcSessionTest::Init();
+  EXPECT_TRUE(CheckChannels());
+  CheckTransportChannels();
+  talk_base::Thread::Current()->ProcessMessages(1);
 }