Refactoring that removes P2PTransport and DtlsTransport classes.

webrtc/p2p/base/transportcontroller.h

 /*
  *  Copyright 2015 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.
  */
 
 #ifndef WEBRTC_P2P_BASE_TRANSPORTCONTROLLER_H_
 #define WEBRTC_P2P_BASE_TRANSPORTCONTROLLER_H_
 
 #include <map>
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "webrtc/base/asyncinvoker.h"
 #include "webrtc/base/sigslot.h"
 #include "webrtc/base/sslstreamadapter.h"
 #include "webrtc/p2p/base/candidate.h"
+#include "webrtc/p2p/base/dtlstransportchannel.h"
+#include "webrtc/p2p/base/p2ptransportchannel.h"
 #include "webrtc/p2p/base/transport.h"
 
 namespace rtc {
 class Thread;
 class PacketTransportInterface;
 }
 namespace webrtc {
 class MetricsObserverInterface;
 }
 
@@ skipping 22 matching lines @@
   PortAllocator* port_allocator() const { return port_allocator_; }
 
   // Can only be set before transports are created.
   // TODO(deadbeef): Make this an argument to the constructor once BaseSession
   // and WebRtcSession are combined
   bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version);
 
   void SetIceConfig(const IceConfig& config);
   void SetIceRole(IceRole ice_role);
 
-  bool GetSslRole(const std::string& transport_name, rtc::SSLRole* role);
+  bool GetSslRole(const std::string& transport_name, rtc::SSLRole* role) const;
 
   // Specifies the identity to use in this session.
   // Can only be called once.
   bool SetLocalCertificate(
       const rtc::scoped_refptr<rtc::RTCCertificate>& certificate);
   bool GetLocalCertificate(
       const std::string& transport_name,
-      rtc::scoped_refptr<rtc::RTCCertificate>* certificate);
-  // Caller owns returned certificate
+      rtc::scoped_refptr<rtc::RTCCertificate>* certificate) const;
+  // Caller owns returned certificate. This method mainly exists for stats
+  // reporting.
   std::unique_ptr<rtc::SSLCertificate> GetRemoteSSLCertificate(
-      const std::string& transport_name);
+      const std::string& transport_name) const;
   bool SetLocalTransportDescription(const std::string& transport_name,
                                     const TransportDescription& tdesc,
                                     ContentAction action,
                                     std::string* err);
   bool SetRemoteTransportDescription(const std::string& transport_name,
                                      const TransportDescription& tdesc,
                                      ContentAction action,
                                      std::string* err);
   // Start gathering candidates for any new transports, or transports doing an
   // ICE restart.
   void MaybeStartGathering();
   bool AddRemoteCandidates(const std::string& transport_name,
                            const Candidates& candidates,
                            std::string* err);
   bool RemoveRemoteCandidates(const Candidates& candidates, std::string* err);
-  bool ReadyForRemoteCandidates(const std::string& transport_name);
-  bool GetStats(const std::string& transport_name, TransportStats* stats);
+  bool ReadyForRemoteCandidates(const std::string& transport_name) const;
+  bool GetStats(const std::string& transport_name, TransportStats* stats) const;
+  void SetMetricsObserver(webrtc::MetricsObserverInterface* metrics_observer);
 
   // Creates a channel if it doesn't exist. Otherwise, increments a reference
   // count and returns an existing channel.
   virtual TransportChannel* CreateTransportChannel_n(
       const std::string& transport_name,
       int component);
 
   // Decrements a channel's reference count, and destroys the channel if
   // nothing is referencing it.
   virtual void DestroyTransportChannel_n(const std::string& transport_name,
                                          int component);
 
   void use_quic() { quic_ = true; }
   bool quic() const { return quic_; }
 
+  // TODO(deadbeef): Remove all for_testing methods!
+  const rtc::scoped_refptr<rtc::RTCCertificate>& certificate_for_testing()
+      const {
+    return certificate_;
+  }
+  std::vector<std::string> transport_names_for_testing();
+  std::vector<TransportChannelImpl*> channels_for_testing();
+  TransportChannelImpl* get_channel_for_testing(
+      const std::string& transport_name,
+      int component);
+
   // All of these signals are fired on the signalling thread.
 
   // If any transport failed => failed,
   // Else if all completed => completed,
   // Else if all connected => connected,
   // Else => connecting
   sigslot::signal1<IceConnectionState> SignalConnectionState;
 
   // Receiving if any transport is receiving
   sigslot::signal1<bool> SignalReceiving;
 
   // If all transports done gathering => complete,
   // Else if any are gathering => gathering,
   // Else => new
   sigslot::signal1<IceGatheringState> SignalGatheringState;
 
   // (transport_name, candidates)
   sigslot::signal2<const std::string&, const Candidates&>
       SignalCandidatesGathered;
 
   sigslot::signal1<const Candidates&> SignalCandidatesRemoved;
 
