Refactoring that removes P2PTransport and DtlsTransport classes.

webrtc/p2p/base/transport.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 25 matching lines @@
 #include "webrtc/p2p/base/p2pconstants.h"
 #include "webrtc/p2p/base/sessiondescription.h"
 #include "webrtc/p2p/base/transportinfo.h"
 #include "webrtc/base/messagequeue.h"
 #include "webrtc/base/rtccertificate.h"
 #include "webrtc/base/sigslot.h"
 #include "webrtc/base/sslstreamadapter.h"
 
 namespace cricket {
 
-class PortAllocator;
-class TransportChannel;
+class TransportChannelImpl;
 class TransportChannelImpl;
 
 typedef std::vector<Candidate> Candidates;
 
+// TODO(deadbeef): Move all of these enums, POD types and utility methods to
+// another header file.
+
 // TODO(deadbeef): Unify with PeerConnectionInterface::IceConnectionState
 // once /talk/ and /webrtc/ are combined, and also switch to ENUM_NAME naming
 // style.
 enum IceConnectionState {
   kIceConnectionConnecting = 0,
   kIceConnectionFailed,
   kIceConnectionConnected,  // Writable, but still checking one or more
                             // connections
   kIceConnectionCompleted,
 };
@@ skipping 173 matching lines @@
         receiving_switching_delay(receiving_switching_delay_ms) {}
 };
 
 bool BadTransportDescription(const std::string& desc, std::string* err_desc);
 
 bool IceCredentialsChanged(const std::string& old_ufrag,
                            const std::string& old_pwd,
                            const std::string& new_ufrag,
                            const std::string& new_pwd);
 
+// If a candidate is not acceptable, returns false and sets error.
+bool VerifyCandidate(const Candidate& candidate, std::string* error);
+bool VerifyCandidates(const Candidates& candidates, std::string* error);
+
+// Helper class used by TransportController that processes
+// TransportDescriptions. A TransportDescription represents the
+// transport-specific properties of an SDP m= section, processed according to
+// JSEP. Each transport consists of DTLS and ICE transport channels for RTP

[pthatcher1, 2016/11/30 01:34:35]

I was hoping we could just delete this class.  But this comment makes me think
that we could instead just move this to the PeerConnection implementation (along
with channel.cc) instead of being a part of the network code and
TransportController.  

For example, we could call this thing a PeerConnectionTransport or a
JsepTransport.  I wish I could call it an RtpTransport, but then it would be
awkward for the data channels to use.  

Then the TransportController could just be in charge of creating, ref counting,
and destroying P2pTransportChannels, DtlsTransportChannels, and (soon)
RawUdpTransportChannels.  That would be much more simple.

[Taylor Brandstetter, 2016/11/30 20:31:39]

Maybe we'll do that eventually. But right now, TransportController is the thing
responsible for applying transport descriptions, and I don't want to change that
in this CL.
JsepTransport, SdpTransport, PeerConnectionTransport, etc. would all be sensible
names. I don't think RtpTransport would. This class doesn't do anything
RTP-specific, and I had already planned to introduce an RtpTransport object that
was completely independent of SDP.

