Make P2PTransportChannel inherit from IceTransportInternal.

webrtc/p2p/base/icetransportinternal.h

 /*
  *  Copyright 2016 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_ICETRANSPORTINTERNAL_H_
 #define WEBRTC_P2P_BASE_ICETRANSPORTINTERNAL_H_
 
 #include <string>
 
 #include "webrtc/p2p/base/candidate.h"
 #include "webrtc/p2p/base/candidatepairinterface.h"
 #include "webrtc/p2p/base/jseptransport.h"
 #include "webrtc/p2p/base/packettransportinterface.h"
 #include "webrtc/p2p/base/transportdescription.h"
 
 namespace webrtc {
 class MetricsObserverInterface;
 }
 
 namespace cricket {
 
 enum class TransportState {

[Taylor Brandstetter, 2016/12/20 03:37:26]

Sorry for not catching this earlier, but: if this is only used to represent an
ICE state, can we call it IceTransportState? And leave a TODO to replace it with
IceConnectionState later
(https://cs.chromium.org/chromium/src/third_party/webrtc/api/peerconnectionint...)

[Zhi Huang, 2016/12/20 20:17:43]

Done.

   STATE_INIT,
   STATE_CONNECTING,  // Will enter this state once a connection is created
   STATE_COMPLETED,
-  STATE_FAILEDs
+  STATE_FAILED
 };
 
 // TODO(zhihuang): Remove this once it's no longer used in
 // remoting/protocol/libjingle_transport_factory.cc
 enum IceProtocolType {
   ICEPROTO_RFC5245  // Standard RFC 5245 version of ICE.
 };
 
 // IceTransportInternal is an internal abstract class that does ICE.
 // Once the public interface is supported,
 // (https://www.w3.org/TR/webrtc/#rtcicetransport-interface)
 // the IceTransportInterface will be split from this class.
 class IceTransportInternal : public rtc::PacketTransportInterface {
  public:
-  virtual ~IceTransportInternal();
+  virtual ~IceTransportInternal(){};

[Taylor Brandstetter, 2016/12/20 03:37:26]

Does this destructor need a body?

[Zhi Huang, 2016/12/20 20:17:43]

Done.

 
   virtual TransportState GetState() const = 0;
 
   virtual const std::string& transport_name() const = 0;
 
   virtual int component() const = 0;
 
   virtual IceRole GetIceRole() const = 0;
 
   virtual void SetIceRole(IceRole role) = 0;
@@ skipping 30 matching lines @@
 
   virtual void SetMetricsObserver(
       webrtc::MetricsObserverInterface* observer) = 0;
 
   virtual void AddRemoteCandidate(const Candidate& candidate) = 0;
 
   virtual void RemoveRemoteCandidate(const Candidate& candidate) = 0;
 
   virtual IceGatheringState gathering_state() const = 0;
 
+  // Returns the current stats for this connection.
+  virtual bool GetStats(ConnectionInfos* infos) = 0;
+
   sigslot::signal1<IceTransportInternal*> SignalGatheringState;
 
   // Handles sending and receiving of candidates.
   sigslot::signal2<IceTransportInternal*, const Candidate&>
       SignalCandidateGathered;
 
   sigslot::signal2<IceTransportInternal*, const Candidates&>
       SignalCandidatesRemoved;
 
   // Deprecated by SignalSelectedCandidatePairChanged
   // This signal occurs when there is a change in the way that packets are
   // being routed, i.e. to a different remote location. The candidate
   // indicates where and how we are currently sending media.
   sigslot::signal2<IceTransportInternal*, const Candidate&> SignalRouteChange;
 
   // Signalled when the current selected candidate pair has changed.
   // The first parameter is the transport that signals the event.
   // The second parameter is the new selected candidate pair. The third
   // parameter is the last packet id sent on the previous candidate pair.
   // The fourth parameter is a boolean which is true if the Transport
   // is ready to send with this candidate pair.
   sigslot::signal4<IceTransportInternal*, CandidatePairInterface*, int, bool>
       SignalSelectedCandidatePairChanged;
 
+  // Invoked when there is conflict in the ICE role between local and remote
+  // agents.
+  sigslot::signal1<IceTransportInternal*> SignalRoleConflict;
+
+  // Emitted whenever the transport channel state changed.

[Taylor Brandstetter, 2016/12/20 03:37:26]

transport channel -> transport

[Zhi Huang, 2016/12/20 20:17:43]

Done.

+  sigslot::signal1<IceTransportInternal*> SignalStateChanged;
+
   // Invoked when the transport is being destroyed.
   sigslot::signal1<IceTransportInternal*> SignalDestroyed;
 
   // Debugging description of this transport.
-  std::string ToString() const;
+  virtual const std::string debug_name() const {

[Taylor Brandstetter, 2016/12/20 03:37:26]

nit: Doesn't need to return a const string.

[Zhi Huang, 2016/12/20 20:17:43]

I meant to override the PacketTransportInterface::debug_name().
I'll change it to 
 
   const std::string debug_name() const override {...}

Or do you think it's better to change the method signature in
PacketTransportInterface?

[Taylor Brandstetter, 2016/12/20 20:27:15]

Ah, I see. I think in PacketTransportInterface it was meant to return a const
ref. That's fine, although that would mean you can't construct the debug name in
this method, it would need to be stored as a member variable. Which means the
implementation should move to P2PTransportChannel/FakeIceTransport.

Or you can just return a string by value; since it's only used for debug log
statements, I don't think the performance overhead is important.

+    return transport_name() + " " + std::to_string(component());
+  }
 };
 
 }  // namespace cricket
 
 #endif  // WEBRTC_P2P_BASE_ICETRANSPORTINTERNAL_H_