Revert "Fixes two issues in how we handle OfferToReceiveX for CreateOffer:" because it broke content_browsertests on Android.

This reverts commit r7068.

TBR=kjellander@webrtc.org
BUG=2108

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

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

talk/app/webrtc/mediastreamsignaling.cc

@@ -51,9 +51,9 @@ namespace webrtc {
 using rtc::scoped_ptr;
 using rtc::scoped_refptr;
 
-static bool ParseConstraintsForAnswer(
+static bool ParseConstraints(
     const MediaConstraintsInterface* constraints,
-    cricket::MediaSessionOptions* options) {
+    cricket::MediaSessionOptions* options, bool is_answer) {
   bool value;
   size_t mandatory_constraints_satisfied = 0;
 
@@ -82,7 +82,7 @@ static bool ParseConstraintsForAnswer(
     // kOfferToReceiveVideo defaults to false according to spec. But
     // if it is an answer and video is offered, we should still accept video
     // per default.
-    options->has_video = true;
+    options->has_video |= is_answer;
   }
 
   if (FindConstraint(constraints,
@@ -133,55 +133,6 @@ static bool IsValidOfferToReceiveMedia(int value) {
       (value <= Options::kMaxOfferToReceiveMedia);
 }
 
-// Add the stream and RTP data channel info to |session_options|.
-static void SetStreams(
-    cricket::MediaSessionOptions* session_options,
-    rtc::scoped_refptr<StreamCollection> streams,
-    const MediaStreamSignaling::RtpDataChannels& rtp_data_channels) {
-  session_options->streams.clear();
-  if (streams != NULL) {
-    for (size_t i = 0; i < streams->count(); ++i) {
-      MediaStreamInterface* stream = streams->at(i);
-
-      AudioTrackVector audio_tracks(stream->GetAudioTracks());
-
-      // For each audio track in the stream, add it to the MediaSessionOptions.
-      for (size_t j = 0; j < audio_tracks.size(); ++j) {
-        scoped_refptr<MediaStreamTrackInterface> track(audio_tracks[j]);
-        session_options->AddStream(
-            cricket::MEDIA_TYPE_AUDIO, track->id(), stream->label());
-      }
-
-      VideoTrackVector video_tracks(stream->GetVideoTracks());
-
-      // For each video track in the stream, add it to the MediaSessionOptions.
-      for (size_t j = 0; j < video_tracks.size(); ++j) {
-        scoped_refptr<MediaStreamTrackInterface> track(video_tracks[j]);
-        session_options->AddStream(
-            cricket::MEDIA_TYPE_VIDEO, track->id(), stream->label());
-      }
-    }
-  }
-
-  // Check for data channels.
-  MediaStreamSignaling::RtpDataChannels::const_iterator data_channel_it =
-      rtp_data_channels.begin();
-  for (; data_channel_it != rtp_data_channels.end(); ++data_channel_it) {
-    const DataChannel* channel = data_channel_it->second;
-    if (channel->state() == DataChannel::kConnecting ||
-        channel->state() == DataChannel::kOpen) {
-      // |streamid| and |sync_label| are both set to the DataChannel label
-      // here so they can be signaled the same way as MediaStreams and Tracks.
-      // For MediaStreams, the sync_label is the MediaStream label and the
-      // track label is the same as |streamid|.
-      const std::string& streamid = channel->label();
-      const std::string& sync_label = channel->label();
-      session_options->AddStream(
-          cricket::MEDIA_TYPE_DATA, streamid, sync_label);
-    }
-  }
-}
-
 // Factory class for creating remote MediaStreams and MediaStreamTracks.
 class RemoteMediaStreamFactory {
  public:
@@ -241,6 +192,8 @@ MediaStreamSignaling::MediaStreamSignaling(
                                                           channel_manager)),
       last_allocated_sctp_even_sid_(-2),
       last_allocated_sctp_odd_sid_(-1) {
+  options_.has_video = false;
+  options_.has_audio = false;
 }
 
 MediaStreamSignaling::~MediaStreamSignaling() {
@@ -425,38 +378,40 @@ bool MediaStreamSignaling::GetOptionsForOffer(
     return false;
   }
 
-  session_options->has_audio = false;
+  UpdateSessionOptions();
-  session_options->has_video = false;
-  SetStreams(session_options, local_streams_, rtp_data_channels_);
 
-  // If |offer_to_receive_[audio/video]| is undefined, respect the flags set
+  // |options.has_audio| and |options.has_video| can only change from false to
-  // from SetStreams. Otherwise, overwrite it based on |rtc_options|.
+  // true, but never change from true to false. This is to make sure
-  if (rtc_options.offer_to_receive_audio != RTCOfferAnswerOptions::kUndefined) {
+  // CreateOffer / CreateAnswer doesn't remove a media content
-    session_options->has_audio = rtc_options.offer_to_receive_audio > 0;
+  // description that has been created.
+  if (rtc_options.offer_to_receive_audio > 0) {
+    options_.has_audio = true;
   }
-  if (rtc_options.offer_to_receive_video != RTCOfferAnswerOptions::kUndefined) {
+  if (rtc_options.offer_to_receive_video > 0) {
-    session_options->has_video = rtc_options.offer_to_receive_video > 0;
+    options_.has_video = true;
   }
+  options_.vad_enabled = rtc_options.voice_activity_detection;
+  options_.transport_options.ice_restart = rtc_options.ice_restart;
+  options_.bundle_enabled = rtc_options.use_rtp_mux;
 
-  session_options->vad_enabled = rtc_options.voice_activity_detection;
+  options_.bundle_enabled = EvaluateNeedForBundle(options_);
-  session_options->transport_options.ice_restart = rtc_options.ice_restart;
+  *session_options = options_;
-  session_options->bundle_enabled = rtc_options.use_rtp_mux;
-
-  session_options->bundle_enabled = EvaluateNeedForBundle(*session_options);
   return true;
 }
 
 bool MediaStreamSignaling::GetOptionsForAnswer(
     const MediaConstraintsInterface* constraints,
     cricket::MediaSessionOptions* options) {
-  options->has_audio = false;
+  UpdateSessionOptions();
-  options->has_video = false;
-  SetStreams(options, local_streams_, rtp_data_channels_);
 
-  if (!ParseConstraintsForAnswer(constraints, options)) {
+  // Copy the |options_| to not let the flag MediaSessionOptions::has_audio and
+  // MediaSessionOptions::has_video affect subsequent offers.
+  cricket::MediaSessionOptions current_options = options_;
+  if (!ParseConstraints(constraints, &current_options, true)) {
     return false;
   }
-  options->bundle_enabled = EvaluateNeedForBundle(*options);
+  current_options.bundle_enabled = EvaluateNeedForBundle(current_options);
+  *options = current_options;
   return true;
 }
 
@@ -591,6 +546,54 @@ void MediaStreamSignaling::OnDataChannelClose() {
   }
 }
 
+void MediaStreamSignaling::UpdateSessionOptions() {
+  options_.streams.clear();
+  if (local_streams_ != NULL) {
+    for (size_t i = 0; i < local_streams_->count(); ++i) {
+      MediaStreamInterface* stream = local_streams_->at(i);
+
+      AudioTrackVector audio_tracks(stream->GetAudioTracks());
+      if (!audio_tracks.empty()) {
+        options_.has_audio = true;
+      }
+
+      // For each audio track in the stream, add it to the MediaSessionOptions.
+      for (size_t j = 0; j < audio_tracks.size(); ++j) {
+        scoped_refptr<MediaStreamTrackInterface> track(audio_tracks[j]);
+        options_.AddStream(cricket::MEDIA_TYPE_AUDIO, track->id(),
+                           stream->label());
+      }
+
+      VideoTrackVector video_tracks(stream->GetVideoTracks());
+      if (!video_tracks.empty()) {
+        options_.has_video = true;
+      }
+      // For each video track in the stream, add it to the MediaSessionOptions.
+      for (size_t j = 0; j < video_tracks.size(); ++j) {
+        scoped_refptr<MediaStreamTrackInterface> track(video_tracks[j]);
+        options_.AddStream(cricket::MEDIA_TYPE_VIDEO, track->id(),
+                           stream->label());
+      }
+    }
+  }
+
+  // Check for data channels.
+  RtpDataChannels::const_iterator data_channel_it = rtp_data_channels_.begin();
+  for (; data_channel_it != rtp_data_channels_.end(); ++data_channel_it) {
+    const DataChannel* channel = data_channel_it->second;
+    if (channel->state() == DataChannel::kConnecting ||
+        channel->state() == DataChannel::kOpen) {
+      // |streamid| and |sync_label| are both set to the DataChannel label
+      // here so they can be signaled the same way as MediaStreams and Tracks.
+      // For MediaStreams, the sync_label is the MediaStream label and the
+      // track label is the same as |streamid|.
+      const std::string& streamid = channel->label();
+      const std::string& sync_label = channel->label();
+      options_.AddStream(cricket::MEDIA_TYPE_DATA, streamid, sync_label);
+    }
+  }
+}
+
 void MediaStreamSignaling::UpdateRemoteStreamsList(
     const cricket::StreamParamsVec& streams,
     cricket::MediaType media_type,

talk/app/webrtc/mediastreamsignaling.h

@@ -160,9 +160,6 @@ class MediaStreamSignalingObserver {
 
 class MediaStreamSignaling : public sigslot::has_slots<> {
  public:
-  typedef std::map<std::string, rtc::scoped_refptr<DataChannel> >
-      RtpDataChannels;
-
   MediaStreamSignaling(rtc::Thread* signaling_thread,
                        MediaStreamSignalingObserver* stream_observer,
                        cricket::ChannelManager* channel_manager);
@@ -292,6 +289,8 @@ class MediaStreamSignaling : public sigslot::has_slots<> {
   };
   typedef std::vector<TrackInfo> TrackInfos;
 
+  void UpdateSessionOptions();
+
   // Makes sure a MediaStream Track is created for each StreamParam in
   // |streams|. |media_type| is the type of the |streams| and can be either
   // audio or video.
@@ -379,6 +378,7 @@ class MediaStreamSignaling : public sigslot::has_slots<> {
   RemotePeerInfo remote_info_;
   rtc::Thread* signaling_thread_;
   DataChannelFactory* data_channel_factory_;
+  cricket::MediaSessionOptions options_;
   MediaStreamSignalingObserver* stream_observer_;
   rtc::scoped_refptr<StreamCollection> local_streams_;
   rtc::scoped_refptr<StreamCollection> remote_streams_;
@@ -392,6 +392,8 @@ class MediaStreamSignaling : public sigslot::has_slots<> {
   int last_allocated_sctp_even_sid_;
   int last_allocated_sctp_odd_sid_;
 
+  typedef std::map<std::string, rtc::scoped_refptr<DataChannel> >
+      RtpDataChannels;
   typedef std::vector<rtc::scoped_refptr<DataChannel> > SctpDataChannels;
 
   RtpDataChannels rtp_data_channels_;

talk/app/webrtc/mediastreamsignaling_unittest.cc

@@ -782,10 +782,8 @@ TEST_F(MediaStreamSignalingTest, MediaConstraintsInAnswer) {
   RTCOfferAnswerOptions default_rtc_options;
   EXPECT_TRUE(signaling_->GetOptionsForOffer(default_rtc_options,
                                              &updated_offer_options));
-  // By default, |has_audio| or |has_video| are false if there is no media
+  EXPECT_TRUE(updated_offer_options.has_audio);
-  // track.
+  EXPECT_TRUE(updated_offer_options.has_video);
-  EXPECT_FALSE(updated_offer_options.has_audio);
-  EXPECT_FALSE(updated_offer_options.has_video);
 }
 
 // This test verifies that the remote MediaStreams corresponding to a received

talk/app/webrtc/peerconnection.cc

@@ -508,6 +508,10 @@ void PeerConnection::CreateOffer(CreateSessionDescriptionObserver* observer,
     return;
   }
   RTCOfferAnswerOptions options;
+  // Defaults to receiving audio and not receiving video.
+  options.offer_to_receive_audio =
+      RTCOfferAnswerOptions::kOfferToReceiveMediaTrue;
+  options.offer_to_receive_video = 0;
 
   bool value;
   size_t mandatory_constraints = 0;

talk/app/webrtc/webrtcsession_unittest.cc

@@ -512,7 +512,7 @@ class WebRtcSessionTest : public testing::Test {
   }
 
   void VerifyAnswerFromNonCryptoOffer() {
-    // Create an SDP without Crypto.
+    // Create a SDP without Crypto.
     cricket::MediaSessionOptions options;
     options.has_video = true;
     JsepSessionDescription* offer(
@@ -1272,13 +1272,14 @@ TEST_F(WebRtcSessionTest, TestCreateSdesOfferReceiveSdesAnswer) {
             rtc::FromString<uint64>(offer->session_version()));
 
   SetLocalDescriptionWithoutError(offer);
-  EXPECT_EQ(0u, video_channel_->send_streams().size());
-  EXPECT_EQ(0u, voice_channel_->send_streams().size());
 
   mediastream_signaling_.SendAudioVideoStream2();
   answer = CreateRemoteAnswer(session_->local_description());
   SetRemoteDescriptionWithoutError(answer);
 
+  EXPECT_EQ(0u, video_channel_->send_streams().size());
+  EXPECT_EQ(0u, voice_channel_->send_streams().size());
+
   // Make sure the receive streams have not changed.
   ASSERT_EQ(1u, video_channel_->recv_streams().size());
   EXPECT_TRUE(kVideoTrack2 == video_channel_->recv_streams()[0].id);
@@ -2052,21 +2053,13 @@ TEST_F(WebRtcSessionTest, CreateOfferWithConstraints) {
 
   const cricket::ContentInfo* content =
       cricket::GetFirstAudioContent(offer->description());
-  EXPECT_TRUE(content != NULL);
 
+  EXPECT_TRUE(content != NULL);
   content = cricket::GetFirstVideoContent(offer->description());
   EXPECT_TRUE(content != NULL);
 
-  // Sets constraints to false and verifies that audio/video contents are
+  // TODO(perkj): Should the direction be set to SEND_ONLY if
-  // removed.
+  // The constraints is set to not receive audio or video but a track is added?
-  options.offer_to_receive_audio = 0;
-  options.offer_to_receive_video = 0;
-  offer.reset(CreateOffer(options));
-
-  content = cricket::GetFirstAudioContent(offer->description());
-  EXPECT_TRUE(content == NULL);
-  content = cricket::GetFirstVideoContent(offer->description());
-  EXPECT_TRUE(content == NULL);
 }
 
 // Test that an answer can not be created if the last remote description is not
@@ -2105,7 +2098,8 @@ TEST_F(WebRtcSessionTest, CreateAudioAnswerWithoutConstraintsOrStreams) {
   Init(NULL);
   // Create a remote offer with audio only.
   cricket::MediaSessionOptions options;
-
+  options.has_audio = true;
+  options.has_video = false;
   rtc::scoped_ptr<JsepSessionDescription> offer(
       CreateRemoteOffer(options));
   ASSERT_TRUE(cricket::GetFirstVideoContent(offer->description()) == NULL);
@@ -2241,6 +2235,7 @@ TEST_F(WebRtcSessionTest, TestAVOfferWithAudioOnlyAnswer) {
   SessionDescriptionInterface* offer = CreateOffer();
 
   cricket::MediaSessionOptions options;
+  options.has_video = false;
   SessionDescriptionInterface* answer = CreateRemoteAnswer(offer, options);
 
   // SetLocalDescription and SetRemoteDescriptions takes ownership of offer
@@ -2953,6 +2948,7 @@ TEST_F(WebRtcSessionTest, TestCryptoAfterSetLocalDescriptionWithDisabled) {
 TEST_F(WebRtcSessionTest, TestCreateAnswerWithNewUfragAndPassword) {
   Init(NULL);
   cricket::MediaSessionOptions options;
+  options.has_audio = true;
   options.has_video = true;
   rtc::scoped_ptr<JsepSessionDescription> offer(
       CreateRemoteOffer(options));
@@ -2984,6 +2980,7 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithNewUfragAndPassword) {
 TEST_F(WebRtcSessionTest, TestCreateAnswerWithOldUfragAndPassword) {
   Init(NULL);
   cricket::MediaSessionOptions options;
+  options.has_audio = true;
   options.has_video = true;
   rtc::scoped_ptr<JsepSessionDescription> offer(
       CreateRemoteOffer(options));
@@ -3049,6 +3046,7 @@ TEST_F(WebRtcSessionTest, TestIceStatesBundle) {
 TEST_F(WebRtcSessionTest, SetSdpFailedOnSessionError) {
   Init(NULL);
   cricket::MediaSessionOptions options;
+  options.has_audio = true;
   options.has_video = true;
 
   cricket::BaseSession::Error error_code = cricket::BaseSession::ERROR_CONTENT;

talk/session/media/mediasession.cc

@@ -1170,28 +1170,28 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer(
   if (current_description) {
     ContentInfos::const_iterator it = current_description->contents().begin();
     for (; it != current_description->contents().end(); ++it) {
-      if (IsMediaContentOfType(&*it, MEDIA_TYPE_AUDIO)) {
+      if (IsMediaContentOfType(&*it, MEDIA_TYPE_AUDIO) && options.has_audio) {
         if (!AddAudioContentForOffer(options, current_description,
                                      audio_rtp_extensions, audio_codecs,
                                      &current_streams, offer.get())) {
           return NULL;
         }
         audio_added = true;
-      } else if (IsMediaContentOfType(&*it, MEDIA_TYPE_VIDEO)) {
+      } else if (IsMediaContentOfType(&*it, MEDIA_TYPE_VIDEO) &&
+                 options.has_video) {
         if (!AddVideoContentForOffer(options, current_description,
                                      video_rtp_extensions, video_codecs,
                                      &current_streams, offer.get())) {
           return NULL;
         }
         video_added = true;
-      } else if (IsMediaContentOfType(&*it, MEDIA_TYPE_DATA)) {
+      } else if (IsMediaContentOfType(&*it, MEDIA_TYPE_DATA) &&
+                 options.has_data()) {
         if (!AddDataContentForOffer(options, current_description, &data_codecs,
                                     &current_streams, offer.get())) {
           return NULL;
         }
         data_added = true;
-      } else {
-        ASSERT(false);
       }
     }
   }
@@ -1459,7 +1459,6 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer(
 
   bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED);
   SetMediaProtocol(secure_transport, audio.get());
-
   desc->AddContent(CN_AUDIO, NS_JINGLE_RTP, audio.release());
   if (!AddTransportOffer(CN_AUDIO, options.transport_options,
                          current_description, desc)) {