-  // for unit test
-  const rtc::scoped_refptr<rtc::RTCCertificate>& certificate_for_testing();
-
   sigslot::signal1<rtc::SSLHandshakeError> SignalDtlsHandshakeError;
 
-  void SetMetricsObserver(webrtc::MetricsObserverInterface* metrics_observer);
-
  protected:
-  // Protected and virtual so we can override it in unit tests.
-  virtual Transport* CreateTransport_n(const std::string& transport_name);
-
-  // For unit tests
-  const std::map<std::string, Transport*>& transports() { return transports_; }
-  Transport* GetTransport_n(const std::string& transport_name);
+  // TODO(deadbeef): Get rid of these virtual methods. Used by
+  // FakeTransportController currently, but FakeTransportController shouldn't
+  // even be functioning by subclassing TransportController.
+  virtual TransportChannelImpl* CreateIceTransportChannel_n(
+      const std::string& transport_name,
+      int component);
+  virtual TransportChannelImpl* CreateDtlsTransportChannel_n(
+      const std::string& transport_name,
+      int component,
+      TransportChannelImpl* ice);

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

Why don't we just change TransportController and the BaseChannel to call these
instead of CreateTransportChannel_n?

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

TransportController already does call these. As for BaseChannel, I'd prefer to
address that in a follow-up CL.

 
  private:
   void OnMessage(rtc::Message* pmsg) override;
 
-  // It's the Transport that's currently responsible for creating/destroying
-  // channels, but the TransportController keeps track of how many external
-  // objects (BaseChannels) reference each channel.
+  // This structure groups the DTLS and ICE channels, and helps keep track of
+  // how many external objects (BaseChannels) reference each channel.
   struct RefCountedChannel {

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

Would it make more sense to have a RefCounted<T> with a
RefCounted<P2pTransportChannel> and RefCounted<DtlsTransportChannel>?

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

Maybe. But again, I don't think it's necessary for this CL.

-    RefCountedChannel() : impl_(nullptr), ref_(0) {}
-    explicit RefCountedChannel(TransportChannelImpl* impl)
-        : impl_(impl), ref_(0) {}
+    RefCountedChannel() = default;
+    // TODO(deadbeef): Change the types of |dtls| and |ice| to
+    // DtlsTransportChannelWrapper and P2PTransportChannelWrapper,
+    // once TransportChannelImpl is removed.
+    explicit RefCountedChannel(TransportChannelImpl* dtls,
+                               TransportChannelImpl* ice)
+        : ice_(ice), dtls_(dtls), ref_(0) {}
 
     void AddRef() { ++ref_; }
     void DecRef() {
       ASSERT(ref_ > 0);
       --ref_;
     }
     int ref() const { return ref_; }
 
-    TransportChannelImpl* get() const { return impl_; }
-    TransportChannelImpl* operator->() const { return impl_; }
+    // 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.
+    const TransportChannelImpl* dtls() const { return dtls_.get(); }
+    TransportChannelImpl* dtls() { return dtls_.get(); }
+    const TransportChannelImpl* ice() const { return ice_.get(); }
+    TransportChannelImpl* ice() { return ice_.get(); }
 
    private:
-    TransportChannelImpl* impl_;
-    int ref_;
+    std::unique_ptr<TransportChannelImpl> ice_;
+    std::unique_ptr<TransportChannelImpl> dtls_;
+    int ref_ = 0;
   };
 