+// (and possibly RTCP, if rtcp-mux isn't used).
 class Transport : public sigslot::has_slots<> {
  public:
-  Transport(const std::string& name, PortAllocator* allocator);
-  virtual ~Transport();
+  // |name| is just used for log statements in order to identify the Transport.
+  // Note that |certificate| is allowed to be null since a remote description
+  // may be set before a local certificate is generated.
+  Transport(const std::string& name,
+            const rtc::scoped_refptr<rtc::RTCCertificate>& certificate);
 
   // Returns the name of this transport.
   const std::string& name() const { return name_; }
 
-  // Returns the port allocator object for this transport.
-  PortAllocator* port_allocator() { return allocator_; }
+  // Add or remove channels that are affected when a local/remote transport
+  // description is set on this transport. Need to add all channels before
+  // setting a transport description.
+  bool AddChannel(TransportChannelImpl* dtls,
+                  TransportChannelImpl* ice,
+                  int component);
+  bool RemoveChannel(int component);
+  bool HasChannels() const;
 
   bool ready_for_remote_candidates() const {
     return local_description_set_ && remote_description_set_;
   }
 
-  void SetIceRole(IceRole role);
-  IceRole ice_role() const { return ice_role_; }
-
-  void SetIceTiebreaker(uint64_t IceTiebreaker) { tiebreaker_ = IceTiebreaker; }
-  uint64_t IceTiebreaker() { return tiebreaker_; }
-
-  void SetIceConfig(const IceConfig& config);
-
   // Must be called before applying local session description.
-  virtual void SetLocalCertificate(
-      const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {}
+  // Needed in order to verify the local fingerprint.
+  void SetLocalCertificate(
+      const rtc::scoped_refptr<rtc::RTCCertificate>& certificate);
 
   // Get a copy of the local certificate provided by SetLocalCertificate.
-  virtual bool GetLocalCertificate(
-      rtc::scoped_refptr<rtc::RTCCertificate>* certificate) {
-    return false;
-  }
+  bool GetLocalCertificate(
+      rtc::scoped_refptr<rtc::RTCCertificate>* certificate) const;
 
-  // Get a copy of the remote certificate in use by the specified channel.
-  std::unique_ptr<rtc::SSLCertificate> GetRemoteSSLCertificate();
-
-  // Create, destroy, and lookup the channels of this type by their components.
-  TransportChannelImpl* CreateChannel(int component);
-
-  TransportChannelImpl* GetChannel(int component);
-
-  bool HasChannel(int component) {
-    return (NULL != GetChannel(component));
-  }
-  bool HasChannels();
-
-  void DestroyChannel(int component);
-
-  // Set the local TransportDescription to be used by TransportChannels.
+  // Set the local TransportDescription to be used by DTLS and ICE channels
+  // that are part of this Transport.
   bool SetLocalTransportDescription(const TransportDescription& description,
                                     ContentAction action,
                                     std::string* error_desc);
 
-  // Set the remote TransportDescription to be used by TransportChannels.
+  // 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);
 
-  // Tells channels to start gathering candidates if necessary.
-  // Should be called after ConnectChannels() has been called at least once,
-  // which will happen in SetLocalTransportDescription.
-  void MaybeStartGathering();
+  void GetSslRole(rtc::SSLRole* ssl_role) const;
 
-  // Resets all of the channels back to their initial state.  They are no
-  // longer connecting.
-  void ResetChannels();
+  bool GetStats(TransportStats* stats) const;
 
-  // Destroys every channel created so far.
-  void DestroyAllChannels();
-
-  bool GetStats(TransportStats* stats);
-
-  // Called when one or more candidates are ready from the remote peer.
-  bool AddRemoteCandidates(const std::vector<Candidate>& candidates,
-                           std::string* error);
-  bool RemoveRemoteCandidates(const std::vector<Candidate>& candidates,
-                              std::string* error);
-
-  virtual bool GetSslRole(rtc::SSLRole* ssl_role) const { return false; }
-
-  // Must be called before channel is starting to connect.
-  virtual bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) {
-    return false;
-  }
-
-  // The current local transport description, for use by derived classes
-  // when performing transport description negotiation, and possibly used
+  // The current local transport description, possibly used
   // by the transport controller.
   const TransportDescription* local_description() const {
     return local_description_.get();
   }
 
-  // The current remote transport description, for use by derived classes
-  // when performing transport description negotiation, and possibly used
+  // The current remote transport description, possibly used
   // by the transport controller.
   const TransportDescription* remote_description() const {
     return remote_description_.get();
   }
 
- protected:
-  // These are called by Create/DestroyChannel above in order to create or
-  // destroy the appropriate type of channel.
-  virtual TransportChannelImpl* CreateTransportChannel(int component) = 0;
-  virtual void DestroyTransportChannel(TransportChannelImpl* channel) = 0;
+  // TODO(deadbeef): The methods below are only public for testing. Should make
+  // them utility functions or objects so they can be tested independently from
+  // this class.
 
-  // Pushes down the transport parameters from the local description, such
-  // as the ICE ufrag and pwd.
-  // Derived classes can override, but must call the base as well.
-  virtual bool ApplyLocalTransportDescription(TransportChannelImpl* channel,
-                                              std::string* error_desc);
+  // Returns false if the certificate's identity does not match the fingerprint,
+  // or either is NULL.
+  bool VerifyCertificateFingerprint(const rtc::RTCCertificate* certificate,
+                                    const rtc::SSLFingerprint* fingerprint,
+                                    std::string* error_desc) const;
 
-  // Pushes down remote ice credentials from the remote description to the
-  // transport channel.
-  virtual bool ApplyRemoteTransportDescription(TransportChannelImpl* ch,
-                                               std::string* error_desc);
+  // Negotiates the SSL role based off the offer and answer as specified by
+  // RFC 4145, section-4.1. Returns false if the SSL role cannot be determined
+  // from the local description and remote description.
+  bool NegotiateRole(ContentAction local_role,
+                     rtc::SSLRole* ssl_role,
+                     std::string* error_desc) const;
+
+ private:
+  struct ChannelPair {
+    // Currently, all ICE-related calls still go through this DTLS channel. But
+    // that will change once we get rid of TransportChannelImpl, and the DTLS
+    // channel interface no longer includes ICE-specific methods.
+    TransportChannelImpl* dtls;
+    TransportChannelImpl* ice;
+  };
+
+  ChannelPair* GetChannel(int component);
 
   // Negotiates the transport parameters based on the current local and remote
   // transport description, such as the ICE role to use, and whether DTLS
   // should be activated.
-  // Derived classes can negotiate their specific parameters here, but must call
-  // the base as well.
-  virtual bool NegotiateTransportDescription(ContentAction local_role,
-                                             std::string* error_desc);
+  //
+  // Called when an answer TransportDescription is applied.
+  bool NegotiateTransportDescription(ContentAction local_role,
+                                     std::string* error_desc);
+
+  // Pushes down the transport parameters from the local description, such
+  // as the ICE ufrag and pwd.
+  bool ApplyLocalTransportDescription(const ChannelPair& channel,
+                                      std::string* error_desc);
+
+  // Pushes down the transport parameters from the remote description to the
+  // transport channel.
+  bool ApplyRemoteTransportDescription(const ChannelPair& channel,
+                                       std::string* error_desc);
 
   // Pushes down the transport parameters obtained via negotiation.
-  // Derived classes can set their specific parameters here, but must call the
-  // base as well.
-  virtual bool ApplyNegotiatedTransportDescription(
-      TransportChannelImpl* channel,
-      std::string* error_desc);
+  bool ApplyNegotiatedTransportDescription(const ChannelPair& channel,
+                                           std::string* error_desc);
 
-  // Returns false if the certificate's identity does not match the fingerprint,
-  // or either is NULL.
-  virtual bool VerifyCertificateFingerprint(
-      const rtc::RTCCertificate* certificate,
-      const rtc::SSLFingerprint* fingerprint,
-      std::string* error_desc) const;
-
-  // Negotiates the SSL role based off the offer and answer as specified by
-  // RFC 4145, section-4.1. Returns false if the SSL role cannot be determined
-  // from the local description and remote description.
-  virtual bool NegotiateRole(ContentAction local_role,
-                             rtc::SSLRole* ssl_role,
-                             std::string* error_desc) const;
-
- private:
-  // If a candidate is not acceptable, returns false and sets error.
-  // Call this before calling OnRemoteCandidates.
-  bool VerifyCandidate(const Candidate& candidate, std::string* error);
-  bool VerifyCandidates(const Candidates& candidates, std::string* error);
-
-  // Candidate component => TransportChannelImpl*
-  typedef std::map<int, TransportChannelImpl*> ChannelMap;
-
-  // Helper function that invokes the given function on every channel.
-  typedef void (TransportChannelImpl::* TransportChannelFunc)();
-  void CallChannels(TransportChannelFunc func);
+  // Candidate component => ChannelPair
+  typedef std::map<int, ChannelPair> ChannelMap;
 
   const std::string name_;
-  PortAllocator* const allocator_;
-  bool channels_destroyed_ = false;
-  IceRole ice_role_ = ICEROLE_UNKNOWN;
-  uint64_t tiebreaker_ = 0;
-  IceMode remote_ice_mode_ = ICEMODE_FULL;
-  IceConfig ice_config_;
+  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;
 
   ChannelMap channels_;

[pthatcher1, 2016/11/30 01:34:35]

While we're in here, I've always thought this map was kind of weird, since it
will only have at most 2 things in it, since we only ever use a maximum of 2
components. It could just be channel_ and rtcp_channel_.  

[Taylor Brandstetter, 2016/11/30 20:31:39]

I can change that in a separate CL, but I don't think it belongs in this CL.

 
   RTC_DISALLOW_COPY_AND_ASSIGN(Transport);
 };
 
 
 }  // namespace cricket
 
 #endif  // WEBRTC_P2P_BASE_TRANSPORT_H_