Revert of Added send-thresholding and fixed text packet dumping. (patchset #4 id:160001 of https://codereview.webrtc.org/1266033005/ )

Reason for revert:
The CL adds a global variable.

Original issue's description:
> Added send-thresholding and fixed text packet dumping.  Also a little squelch for the over-max-MTU log spam we see in there.
>
> BUG=https://code.google.com/p/webrtc/issues/detail?id=4468
> R=pthatcher@chromium.org, pthatcher@webrtc.org
>
> Committed: d838d27919

TBR=pthatcher@webrtc.org,bemasc@webrtc.org,pthatcher@chromium.org,thakis@chromium.org,lally@webrtc.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=https://code.google.com/p/webrtc/issues/detail?id=4468

Review URL: https://codereview.webrtc.org/1315923003

Cr-Commit-Position: refs/heads/master@{#9796}

talk/media/sctp/sctpdataengine.cc

@@ -109,8 +109,6 @@ typedef rtc::ScopedMessageData<rtc::Buffer> OutboundPacketMessage;
 // take off 80 bytes for DTLS/TURN/TCP/IP overhead.
 static const size_t kSctpMtu = 1200;
 
-// The size of the SCTP association send buffer.  256kB, the usrsctp default.
-static const int kSendBufferSize = 262144;
 enum {
   MSG_SCTPINBOUNDPACKET = 1,   // MessageData is SctpInboundPacket
   MSG_SCTPOUTBOUNDPACKET = 2,  // MessageData is rtc:Buffer
@@ -179,11 +177,11 @@ static bool GetDataMediaType(
 }
 
 // Log the packet in text2pcap format, if log level is at LS_VERBOSE.
-static void VerboseLogPacket(void *data, size_t length, int direction) {
+static void VerboseLogPacket(void *addr, size_t length, int direction) {
   if (LOG_CHECK_LEVEL(LS_VERBOSE) && length > 0) {
     char *dump_buf;
     if ((dump_buf = usrsctp_dumppacket(
-             data, length, direction)) != NULL) {
+             addr, length, direction)) != NULL) {
       LOG(LS_VERBOSE) << dump_buf;
       usrsctp_freedumpbuffer(dump_buf);
     }
@@ -246,10 +244,6 @@ static int OnSctpInboundPacket(struct socket* sock, union sctp_sockstore addr,
 
 // Set the initial value of the static SCTP Data Engines reference count.
 int SctpDataEngine::usrsctp_engines_count = 0;
-// All the channels created by this engine, used for callbacks from
-// usrsctplib that only contain socket pointers.  Channels are removed when
-// SignalDestroyed is fired.
-std::vector<SctpDataMediaChannel*> SctpDataEngine::channels_;
 
 SctpDataEngine::SctpDataEngine() {
   if (usrsctp_engines_count == 0) {
@@ -264,11 +258,6 @@ SctpDataEngine::SctpDataEngine() {
     // TODO(ldixon): Consider turning this on/off.
     usrsctp_sysctl_set_sctp_ecn_enable(0);
 
-    int send_size = usrsctp_sysctl_get_sctp_sendspace();
-    if (send_size != kSendBufferSize) {
-      LOG(LS_ERROR) << "Got different send size than expected: " << send_size;
-    }
-
     // TODO(ldixon): Consider turning this on/off.
     // This is not needed right now (we don't do dynamic address changes):
     // If SCTP Auto-ASCONF is enabled, the peer is informed automatically
@@ -323,48 +312,9 @@ DataMediaChannel* SctpDataEngine::CreateChannel(
   if (data_channel_type != DCT_SCTP) {
     return NULL;
   }
-  SctpDataMediaChannel *channel = new SctpDataMediaChannel(
+  return new SctpDataMediaChannel(rtc::Thread::Current());
-      rtc::Thread::Current());
-  channels_.push_back(channel);
-  channel->SignalDestroyed.connect(this, &SctpDataEngine::OnChannelDestroyed);
-  return channel;
 }
 
-// static
-SctpDataMediaChannel* SctpDataEngine::GetChannelFromSocket(
-    struct socket* sock) {
-  for (auto p:channels_) {
-    if (p->socket() == sock) {
-      return p;
-    }
-  }
-  return 0;
-}
-
-
-void SctpDataEngine::OnChannelDestroyed(SctpDataMediaChannel* channel) {
-  auto it = std::find(channels_.begin(), channels_.end(), channel);
-  if (it == channels_.end()) {
-    LOG(LS_ERROR) << "OnChannelDestroyed: the channel wasn't registered.";
-    return;
-  }
-  channels_.erase(it);
-}
-
-// static
-int SctpDataEngine::SendThresholdCallback(struct socket* sock,
-                                          uint32_t sb_free) {
-  SctpDataMediaChannel *channel = GetChannelFromSocket(sock);
-  if (!channel) {
-    LOG(LS_ERROR) << "SendThresholdCallback: Failed to get channel for socket "
-                  << sock;
-    return 0;
-  }
-  channel->SignalReadyToSend(true);
-  return 0;
-}
-
-
 SctpDataMediaChannel::SctpDataMediaChannel(rtc::Thread* thread)
     : worker_thread_(thread),
       local_port_(kSctpDefaultPort),
@@ -377,7 +327,6 @@ SctpDataMediaChannel::SctpDataMediaChannel(rtc::Thread* thread)
 
 SctpDataMediaChannel::~SctpDataMediaChannel() {
   CloseSctpSocket();
-  SignalDestroyed(this);
 }
 
 sockaddr_conn SctpDataMediaChannel::GetSctpSockAddr(int port) {
@@ -398,16 +347,8 @@ bool SctpDataMediaChannel::OpenSctpSocket() {
                     << "->Ignoring attempt to re-create existing socket.";
     return false;
   }
-
-  // If kSendBufferSize isn't reflective of reality, we log an error, but we
-  // still have to do something reasonable here.  Look up what the buffer's
-  // real size is and set our threshold to something reasonable.
-  const static int send_threshold = usrsctp_sysctl_get_sctp_sendspace() / 2;
-
   sock_ = usrsctp_socket(AF_CONN, SOCK_STREAM, IPPROTO_SCTP,
-                         cricket::OnSctpInboundPacket,
+                         cricket::OnSctpInboundPacket, NULL, 0, this);
-                         &SctpDataEngine::SendThresholdCallback,
-                         send_threshold, this);
   if (!sock_) {
     LOG_ERRNO(LS_ERROR) << debug_name_ << "Failed to create SCTP socket.";
     return false;
@@ -452,8 +393,7 @@ bool SctpDataMediaChannel::OpenSctpSocket() {
   }
 
   // Disable MTU discovery
-  struct sctp_paddrparams params;
+  struct sctp_paddrparams params = {{0}};
-  memset(&params, 0, sizeof(params));
   params.spp_assoc_id = 0;
   params.spp_flags = SPP_PMTUD_DISABLE;
   params.spp_pathmtu = kSctpMtu;
@@ -964,17 +904,10 @@ bool SctpDataMediaChannel::SetRecvCodecs(const std::vector<DataCodec>& codecs) {
 
 void SctpDataMediaChannel::OnPacketFromSctpToNetwork(
     rtc::Buffer* buffer) {
-  // usrsctp seems to interpret the MTU we give it strangely -- it seems to
+  if (buffer->size() > kSctpMtu) {
-  // give us back packets bigger than that MTU, if only by a fixed amount.
-  // This is that amount that we've observed.
-  const int kSctpOverhead = 76;
-  if (buffer->size() > (kSctpOverhead + kSctpMtu)) {
     LOG(LS_ERROR) << debug_name_ << "->OnPacketFromSctpToNetwork(...): "
                   << "SCTP seems to have made a packet that is bigger "
-                  << "than its official MTU: " << buffer->size()
+                     "than its official MTU.";
-                  << " vs max of " << kSctpMtu
-                  << " even after adding " << kSctpOverhead
-                  << " extra SCTP overhead";
   }
   MediaChannel::SendPacket(buffer);
 }

talk/media/sctp/sctpdataengine.h

@@ -64,8 +64,6 @@ const uint32 kMaxSctpSid = 1023;
 // usrsctp.h)
 const int kSctpDefaultPort = 5000;
 
-class SctpDataMediaChannel;
-
 // A DataEngine that interacts with usrsctp.
 //
 // From channel calls, data flows like this:
@@ -90,7 +88,7 @@ class SctpDataMediaChannel;
 //  14. SctpDataMediaChannel::SignalDataReceived(data)
 // [from the same thread, methods registered/connected to
 //  SctpDataMediaChannel are called with the recieved data]
-class SctpDataEngine : public DataEngineInterface, public sigslot::has_slots<> {
+class SctpDataEngine : public DataEngineInterface {
  public:
   SctpDataEngine();
   virtual ~SctpDataEngine();
@@ -99,15 +97,9 @@ class SctpDataEngine : public DataEngineInterface, public sigslot::has_slots<> {
 
   virtual const std::vector<DataCodec>& data_codecs() { return codecs_; }
 
-  static int SendThresholdCallback(struct socket* sock, uint32_t sb_free);
-
  private:
-  static std::vector<SctpDataMediaChannel*> channels_;
   static int usrsctp_engines_count;
   std::vector<DataCodec> codecs_;
-
-  static SctpDataMediaChannel* GetChannelFromSocket(struct socket* sock);
-  void OnChannelDestroyed(SctpDataMediaChannel *channel);
 };
 
 // TODO(ldixon): Make into a special type of TypedMessageData.
@@ -196,9 +188,6 @@ class SctpDataMediaChannel : public DataMediaChannel,
     debug_name_ = debug_name;
   }
   const std::string& debug_name() const { return debug_name_; }
-  const struct socket* socket() { return sock_; }
-
-  sigslot::signal1<SctpDataMediaChannel*> SignalDestroyed;
 
  private:
   sockaddr_conn GetSctpSockAddr(int port);

talk/media/sctp/sctpdataengine_unittest.cc

@@ -240,16 +240,10 @@ class SctpDataMediaChannelTest : public testing::Test,
     net2_.reset(new SctpFakeNetworkInterface(rtc::Thread::Current()));
     recv1_.reset(new SctpFakeDataReceiver());
     recv2_.reset(new SctpFakeDataReceiver());
-    chan1_ready_to_send_count_ = 0;
-    chan2_ready_to_send_count_ = 0;
     chan1_.reset(CreateChannel(net1_.get(), recv1_.get()));
     chan1_->set_debug_name("chan1/connector");
-    chan1_->SignalReadyToSend.connect(
-        this, &SctpDataMediaChannelTest::OnChan1ReadyToSend);
     chan2_.reset(CreateChannel(net2_.get(), recv2_.get()));
     chan2_->set_debug_name("chan2/listener");
-    chan2_->SignalReadyToSend.connect(
-        this, &SctpDataMediaChannelTest::OnChan2ReadyToSend);
     // Setup two connected channels ready to send and receive.
     net1_->SetDestination(chan2_.get());
     net2_->SetDestination(chan1_.get());
@@ -336,8 +330,6 @@ class SctpDataMediaChannelTest : public testing::Test,
   SctpFakeDataReceiver* receiver1() { return recv1_.get(); }
   SctpFakeDataReceiver* receiver2() { return recv2_.get(); }
 
-  int channel1_ready_to_send_count() { return chan1_ready_to_send_count_; }
-  int channel2_ready_to_send_count() { return chan2_ready_to_send_count_; }
  private:
   rtc::scoped_ptr<cricket::SctpDataEngine> engine_;
   rtc::scoped_ptr<SctpFakeNetworkInterface> net1_;
@@ -346,12 +338,6 @@ class SctpDataMediaChannelTest : public testing::Test,
   rtc::scoped_ptr<SctpFakeDataReceiver> recv2_;
   rtc::scoped_ptr<cricket::SctpDataMediaChannel> chan1_;
   rtc::scoped_ptr<cricket::SctpDataMediaChannel> chan2_;
-
-  int chan1_ready_to_send_count_;
-  int chan2_ready_to_send_count_;
-
-  void OnChan1ReadyToSend(bool send) { if (send) chan1_ready_to_send_count_++; }
-  void OnChan2ReadyToSend(bool send) { if (send) chan2_ready_to_send_count_++; }
 };
 
 // Verifies that SignalReadyToSend is fired.
@@ -500,15 +486,6 @@ TEST_F(SctpDataMediaChannelTest, ClosesStreamsOnBothSides) {
   EXPECT_TRUE_WAIT(chan_1_sig_receiver.WasStreamClosed(4), 1000);
 }
 
-TEST_F(SctpDataMediaChannelTest, EngineSignalsRightChannel) {
-  SetupConnectedChannels();
-  EXPECT_TRUE_WAIT(channel1()->socket() != NULL, 1000);
-  struct socket *sock = const_cast<struct socket*>(channel1()->socket());
-  int prior_count = channel1_ready_to_send_count();
-  cricket::SctpDataEngine::SendThresholdCallback(sock, 0);
-  EXPECT_GT(channel1_ready_to_send_count(), prior_count);
-}
-
 // Flaky on Linux and Windows. See webrtc:4453.
 #if defined(WEBRTC_WIN) || defined(WEBRTC_LINUX)
 #define MAYBE_ReusesAStream DISABLED_ReusesAStream