-  std::vector<RefCountedChannel>::iterator FindChannel_n(
+  // Helper functions to get a channel or transport, or iterator to it (in case
+  // it needs to be erased).
+  std::vector<RefCountedChannel>::iterator GetChannelIterator_n(
       const std::string& transport_name,
       int component);
+  std::vector<RefCountedChannel>::const_iterator GetChannelIterator_n(
+      const std::string& transport_name,
+      int component) const;
+  const Transport* GetTransport_n(const std::string& transport_name) const;
+  Transport* GetTransport_n(const std::string& transport_name);
+  const RefCountedChannel* GetChannel_n(const std::string& transport_name,
+                                        int component) const;
+  RefCountedChannel* GetChannel_n(const std::string& transport_name,
+                                  int component);
 
   Transport* GetOrCreateTransport_n(const std::string& transport_name);
-  void DestroyTransport_n(const std::string& transport_name);
-  void DestroyAllTransports_n();
+  void DestroyAllChannels_n();
 
   bool SetSslMaxProtocolVersion_n(rtc::SSLProtocolVersion version);
   void SetIceConfig_n(const IceConfig& config);
   void SetIceRole_n(IceRole ice_role);
-  bool GetSslRole_n(const std::string& transport_name, rtc::SSLRole* role);
+  bool GetSslRole_n(const std::string& transport_name,
+                    rtc::SSLRole* role) const;
   bool SetLocalCertificate_n(
       const rtc::scoped_refptr<rtc::RTCCertificate>& certificate);
   bool GetLocalCertificate_n(
       const std::string& transport_name,
-      rtc::scoped_refptr<rtc::RTCCertificate>* certificate);
+      rtc::scoped_refptr<rtc::RTCCertificate>* certificate) const;
   std::unique_ptr<rtc::SSLCertificate> GetRemoteSSLCertificate_n(
-      const std::string& transport_name);
+      const std::string& transport_name) const;
   bool SetLocalTransportDescription_n(const std::string& transport_name,
                                       const TransportDescription& tdesc,
                                       ContentAction action,
                                       std::string* err);
   bool SetRemoteTransportDescription_n(const std::string& transport_name,
                                        const TransportDescription& tdesc,
                                        ContentAction action,
                                        std::string* err);
   void MaybeStartGathering_n();
   bool AddRemoteCandidates_n(const std::string& transport_name,
                              const Candidates& candidates,
                              std::string* err);
   bool RemoveRemoteCandidates_n(const Candidates& candidates, std::string* err);
-  bool ReadyForRemoteCandidates_n(const std::string& transport_name);
-  bool GetStats_n(const std::string& transport_name, TransportStats* stats);
+  bool ReadyForRemoteCandidates_n(const std::string& transport_name) const;
+  bool GetStats_n(const std::string& transport_name,
+                  TransportStats* stats) const;
+  void SetMetricsObserver_n(webrtc::MetricsObserverInterface* metrics_observer);
 
   // Handlers for signals from Transport.
   void OnChannelWritableState_n(rtc::PacketTransportInterface* transport);
   void OnChannelReceivingState_n(rtc::PacketTransportInterface* transport);
   void OnChannelGatheringState_n(TransportChannelImpl* channel);
   void OnChannelCandidateGathered_n(TransportChannelImpl* channel,
                                     const Candidate& candidate);
   void OnChannelCandidatesRemoved(const Candidates& candidates);
   void OnChannelCandidatesRemoved_n(TransportChannelImpl* channel,
                                     const Candidates& candidates);
   void OnChannelRoleConflict_n(TransportChannelImpl* channel);
   void OnChannelStateChanged_n(TransportChannelImpl* channel);
 
   void UpdateAggregateStates_n();
 
   void OnDtlsHandshakeError(rtc::SSLHandshakeError error);
 
   rtc::Thread* const signaling_thread_ = nullptr;
   rtc::Thread* const network_thread_ = nullptr;
-  typedef std::map<std::string, Transport*> TransportMap;
-  TransportMap transports_;
+  PortAllocator* const port_allocator_ = nullptr;
 
+  std::map<std::string, std::unique_ptr<Transport>> transports_;
   std::vector<RefCountedChannel> channels_;
 
-  PortAllocator* const port_allocator_ = nullptr;
-  rtc::SSLProtocolVersion ssl_max_version_ = rtc::SSL_PROTOCOL_DTLS_12;
-
   // Aggregate state for TransportChannelImpls.
   IceConnectionState connection_state_ = kIceConnectionConnecting;
   bool receiving_ = false;
   IceGatheringState gathering_state_ = kIceGatheringNew;
 
-  // TODO(deadbeef): Move the fields below down to the transports themselves
   IceConfig ice_config_;
   IceRole ice_role_ = ICEROLE_CONTROLLING;
   bool redetermine_role_on_ice_restart_;
   uint64_t ice_tiebreaker_ = rtc::CreateRandomId64();
+  rtc::SSLProtocolVersion ssl_max_version_ = rtc::SSL_PROTOCOL_DTLS_12;
   rtc::scoped_refptr<rtc::RTCCertificate> certificate_;
   rtc::AsyncInvoker invoker_;
   // True if QUIC is used instead of DTLS.
   bool quic_ = false;
 
   webrtc::MetricsObserverInterface* metrics_observer_ = nullptr;
 };
 
 }  // namespace cricket
 
 #endif  // WEBRTC_P2P_BASE_TRANSPORTCONTROLLER_H_