Implement the "needs-ice-restart" logic for SetConfiguration.

webrtc/p2p/base/jseptransport.h

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 270 matching lines @@
   bool SetLocalTransportDescription(const TransportDescription& description,
                                     ContentAction action,
                                     std::string* error_desc);
 
   // Set the remote TransportDescription to be used by DTLS and ICE channels
   // that are part of this Transport.
   bool SetRemoteTransportDescription(const TransportDescription& description,
                                      ContentAction action,
                                      std::string* error_desc);
 
+  // Set the "needs-ice-restart" flag as described in JSEP.
+  // This and the below method can be called safely from any thread as long as
+  // SetXTransportDescription is not in progress.

[pthatcher1, 2016/12/10 02:26:31]

And if SetXTransportDescription is in progress, what happens?

[Taylor Brandstetter, 2016/12/10 19:00:35]

Undefined behavior. So don't do that.

+  void SetNeedsIceRestartFlag();
+  // Returns true if the ICE restart flag above was set, and no ICE restart has
+  // occurred yet for this transport.

[pthatcher1, 2016/12/10 02:26:31]

"no ICE restart has occured", meaning that an ICE restart has been proposed (by
local or remote) or finalized (by both)?  This may not matter now, but it will
when we add rollback.

[Taylor Brandstetter, 2016/12/10 19:00:35]

I clarified the comments. As for rollback, there are tons of things we'll need
to figure out when we start supporting that. My understanding is that a rollback
reverts *all* state unless otherwise stated (such as the addTrack weirdness),
which would include the needs-ice-restart flag.

+  bool NeedsIceRestart() const;
+
   void GetSslRole(rtc::SSLRole* ssl_role) const;
 
   // TODO(deadbeef): Make this const. See comment in transportcontroller.h.
   bool GetStats(TransportStats* stats);
 
   // The current local transport description, possibly used
   // by the transport controller.
   const TransportDescription* local_description() const {
     return local_description_.get();
   }
@@ skipping 40 matching lines @@
   // Pushes down the transport parameters from the remote description to the
   // transport channel.
   bool ApplyRemoteTransportDescription(TransportChannelImpl* channel,
                                        std::string* error_desc);
 
   // Pushes down the transport parameters obtained via negotiation.
   bool ApplyNegotiatedTransportDescription(TransportChannelImpl* channel,
                                            std::string* error_desc);
 
   const std::string mid_;
+  // needs-ice-restart bit as described in JSEP.

[pthatcher1, 2016/12/10 02:26:31]

A summary of what it means would be nice, even if just one sentence.

[Taylor Brandstetter, 2016/12/10 19:00:35]

Doesn't need to be here, since this is a private variable where the comment is
just a small reminder about what it's for. But I added more comments for
SetNeedsIceRestartFlag.

+  bool needs_ice_restart_ = false;
   rtc::scoped_refptr<rtc::RTCCertificate> certificate_;
   rtc::SSLRole secure_role_ = rtc::SSL_CLIENT;
   std::unique_ptr<rtc::SSLFingerprint> remote_fingerprint_;
   std::unique_ptr<TransportDescription> local_description_;
   std::unique_ptr<TransportDescription> remote_description_;
   bool local_description_set_ = false;
   bool remote_description_set_ = false;
 
   // Candidate component => DTLS channel
   std::map<int, TransportChannelImpl*> channels_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransport);
 };
 
 }  // namespace cricket
 
 #endif  // WEBRTC_P2P_BASE_JSEPTRANSPORT_H_