Bug 1551702 - hide DataChannelConnection ctor, and set local port - r=bwc
authorNico Grunbaum <na-g@nostrum.com>
Thu, 16 May 2019 18:43:13 +0000
changeset 532980 578cc4c154efb9d1d54da26e2f2818df88b3971e
parent 532979 cb21c0b524ff08b12769387a1f8d3fa42258cb22
child 532981 5fae8054799fd916ceca727611c2f99afc83e93e
push id11276
push userrgurzau@mozilla.com
push dateMon, 20 May 2019 13:11:24 +0000
treeherdermozilla-beta@847755a7c325 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1551702
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1551702 - hide DataChannelConnection ctor, and set local port - r=bwc Differential Revision: https://phabricator.services.mozilla.com/D31343
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
netwerk/sctp/datachannel/DataChannel.cpp
netwerk/sctp/datachannel/DataChannel.h
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -849,25 +849,26 @@ PeerConnectionImpl::EnsureDataConnection
   if (mDataConnection) {
     CSFLogDebug(LOGTAG, "%s DataConnection already connected", __FUNCTION__);
     mDataConnection->SetMaxMessageSize(aMMSSet, aMaxMessageSize);
     return NS_OK;
   }
 
   nsCOMPtr<nsIEventTarget> target =
       mWindow ? mWindow->EventTargetFor(TaskCategory::Other) : nullptr;
-  mDataConnection = new DataChannelConnection(this, target, mTransportHandler);
-  if (!mDataConnection->Init(aLocalPort, aNumstreams, aMMSSet,
-                             aMaxMessageSize)) {
-    CSFLogError(LOGTAG, "%s DataConnection Init Failed", __FUNCTION__);
-    return NS_ERROR_FAILURE;
+  Maybe<uint64_t> mms = aMMSSet ? Some(aMaxMessageSize) : Nothing();
+  if (auto res = DataChannelConnection::Create(this, target, mTransportHandler,
+                                               aLocalPort, aNumstreams, mms)) {
+    mDataConnection = res.value();
+    CSFLogDebug(LOGTAG, "%s DataChannelConnection %p attached to %s",
+                __FUNCTION__, (void*)mDataConnection.get(), mHandle.c_str());
+    return NS_OK;
   }
-  CSFLogDebug(LOGTAG, "%s DataChannelConnection %p attached to %s",
-              __FUNCTION__, (void*)mDataConnection.get(), mHandle.c_str());
-  return NS_OK;
+  CSFLogError(LOGTAG, "%s DataConnection Create Failed", __FUNCTION__);
+  return NS_ERROR_FAILURE;
 }
 
 nsresult PeerConnectionImpl::GetDatachannelParameters(
     uint32_t* channels, uint16_t* localport, uint16_t* remoteport,
     uint32_t* remotemaxmessagesize, bool* mmsset, std::string* transportId,
     bool* client) const {
   for (const auto& transceiver : mJsepSession->GetTransceivers()) {
     bool dataChannel =
--- a/netwerk/sctp/datachannel/DataChannel.cpp
+++ b/netwerk/sctp/datachannel/DataChannel.cpp
@@ -284,44 +284,16 @@ static void debug_printf(const char* for
     if (VsprintfLiteral(buffer, format, ap) > 0) {
 #endif
       SCTP_LOG(("%s", buffer));
     }
     va_end(ap);
   }
 }
 
-DataChannelConnection::DataChannelConnection(DataConnectionListener* listener,
-                                             nsIEventTarget* aTarget,
-                                             MediaTransportHandler* aHandler)
-    : NeckoTargetHolder(aTarget),
-      mLock("netwerk::sctp::DataChannelConnection"),
-      mSendInterleaved(false),
-      mPpidFragmentation(false),
-      mMaxMessageSizeSet(false),
-      mMaxMessageSize(0),
-      mAllocateEven(false),
-      mTransportHandler(aHandler),
-      mDeferSend(false) {
-  mCurrentStream = 0;
-  mState = CLOSED;
-  mSocket = nullptr;
-  mMasterSocket = nullptr;
-  mListener = listener;
-  mLocalPort = 0;
-  mRemotePort = 0;
-  mPendingType = PENDING_NONE;
-  LOG(("Constructor DataChannelConnection=%p, listener=%p", this,
-       mListener.get()));
-  mInternalIOThread = nullptr;
-#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
-  mShutdown = false;
-#endif
-}
-
 DataChannelConnection::~DataChannelConnection() {
   LOG(("Deleting DataChannelConnection %p", (void*)this));
   // This may die on the MainThread, or on the STS thread
   ASSERT_WEBRTC(mState == CLOSED);
   MOZ_ASSERT(!mMasterSocket);
   MOZ_ASSERT(mPending.GetSize() == 0);
 
   if (!IsSTSThread()) {
@@ -400,38 +372,64 @@ void DataChannelConnection::DestroyOnSTS
                               &DataChannelConnection::DestroyOnSTSFinal),
                  NS_DISPATCH_NORMAL);
 }
 
 void DataChannelConnection::DestroyOnSTSFinal() {
   sDataChannelShutdown->CreateConnectionShutdown(this);
 }
 
