Interface changes after we have the Serialize and Deserialize.

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

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

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

@@ -224,8 +224,7 @@ void PeerConnectionImpl::OnMessage(talk_base::Message* msg) {
     }
     case MSG_PROCESSSIGNALINGMESSAGE: {
       SignalingParams* params(static_cast<SignalingParams*> (data));
-      // TODO(perkj) Deserialize params->msg into a PeerConnection Message.
-      signaling_->ProcessSignalingMessage(NULL, params->local_streams);
+      signaling_->ProcessSignalingMessage(params->msg, params->local_streams);
       delete data;  // Because it is Posted.
       break;
     }
@@ -239,10 +238,8 @@ void PeerConnectionImpl::OnMessage(talk_base::Message* msg) {
 }
 
 void PeerConnectionImpl::OnNewPeerConnectionMessage(
-    PeerConnectionMessage* message) {
-  // TODO(perkj): serialize the message.
-  std::string msg;
-  observer_->OnSignalingMessage(msg);
+    const std::string& message) {
+  observer_->OnSignalingMessage(message);
 }
 
 void PeerConnectionImpl::OnRemoteStreamAdded(MediaStream* remote_stream) {

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

@@ -76,7 +76,7 @@ class PeerConnectionImpl : public PeerConnection,
   void OnMessage(talk_base::Message* msg);
 
   // Signals from PeerConnectionSignaling.
-  void OnNewPeerConnectionMessage(PeerConnectionMessage* message);
+  void OnNewPeerConnectionMessage(const std::string& message);
   void OnRemoteStreamAdded(MediaStream* remote_stream);
   void OnRemoteStreamRemoved(MediaStream* remote_stream);
 

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

@@ -45,10 +45,9 @@ scoped_refptr<PeerConnectionMessage> PeerConnectionMessage::Create(
     const std::string& message) {
   scoped_refptr<PeerConnectionMessage>pc_message(new
       RefCountImpl<PeerConnectionMessage> ());
-  if (pc_message->Deserialize(message))
-    return pc_message;
-  else
+  if (!pc_message->Deserialize(message))
     return NULL;
+  return pc_message;
 }
 
 scoped_refptr<PeerConnectionMessage> PeerConnectionMessage::CreateErrorMessage(
@@ -78,9 +77,8 @@ PeerConnectionMessage::PeerConnectionMessage(ErrorCode error)
       error_code_(error) {
 }
 
-bool PeerConnectionMessage::Serialize(std::string* message) {
-  return JsonSerialize(type_, error_code_,
-      desc_.get(), candidates_, message);
+std::string PeerConnectionMessage::Serialize() {
+  return JsonSerialize(type_, error_code_, desc_.get(), candidates_);
 }
 
 bool PeerConnectionMessage::Deserialize(std::string message) {

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

@@ -78,7 +78,7 @@ class PeerConnectionMessage : public RefCount {
   ErrorCode error() {return error_code_;}
   const cricket::SessionDescription* desc() {return desc_.get();}
 
-  bool Serialize(std::string* message);
+  std::string Serialize();
   std::vector<cricket::Candidate>& candidates() { return candidates_; }
 
  protected:
@@ -95,7 +95,6 @@ class PeerConnectionMessage : public RefCount {
   ErrorCode error_code_;
   talk_base::scoped_ptr<cricket::SessionDescription> desc_;
   std::vector<cricket::Candidate> candidates_;
-
 };
 
 }  // namespace webrtc

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

@@ -104,28 +104,25 @@ TEST_F(PeerConnectionMessageTest, Serialize) {
   scoped_refptr<PeerConnectionMessage> pc_message;
 
   // Offer
-  talk_base::scoped_ptr<cricket::SessionDescription> offer(
-      session_description_factory_->CreateOffer(options_));
+  cricket::SessionDescription* offer =
+      session_description_factory_->CreateOffer(options_);
   pc_message = PeerConnectionMessage::Create(PeerConnectionMessage::kOffer,
-      offer.get(), candidates_);
-  EXPECT_TRUE(pc_message->Serialize(&message));
-  pc_message.release();
+      offer, candidates_);
+  message = pc_message->Serialize();
   LOG(LS_INFO) << message;
 
   // Answer
-  talk_base::scoped_ptr<cricket::SessionDescription> answer(
-      session_description_factory_->CreateAnswer(offer.get(), options_));
+  cricket::SessionDescription* answer =
+      session_description_factory_->CreateAnswer(offer, options_);
   pc_message = PeerConnectionMessage::Create(PeerConnectionMessage::kAnswer,
-      answer.get(), candidates_);
-  EXPECT_TRUE(pc_message->Serialize(&message));
-  pc_message.release();
+      answer, candidates_);
+  message = pc_message->Serialize();
   LOG(LS_INFO) << message;
 
   // Error
   pc_message = PeerConnectionMessage::CreateErrorMessage(
       PeerConnectionMessage::kParseError);
-  EXPECT_TRUE(pc_message->Serialize(&message));
-  pc_message.release();
+  message = pc_message->Serialize();
   LOG(LS_INFO) << message;
 
   // TODO(ronghuawu): Verify the serialized message.
@@ -135,50 +132,44 @@ TEST_F(PeerConnectionMessageTest, Deserialize) {
   std::string message_ref;
   std::string message_result;
   scoped_refptr<PeerConnectionMessage> pc_message;
+  cricket::SessionDescription* offer =
+      session_description_factory_->CreateOffer(options_);
+  cricket::SessionDescription* answer =
+      session_description_factory_->CreateAnswer(offer, options_);
 
   // Offer
-  talk_base::scoped_ptr<cricket::SessionDescription> offer(
-      session_description_factory_->CreateOffer(options_));
   pc_message = PeerConnectionMessage::Create(PeerConnectionMessage::kOffer,
-      offer.get(), candidates_);
-  EXPECT_TRUE(pc_message->Serialize(&message_ref));
-  pc_message.release();
+      offer, candidates_);
+  message_ref = pc_message->Serialize();
   LOG(LS_INFO) << "The reference message: " << message_ref;
 
   // Deserialize Offer
   pc_message = PeerConnectionMessage::Create(message_ref);
-  EXPECT_TRUE(pc_message->Serialize(&message_result));
-  pc_message.release();
+  message_result = pc_message->Serialize();
   LOG(LS_INFO) << "The result message: " << message_result;
   EXPECT_EQ(message_ref, message_result);
 
   // Answer
-  talk_base::scoped_ptr<cricket::SessionDescription> answer(
-      session_description_factory_->CreateAnswer(offer.get(), options_));
   pc_message = PeerConnectionMessage::Create(PeerConnectionMessage::kAnswer,
-      answer.get(), candidates_);
-  EXPECT_TRUE(pc_message->Serialize(&message_ref));
-  pc_message.release();
+      answer, candidates_);
+  message_ref = pc_message->Serialize();
   LOG(LS_INFO) << "The reference message: " << message_ref;
 
   // Deserialize Answer
   pc_message = PeerConnectionMessage::Create(message_ref);
-  EXPECT_TRUE(pc_message->Serialize(&message_result));
-  pc_message.release();
+  message_result = pc_message->Serialize();
   LOG(LS_INFO) << "The result message: " << message_result;
   EXPECT_EQ(message_ref, message_result);
 
   // Error
   pc_message = PeerConnectionMessage::CreateErrorMessage(
       PeerConnectionMessage::kParseError);
-  EXPECT_TRUE(pc_message->Serialize(&message_ref));
-  pc_message.release();
+  message_ref = pc_message->Serialize();
   LOG(LS_INFO) << "The reference message: " << message_ref;
 
   // Deserialize Error
   pc_message = PeerConnectionMessage::Create(message_ref);
-  EXPECT_TRUE(pc_message->Serialize(&message_result));
-  pc_message.release();
+  message_result = pc_message->Serialize();
   LOG(LS_INFO) << "The result message: " << message_result;
   EXPECT_EQ(message_ref, message_result);
 }

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

@@ -106,13 +106,22 @@ void PeerConnectionSignaling::Initialize(
 }
 
 void PeerConnectionSignaling::ProcessSignalingMessage(
-    PeerConnectionMessage* message,
+    const std::string& message,
     StreamCollection* local_streams) {
   ASSERT(talk_base::Thread::Current() == signaling_thread_);
 
-  switch (message->type()) {
+  scoped_refptr<PeerConnectionMessage> signaling_message =
+      PeerConnectionMessage::Create(message);
+  if (!signaling_message.get()) {
+    signaling_message = PeerConnectionMessage::CreateErrorMessage(
+        PeerConnectionMessage::kParseError);
+    SignalNewPeerConnectionMessage(signaling_message->Serialize());
+  }
+
+  switch (signaling_message->type()) {
     case PeerConnectionMessage::kOffer: {
-      queued_received_offer_ = RemoteOfferPair(message, local_streams);
+      queued_received_offer_ =
+          RemoteOfferPair(signaling_message, local_streams);
       // If we are still Initializing we need to wait before we can handle
        // the offer. Queue it and handle it when the state change.
       if (state_ == kInitializing) {
@@ -129,7 +138,7 @@ void PeerConnectionSignaling::ProcessSignalingMessage(
         scoped_refptr<PeerConnectionMessage> msg =
             PeerConnectionMessage::CreateErrorMessage(
                 PeerConnectionMessage::kWrongState);
-        SignalNewPeerConnectionMessage(msg);
+        SignalNewPeerConnectionMessage(msg->Serialize());
         break;
       }
       if (state_ == kGlare) {
@@ -144,13 +153,12 @@ void PeerConnectionSignaling::ProcessSignalingMessage(
         return;
       // Signal the resulting local and remote session description.
       SignalUpdateSessionDescription(last_send_offer_->desc(),
-                                     message->desc(),
-                                     message->candidates());
-
-      UpdateRemoteStreams(message->desc());
+                                     signaling_message->desc(),
+                                     signaling_message->candidates());
+      UpdateRemoteStreams(signaling_message->desc());
       scoped_refptr<StreamCollection> streams(queued_offers_.front());
       queued_offers_.pop_front();
-      UpdateSendingLocalStreams(message->desc(), streams);
+      UpdateSendingLocalStreams(signaling_message->desc(), streams);
       // Check if we have more offers waiting in the queue.
       if (queued_offers_.size() > 0) {
         // Send the next offer.
@@ -161,8 +169,8 @@ void PeerConnectionSignaling::ProcessSignalingMessage(
       break;
     }
     case PeerConnectionMessage::kError: {
-      if (message->error() != PeerConnectionMessage::kWrongState)
-        SignalErrorMessageReceived(message->error());
+      if (signaling_message->error() != PeerConnectionMessage::kWrongState)
+        SignalErrorMessageReceived(signaling_message->error());
 
       // An error have occurred that we can't do anything about.
       // Reset the state and wait for user action.
@@ -200,7 +208,6 @@ void PeerConnectionSignaling::CreateOffer_s() {
   ASSERT(queued_offers_.size() > 0);
   scoped_refptr<StreamCollection> local_streams(queued_offers_.front());
   cricket::MediaSessionOptions options;
-  options.is_video = true;
   InitMediaSessionOptions(&options, local_streams);
 
   talk_base::scoped_ptr<cricket::SessionDescription> offer(
@@ -215,7 +222,7 @@ void PeerConnectionSignaling::CreateOffer_s() {
   // before we can handle a remote offer.
   // We also need the response before we are allowed to start send media.
   last_send_offer_ = offer_message;
-  SignalNewPeerConnectionMessage(offer_message);
+  SignalNewPeerConnectionMessage(offer_message->Serialize());
 }
 
 PeerConnectionSignaling::State PeerConnectionSignaling::GetState() {
@@ -234,7 +241,6 @@ void PeerConnectionSignaling::CreateAnswer_s() {
 
   // Create a MediaSessionOptions object with the sources we want to send.
   cricket::MediaSessionOptions options;
-  options.is_video = true;
   InitMediaSessionOptions(&options, local_streams);
 
   // Use the MediaSessionFactory to create an SDP answer.
@@ -258,7 +264,7 @@ void PeerConnectionSignaling::CreateAnswer_s() {
   UpdateRemoteStreams(message->desc());
 
   // Signal that the new answer is ready to be sent.
-  SignalNewPeerConnectionMessage(answer_message);
+  SignalNewPeerConnectionMessage(answer_message->Serialize());
 
   // Start send the local streams.
   // TODO(perkj): Defer the start of sending local media so the remote peer
@@ -274,6 +280,9 @@ void PeerConnectionSignaling::CreateAnswer_s() {
 void PeerConnectionSignaling::InitMediaSessionOptions(
     cricket::MediaSessionOptions* options,
     StreamCollection* local_streams) {
+  // In order to be able to receive video,
+  // the is_video should always be true even if there are not video tracks.
+  options->is_video = true;
   for (size_t i = 0; i < local_streams->count(); ++i) {
     MediaStream* stream = local_streams->at(i);
     scoped_refptr<MediaStreamTrackList> tracks = stream->tracks();

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

@@ -92,7 +92,7 @@ class PeerConnectionSignaling : public talk_base::MessageHandler {
   void Initialize(const cricket::Candidates& candidates);
 
   // Process a received offer/answer from the remote peer.
-  void ProcessSignalingMessage(PeerConnectionMessage* message,
+  void ProcessSignalingMessage(const std::string& message,
                                StreamCollection* local_streams);
 
   // Creates an offer containing all tracks in local_streams.
@@ -107,7 +107,7 @@ class PeerConnectionSignaling : public talk_base::MessageHandler {
   // New PeerConnectionMessage with an SDP offer/answer is ready to be sent.
   // The listener to this signal is expected to serialize and send the
   // PeerConnectionMessage to the remote peer.
-  sigslot::signal1<PeerConnectionMessage*> SignalNewPeerConnectionMessage;
+  sigslot::signal1<const std::string&> SignalNewPeerConnectionMessage;
 
   // A new remote stream have been discovered.
   sigslot::signal1<MediaStream*> SignalRemoteStreamAdded;

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

@@ -101,15 +101,18 @@ class MockSignalingObserver : public sigslot::has_slots<> {
   }
 
   // New answer ready to be sent.
-  void OnSignalingMessage(PeerConnectionMessage* message) {
+  void OnSignalingMessage(const std::string& smessage) {
     if (remote_peer_) {
-      remote_peer_->ProcessSignalingMessage(message, remote_local_collection_);
+      remote_peer_->ProcessSignalingMessage(smessage, remote_local_collection_);
       // Process posted messages to allow the remote peer to process
       // the message.
       talk_base::Thread::Current()->ProcessMessages(1);
     }
-    if (message->type() != PeerConnectionMessage::kError) {
-      last_message = message;
+    scoped_refptr<PeerConnectionMessage> message;
+    message = PeerConnectionMessage::Create(smessage);
+    if (message.get() != NULL &&
+        message->type() != PeerConnectionMessage::kError) {
+      last_message = smessage;
     }
   }
 
@@ -142,7 +145,7 @@ class MockSignalingObserver : public sigslot::has_slots<> {
 
   virtual ~MockSignalingObserver() {}
 
-  scoped_refptr<PeerConnectionMessage> last_message;
+  std::string last_message;
   int update_session_description_counter_;
 
  private:
@@ -294,8 +297,8 @@ TEST_F(PeerConnectionSignalingTest, Glare) {
   talk_base::Thread::Current()->ProcessMessages(1);
 
   // Peer 2 sends the offer to Peer1 and Peer1 sends its offer to Peer2.
-  ASSERT_TRUE(observer1_->last_message != NULL);
-  ASSERT_TRUE(observer2_->last_message != NULL);
+  ASSERT_TRUE(!observer1_->last_message.empty());
+  ASSERT_TRUE(!observer2_->last_message.empty());
   signaling2_->ProcessSignalingMessage(observer1_->last_message,
                                        local_collection2);
 

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

@@ -29,6 +29,7 @@
 
 #include <stdio.h>
 #include <string>
+#include <vector>
 
 #include "talk/base/json.h"
 #include "talk/base/logging.h"
@@ -50,18 +51,18 @@ static const char* kMessageType[] = {
 static std::vector<Json::Value> ReadValues(const Json::Value& value,
                                            const std::string& key);
 
-static bool BuildContent(
+static void BuildContent(
     const cricket::SessionDescription* sdp,
     const cricket::ContentInfo& content_info,
     const std::vector<cricket::Candidate>& candidates,
     bool video,
     Json::Value* content);
 
-static bool BuildCandidate(const std::vector<cricket::Candidate>& candidates,
+static void BuildCandidate(const std::vector<cricket::Candidate>& candidates,
                            bool video,
                            std::vector<Json::Value>* jcandidates);
 
-static bool BuildRtpMapParams(const cricket::ContentInfo& content_info,
+static void BuildRtpMapParams(const cricket::ContentInfo& content_info,
                               bool video,
                               std::vector<Json::Value>* rtpmap);
 
@@ -69,7 +70,7 @@ static void BuildCrypto(const cricket::ContentInfo& content_info,
                         bool video,
                         std::vector<Json::Value>* cryptos);
 
-static bool BuildTrack(const cricket::SessionDescription* sdp,
+static void BuildTrack(const cricket::SessionDescription* sdp,
                        bool video,
                        std::vector<Json::Value>* track);
 
@@ -107,20 +108,18 @@ static void Append(Json::Value* object,
                    const std::string& key,
                    const std::vector<Json::Value>& values);
 
-bool JsonSerialize(
+std::string JsonSerialize(
     const webrtc::PeerConnectionMessage::PeerConnectionMessageType type,
     int error_code,
     const cricket::SessionDescription* sdp,
-    const std::vector<cricket::Candidate>& candidates,
-    std::string* signaling_message) {
+    const std::vector<cricket::Candidate>& candidates) {
   Json::Value media;
   // TODO(ronghuawu): Replace magic strings.
   Append(&media, "SDP", kMessageType[type]);
 
   if (type == webrtc::PeerConnectionMessage::kError) {
     Append(&media, "error_code", error_code);
-    *signaling_message = Serialize(media);
-    return true;
+    return Serialize(media);
   }
 
   const cricket::ContentInfo* audio_content = GetFirstAudioContent(sdp);
@@ -148,12 +147,10 @@ bool JsonSerialize(
   Append(&media, "TOGETHER", together);
 
   // Now serialize.
-  *signaling_message = Serialize(media);
-
-  return true;
+  return Serialize(media);
 }
 
-bool BuildContent(
+void BuildContent(
     const cricket::SessionDescription* sdp,
     const cricket::ContentInfo& content_info,
     const std::vector<cricket::Candidate>& candidates,
@@ -194,11 +191,9 @@ bool BuildContent(
   std::vector<Json::Value> track;
   BuildTrack(sdp, video, &track);
   Append(content, "track", track);
-
-  return true;
 }
 
-bool BuildRtpMapParams(const cricket::ContentInfo& content_info,
+void BuildRtpMapParams(const cricket::ContentInfo& content_info,
                        bool video,
                        std::vector<Json::Value>* rtpmap) {
   if (!video) {
@@ -238,7 +233,6 @@ bool BuildRtpMapParams(const cricket::ContentInfo& content_info,
       rtpmap->push_back(codec_id);
     }
   }
-  return true;
 }
 
 void BuildCrypto(const cricket::ContentInfo& content_info,
@@ -259,7 +253,7 @@ void BuildCrypto(const cricket::ContentInfo& content_info,
   }
 }
 
-bool BuildCandidate(const std::vector<cricket::Candidate>& candidates,
+void BuildCandidate(const std::vector<cricket::Candidate>& candidates,
                     bool video,
                     std::vector<Json::Value>* jcandidates) {
   std::vector<cricket::Candidate>::const_iterator iter =
@@ -287,10 +281,9 @@ bool BuildCandidate(const std::vector<cricket::Candidate>& candidates,
       jcandidates->push_back(jcandidate);
     }
   }
-  return true;
 }
 
-bool BuildTrack(const cricket::SessionDescription* sdp,
+void BuildTrack(const cricket::SessionDescription* sdp,
                 bool video,
                 std::vector<Json::Value>* tracks) {
   const cricket::ContentInfo* content;
@@ -300,7 +293,7 @@ bool BuildTrack(const cricket::SessionDescription* sdp,
     content = GetFirstAudioContent(sdp);
 
   if (!content)
-    return false;
+    return;
 
   const cricket::MediaContentDescription* desc =
       static_cast<const cricket::MediaContentDescription*>(
@@ -314,7 +307,6 @@ bool BuildTrack(const cricket::SessionDescription* sdp,
     Append(&track, "label", it->description);
     tracks->push_back(track);
   }
-  return true;
 }
 
 std::string Serialize(const Json::Value& value) {

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

@@ -29,6 +29,7 @@
 #define TALK_APP_WEBRTC_WEBRTCJSON_H_
 
 #include <string>
+#include <vector>
 
 #ifdef WEBRTC_RELATIVE_PATH
 #include "json/json.h"
@@ -44,12 +45,11 @@ class SessionDescription;
 
 namespace webrtc {
 
-bool JsonSerialize(
+std::string JsonSerialize(
     const webrtc::PeerConnectionMessage::PeerConnectionMessageType type,
     int error_code,
     const cricket::SessionDescription* sdp,
-    const std::vector<cricket::Candidate>& candidates,
-    std::string* signaling_message);
+    const std::vector<cricket::Candidate>& candidates);
 
 bool JsonDeserialize(
     webrtc::PeerConnectionMessage::PeerConnectionMessageType* type,