Discard old-generation candidates when ICE restarts

talk/app/webrtc/webrtcsession.cc

 /*
  * libjingle
  * Copyright 2012 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 474 matching lines @@
   bool Get() const {
     return ice_restart_;
   }
 
   void Reset() {
     if (ice_restart_) {
       ice_restart_ = false;
     }
   }
 
+  // This method has two purposes: 1. Return whether |new_desc| requests
+  // an ICE restart (i.e., new ufrag/pwd). 2. If it requests an ICE restart
+  // and it is an OFFER, remember this in |ice_restart_| so that the next
+  // Local Answer will be created with new ufrag and pwd.
   bool CheckForRemoteIceRestart(const SessionDescriptionInterface* old_desc,
                                 const SessionDescriptionInterface* new_desc) {
-    if (!old_desc || new_desc->type() != SessionDescriptionInterface::kOffer) {
+    if (!old_desc) {
       return false;
     }
+    bool ice_restart = false;
     const SessionDescription* new_sd = new_desc->description();
     const SessionDescription* old_sd = old_desc->description();
     const ContentInfos& contents = new_sd->contents();
     for (size_t index = 0; index < contents.size(); ++index) {
       const ContentInfo* cinfo = &contents[index];
       if (cinfo->rejected) {
         continue;
       }
       // If the content isn't rejected, check if ufrag and password has
       // changed.
       const cricket::TransportDescription* new_transport_desc =
           new_sd->GetTransportDescriptionByName(cinfo->name);
       const cricket::TransportDescription* old_transport_desc =
           old_sd->GetTransportDescriptionByName(cinfo->name);
       if (!new_transport_desc || !old_transport_desc) {
         // No transport description exist. This is not an ice restart.
         continue;
       }
       if (cricket::IceCredentialsChanged(old_transport_desc->ice_ufrag,
                                          old_transport_desc->ice_pwd,
                                          new_transport_desc->ice_ufrag,
                                          new_transport_desc->ice_pwd)) {
         LOG(LS_INFO) << "Remote peer request ice restart.";
-        ice_restart_ = true;
-        return true;
+        ice_restart = true;
+        break;

[pthatcher1, 2015/12/11 01:49:00]

I think it would be more clear to put this here:

if (new_desc->type() == SessionDescriptionInterface::kOffer) {
  ice_restart_ = true;
}
return true;


(And then return false at the end).

[honghaiz3, 2015/12/11 18:46:10]

Done.

       }
     }
-    return false;
+    if (ice_restart &&
+        new_desc->type() == SessionDescriptionInterface::kOffer) {
+      ice_restart_ = true;
+    }
+    return ice_restart;
   }
 
  private:
   bool ice_restart_;
 };
 
 WebRtcSession::WebRtcSession(webrtc::MediaControllerInterface* media_controller,
                              rtc::Thread* signaling_thread,
                              rtc::Thread* worker_thread,
                              cricket::PortAllocator* port_allocator)
@@ skipping 1650 matching lines @@
   }
 }
 
 void WebRtcSession::OnSentPacket_w(cricket::TransportChannel* channel,
                                    const rtc::SentPacket& sent_packet) {
   RTC_DCHECK(worker_thread()->IsCurrent());
   media_controller_->call_w()->OnSentPacket(sent_packet);
 }
 
 }  // namespace webrtc