-bool DataChannelConnection::Init(unsigned short aPort, uint16_t aNumStreams,
-                                 bool aMaxMessageSizeSet,
-                                 uint64_t aMaxMessageSize) {
+Maybe<RefPtr<DataChannelConnection>> DataChannelConnection::Create(
+    DataChannelConnection::DataConnectionListener* aListener,
+    nsIEventTarget* aTarget, MediaTransportHandler* aHandler,
+    const uint16_t aLocalPort, const uint16_t aNumStreams,
+    const Maybe<uint64_t>& aMaxMessageSize) {
+  ASSERT_WEBRTC(NS_IsMainThread());
+
+  RefPtr<DataChannelConnection> connection = new DataChannelConnection(
+      aListener, aTarget, aHandler);  // Walks into a bar
+  return connection->Init(aLocalPort, aNumStreams, aMaxMessageSize)
+             ? Some(connection)
+             : Nothing();
+}
+
+DataChannelConnection::DataChannelConnection(
+    DataChannelConnection::DataConnectionListener* aListener,
+    nsIEventTarget* aTarget, MediaTransportHandler* aHandler)
+    : NeckoTargetHolder(aTarget),
+      mLock("netwerk::sctp::DataChannelConnection"),
+      mListener(aListener),
+      mTransportHandler(aHandler) {
+  LOG(("Constructor DataChannelConnection=%p, listener=%p", this,
+       mListener.get()));
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+  mShutdown = false;
+#endif
+}
+
+bool DataChannelConnection::Init(const uint16_t aLocalPort,
+                                 const uint16_t aNumStreams,
+                                 const Maybe<uint64_t>& aMaxMessageSize) {
+  ASSERT_WEBRTC(NS_IsMainThread());
+
   struct sctp_initmsg initmsg;
   struct sctp_assoc_value av;
   struct sctp_event event;
   socklen_t len;
 
   uint16_t event_types[] = {
       SCTP_ASSOC_CHANGE,          SCTP_PEER_ADDR_CHANGE,
       SCTP_REMOTE_ERROR,          SCTP_SHUTDOWN_EVENT,
       SCTP_ADAPTATION_INDICATION, SCTP_PARTIAL_DELIVERY_EVENT,
       SCTP_SEND_FAILED_EVENT,     SCTP_STREAM_RESET_EVENT,
       SCTP_STREAM_CHANGE_EVENT};
   {
-    ASSERT_WEBRTC(NS_IsMainThread());
     // MutexAutoLock lock(mLock); Not needed since we're on mainthread always
-
-    mSendInterleaved = false;
-    mPpidFragmentation = false;
-    mMaxMessageSizeSet = false;
-    SetMaxMessageSize(aMaxMessageSizeSet, aMaxMessageSize);
+    mLocalPort = aLocalPort;
+    SetMaxMessageSize(aMaxMessageSize.isSome(), aMaxMessageSize.valueOr(0));
 
     if (!sctp_initialized) {
       LOG(("sctp_init"));
 #ifdef MOZ_PEERCONNECTION
       usrsctp_init(0, DataChannelConnection::SctpDtlsOutput, debug_printf);
 #else
       MOZ_CRASH("Trying to use SCTP/DTLS without mtransport");
 #endif
--- a/netwerk/sctp/datachannel/DataChannel.h
+++ b/netwerk/sctp/datachannel/DataChannel.h
@@ -130,22 +130,22 @@ class DataChannelConnection final : publ
     MOZ_DECLARE_WEAKREFERENCE_TYPENAME(
         DataChannelConnection::DataConnectionListener)
     virtual ~DataConnectionListener() = default;
 
     // Called when a new DataChannel has been opened by the other side.
     virtual void NotifyDataChannel(already_AddRefed<DataChannel> channel) = 0;
   };
 
-  DataChannelConnection(DataConnectionListener* listener,
-                        nsIEventTarget* aTarget,
-                        MediaTransportHandler* aTransportHandler);
-
-  bool Init(unsigned short aPort, uint16_t aNumStreams, bool aMaxMessageSizeSet,
-            uint64_t aMaxMessageSize);
+  // Create a new DataChannel Connection
+  // Must be called on Main thread
+  static Maybe<RefPtr<DataChannelConnection>> Create(
+      DataConnectionListener* aListener, nsIEventTarget* aTarget,
+      MediaTransportHandler* aHandler, const uint16_t aLocalPort,
+      const uint16_t aNumStreams, const Maybe<uint64_t>& aMaxMessageSize);
 
   void Destroy();  // So we can spawn refs tied to runnables in shutdown
   // Finish Destroy on STS to avoid SCTP race condition with ABORT from far end
   void DestroyOnSTS(struct socket* aMasterSocket, struct socket* aSocket);
   void DestroyOnSTSFinal();
 
   void SetMaxMessageSize(bool aMaxMessageSizeSet, uint64_t aMaxMessageSize);
   uint64_t GetMaxMessageSize();
@@ -224,16 +224,23 @@ class DataChannelConnection final : publ
   friend class DataChannelOnMessageAvailable;
   // Avoid cycles with PeerConnectionImpl
   // Use from main thread only as WeakPtr is not threadsafe
   WeakPtr<DataConnectionListener> mListener;
 
  private:
   friend class DataChannelConnectRunnable;
 
+  DataChannelConnection(DataConnectionListener* aListener,
+                        nsIEventTarget* aTarget,
+                        MediaTransportHandler* aHandler);
+
+  bool Init(const uint16_t aLocalPort, const uint16_t aNumStreams,
+            const Maybe<uint64_t>& aMaxMessageSize);
+
 #ifdef SCTP_DTLS_SUPPORTED
   static void DTLSConnectThread(void* data);
   void SendPacket(std::unique_ptr<MediaPacket>&& packet);
   void SctpDtlsInput(const std::string& aTransportId, MediaPacket& packet);
   static int SctpDtlsOutput(void* addr, void* buffer, size_t length,
                             uint8_t tos, uint8_t set_df);
 #endif
   DataChannel* FindChannelByStream(uint16_t stream);
@@ -298,57 +305,56 @@ class DataChannelConnection final : publ
     bool on = false;
     if (mSTS) {
       mSTS->IsOnCurrentThread(&on);
     }
     return on;
   }
 #endif
 
-  bool mSendInterleaved;
-  bool mPpidFragmentation;
-  bool mMaxMessageSizeSet;
-  uint64_t mMaxMessageSize;
-
+  bool mSendInterleaved = false;
+  bool mPpidFragmentation = false;
+  bool mMaxMessageSizeSet = false;
+  uint64_t mMaxMessageSize = 0;
+  bool mAllocateEven = false;
   // Data:
   // NOTE: while this array will auto-expand, increases in the number of
   // channels available from the stack must be negotiated!
-  bool mAllocateEven;
   AutoTArray<RefPtr<DataChannel>, 16> mStreams;
-  uint32_t mCurrentStream;
+  uint32_t mCurrentStream = 0;
   nsDeque mPending;  // Holds addref'ed DataChannel's -- careful!
+  uint8_t mPendingType = PENDING_NONE;
   // holds data that's come in before a channel is open
   nsTArray<nsAutoPtr<QueuedDataMessage>> mQueuedData;
   // holds outgoing control messages
   nsTArray<nsAutoPtr<BufferedOutgoingMsg>>
       mBufferedControl;  // GUARDED_BY(mConnection->mLock)
 
   // Streams pending reset
   AutoTArray<uint16_t, 4> mStreamsResetting;
-
-  struct socket* mMasterSocket;  // accessed from STS thread
-  struct socket*
-      mSocket;  // cloned from mMasterSocket on successful Connect on STS thread
-  uint16_t mState;  // Protected with mLock
+  // accessed from STS thread
+  struct socket* mMasterSocket = nullptr;
+  // cloned from mMasterSocket on successful Connect on STS thread
+  struct socket* mSocket = nullptr;
+  uint16_t mState = CLOSED;  // Protected with mLock
 
 #ifdef SCTP_DTLS_SUPPORTED
   std::string mTransportId;
   RefPtr<MediaTransportHandler> mTransportHandler;
   nsCOMPtr<nsIEventTarget> mSTS;
 #endif
-  uint16_t mLocalPort;  // Accessed from connect thread
-  uint16_t mRemotePort;
+  uint16_t mLocalPort = 0;  // Accessed from connect thread
+  uint16_t mRemotePort = 0;
 
-  nsCOMPtr<nsIThread> mInternalIOThread;
-  uint8_t mPendingType;
+  nsCOMPtr<nsIThread> mInternalIOThread = nullptr;
   nsCString mRecvBuffer;
 
   // Workaround to prevent a message from being received on main before the
   // sender sees the decrease in bufferedAmount.
-  bool mDeferSend;
+  bool mDeferSend = false;
   std::vector<std::unique_ptr<MediaPacket>> mDeferredSend;
 
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   bool mShutdown;
 #endif
 };
 
 #define ENSURE_DATACONNECTION \