Introduce rtc::PacketTransportInterface and let cricket::TransportChannel inherit.

webrtc/p2p/base/dtlstransportchannel.cc

 /*
  *  Copyright 2011 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.
  */
 
 #include <memory>
 #include <utility>
 
 #include "webrtc/p2p/base/dtlstransportchannel.h"
 
 #include "webrtc/p2p/base/common.h"
+#include "webrtc/p2p/base/packettransport.h"
 #include "webrtc/base/buffer.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/dscp.h"
 #include "webrtc/base/messagequeue.h"
 #include "webrtc/base/sslstreamadapter.h"
 #include "webrtc/base/stream.h"
 #include "webrtc/base/thread.h"
 
 namespace cricket {
 
@@ skipping 403 matching lines @@
 // The state transition logic here is as follows:
 // (1) If we're not doing DTLS-SRTP, then the state is just the
 //     state of the underlying impl()
 // (2) If we're doing DTLS-SRTP:
 //     - Prior to the DTLS handshake, the state is neither receiving nor
 //       writable
 //     - When the impl goes writable for the first time we
 //       start the DTLS handshake
 //     - Once the DTLS handshake completes, the state is that of the
 //       impl again
-void DtlsTransportChannelWrapper::OnWritableState(TransportChannel* channel) {
+void DtlsTransportChannelWrapper::OnWritableState(rtc::PacketTransport* pt) {
+  TransportChannel* channel = static_cast<TransportChannel*>(pt);

[pthatcher1, 2016/10/13 20:26:37]

The only thing this method needs from pt is ->writable(), which is part of the
interface.  So why do we need the type cast?   I guess there's the RTC_DCHECK,
but that's just a pointer compare, and we could move the cast to just that line
if the compiler requires it.

But it might be better to change the member variable "TransportChannel*
channel_" into "PacketTransportInterface* transport_".  Then the check would be
transport_ == transport, which wouldn't need the type cast.

I only see one place where we need anything from channel_ that we wouldn't get
from transport_, and that's the RTC_DCHECK in OnReceivingState, which is the
same as this RTC_DCHECK except in reverse.

[johan, 2016/10/14 16:09:33]

Done.
In the methods above are several lines of forwarding TransportChannel's signals
to subscribers of DtlsTransportChannel. That code won't compile, if we replace
the member variable channel_ by transport_. What are the plans are for this
signal-fowarding code, anyway?

   ASSERT(rtc::Thread::Current() == worker_thread_);
-  ASSERT(channel == channel_);
+  RTC_DCHECK(channel == channel_);
   LOG_J(LS_VERBOSE, this)
       << "DTLSTransportChannelWrapper: channel writable state changed to "
       << channel_->writable();
 
   if (!dtls_active_) {
     // Not doing DTLS.
     // Note: SignalWritableState fired by set_writable.
     set_writable(channel_->writable());
     return;
   }
@@ skipping 11 matching lines @@
       break;
     case DTLS_TRANSPORT_FAILED:
     case DTLS_TRANSPORT_CLOSED:
       // Should not happen. Do nothing.
       break;
   }
 }
 
 void DtlsTransportChannelWrapper::OnReceivingState(TransportChannel* channel) {
   ASSERT(rtc::Thread::Current() == worker_thread_);
-  ASSERT(channel == channel_);
+  RTC_DCHECK(channel == channel_);
   LOG_J(LS_VERBOSE, this)
       << "DTLSTransportChannelWrapper: channel receiving state changed to "
       << channel_->receiving();
   if (!dtls_active_ || dtls_state() == DTLS_TRANSPORT_CONNECTED) {
     // Note: SignalReceivingState fired by set_receiving.
     set_receiving(channel_->receiving());
   }
 }
 
 void DtlsTransportChannelWrapper::OnReadPacket(
-    TransportChannel* channel, const char* data, size_t size,
-    const rtc::PacketTime& packet_time, int flags) {
+    rtc::PacketTransport* pt,
+    const char* data,
+    size_t size,
+    const rtc::PacketTime& packet_time,
+    int flags) {
+  TransportChannel* channel = static_cast<TransportChannel*>(pt);
   ASSERT(rtc::Thread::Current() == worker_thread_);
-  ASSERT(channel == channel_);
+  RTC_DCHECK(channel == channel_);
   ASSERT(flags == 0);
 
   if (!dtls_active_) {
     // Not doing DTLS.
     SignalReadPacket(this, data, size, packet_time, 0);
     return;
   }
 
   switch (dtls_state()) {
     case DTLS_TRANSPORT_NEW:
@@ skipping 52 matching lines @@
       }
       break;
     case DTLS_TRANSPORT_FAILED:
     case DTLS_TRANSPORT_CLOSED:
       // This shouldn't be happening. Drop the packet.
       break;
   }
 }
 
 void DtlsTransportChannelWrapper::OnSentPacket(
-    TransportChannel* channel,
+    rtc::PacketTransport* pt,
     const rtc::SentPacket& sent_packet) {
   ASSERT(rtc::Thread::Current() == worker_thread_);
 
   SignalSentPacket(this, sent_packet);
 }
 
-void DtlsTransportChannelWrapper::OnReadyToSend(TransportChannel* channel) {
+void DtlsTransportChannelWrapper::OnReadyToSend(rtc::PacketTransport* pt) {
   if (writable()) {
     SignalReadyToSend(this);
   }
 }
 
 void DtlsTransportChannelWrapper::OnDtlsEvent(rtc::StreamInterface* dtls,
                                               int sig, int err) {
   ASSERT(rtc::Thread::Current() == worker_thread_);
   ASSERT(dtls == dtls_.get());
   if (sig & rtc::SE_OPEN) {
@@ skipping 144 matching lines @@
   ASSERT(channel == channel_);
   SignalStateChanged(this);
 }
 
 void DtlsTransportChannelWrapper::OnDtlsHandshakeError(
     rtc::SSLHandshakeError error) {
   SignalDtlsHandshakeError(error);
 }
 
 }  // namespace cricket