Bug 906986 - Rework rollback/finalize to include a committed state. draft
authorMichael Froman <mfroman@mozilla.com>
Tue, 05 Apr 2016 20:12:04 -0500
changeset 348109 3ca2243950f70cb0369eebfb9d18e575bc83742a
parent 348071 3040c6537ad783f17bb675de106f6a6134505219
child 348931 94adca718f98cc3058187bbd2dacec0f17cebfe8
push id14754
push usermfroman@nostrum.com
push dateWed, 06 Apr 2016 17:48:01 +0000
bugs906986
milestone48.0a1
Bug 906986 - Rework rollback/finalize to include a committed state. MozReview-Commit-ID: z7uEn5xEBf
media/mtransport/transportlayerice.cpp
media/mtransport/transportlayerice.h
media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
--- a/media/mtransport/transportlayerice.cpp
+++ b/media/mtransport/transportlayerice.cpp
@@ -123,38 +123,37 @@ void TransportLayerIce::PostSetup() {
   target_ = ctx_->thread();
 
   stream_->SignalReady.connect(this, &TransportLayerIce::IceReady);
   stream_->SignalFailed.connect(this, &TransportLayerIce::IceFailed);
   stream_->SignalPacketReceived.connect(this,
                                         &TransportLayerIce::IcePacketReceived);
   if (stream_->state() == NrIceMediaStream::ICE_OPEN) {
     TL_SET_STATE(TS_OPEN);
-    // Reset old ice stream if new stream is good
-    ResetOldStream();
   }
 }
 
 void TransportLayerIce::ResetOldStream() {
   if (old_stream_ == nullptr) {
     return; // no work to do
   }
-  // ICE Ready on the new stream, we can forget the old stream now
+  // ICE restart successful on the new stream, we can forget the old stream now
   MOZ_MTLOG(ML_INFO, LAYER_INFO << "ResetOldStream(" << old_stream_->name()
                                 << ")");
   old_stream_->SignalReady.disconnect(this);
   old_stream_->SignalFailed.disconnect(this);
   old_stream_->SignalPacketReceived.disconnect(this);
   old_stream_ = nullptr;
 }
 
 void TransportLayerIce::RestoreOldStream() {
   if (old_stream_ == nullptr) {
     return; // no work to do
   }
+  // ICE restart rollback, we need to restore the old stream
   MOZ_MTLOG(ML_INFO, LAYER_INFO << "RestoreOldStream(" << old_stream_->name()
                                 << ")");
   stream_->SignalReady.disconnect(this);
   stream_->SignalFailed.disconnect(this);
   stream_->SignalPacketReceived.disconnect(this);
   stream_ = old_stream_;
   old_stream_ = nullptr;
 
@@ -196,18 +195,16 @@ void TransportLayerIce::IceReady(NrIceMe
   CheckThread();
   // only handle the current stream (not the old stream during restart)
   if (stream != stream_) {
     return;
   }
   MOZ_MTLOG(ML_INFO, LAYER_INFO << "ICE Ready(" << stream->name() << ","
     << component_ << ")");
   TL_SET_STATE(TS_OPEN);
-  // Reset old ice stream if new stream is good after ice restart
-  ResetOldStream();
 }
 
 void TransportLayerIce::IceFailed(NrIceMediaStream *stream) {
   CheckThread();
   // only handle the current stream (not the old stream during restart)
   if (stream != stream_) {
     return;
   }
--- a/media/mtransport/transportlayerice.h
+++ b/media/mtransport/transportlayerice.h
@@ -33,33 +33,34 @@ class TransportLayerIce : public Transpo
   explicit TransportLayerIce(const std::string& name);
 
   virtual ~TransportLayerIce();
 
   void SetParameters(RefPtr<NrIceCtx> ctx,
                      RefPtr<NrIceMediaStream> stream,
                      int component);
 
+  void ResetOldStream(); // called after successful ice restart
+  void RestoreOldStream(); // called after unsuccessful ice restart
+
   // Transport layer overrides.
   virtual TransportResult SendPacket(const unsigned char *data, size_t len);
 
   // Slots for ICE
   void IceCandidate(NrIceMediaStream *stream, const std::string&);
   void IceReady(NrIceMediaStream *stream);
   void IceFailed(NrIceMediaStream *stream);
   void IcePacketReceived(NrIceMediaStream *stream, int component,
                          const unsigned char *data, int len);
 
   TRANSPORT_LAYER_ID("ice")
 
  private:
   DISALLOW_COPY_ASSIGN(TransportLayerIce);
   void PostSetup();
-  void ResetOldStream(); // called after successful ice restart
-  void RestoreOldStream(); // called after unsuccessful ice restart
 
   const std::string name_;
   RefPtr<NrIceCtx> ctx_;
   RefPtr<NrIceMediaStream> stream_;
   int component_;
 
   // used to hold the old stream
   RefPtr<NrIceMediaStream> old_stream_;
--- a/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
+++ b/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
@@ -221,17 +221,17 @@ MediaPipelineFactory::CreateOrGetTranspo
     const JsepTransport& aTransport,
     RefPtr<TransportFlow>* aFlowOutparam)
 {
   nsresult rv;
   RefPtr<TransportFlow> flow;
 
   flow = mPCMedia->GetTransportFlow(aLevel, aIsRtcp);
   if (flow) {
-    if (mPCMedia->ice_ctx_hdlr()->IsRestarting()) {
+    if (mPCMedia->IsIceRestarting()) {
       MOZ_MTLOG(ML_INFO, "Flow[" << flow->id() << "]: "
                                  << "detected ICE restart - level: "
                                  << aLevel << " rtcp: " << aIsRtcp);
 
       rv = mPCMedia->GetSTSThread()->Dispatch(
           WrapRunnableNM(AddNewIceStreamForRestart_s,
                          mPCMedia, flow, aLevel, aIsRtcp),
           NS_DISPATCH_NORMAL);
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -1561,17 +1561,19 @@ static void DeferredCreateOffer(const st
 }
 
 // Used by unit tests and the IDL CreateOffer.
 NS_IMETHODIMP
 PeerConnectionImpl::CreateOffer(const JsepOfferOptions& aOptions)
 {
   PC_AUTO_ENTER_API_CALL(true);
   bool restartIce = aOptions.mIceRestart.isSome() && *(aOptions.mIceRestart);
-  if (!restartIce && mMedia->ice_ctx_hdlr()->IsRestarting()) {
+  if (!restartIce &&
+      mMedia->GetIceRestartState() ==
+          PeerConnectionMedia::ICE_RESTART_PROVISIONAL) {
     RollbackIceRestart();
   }
 
   RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
   if (!pco) {
     return NS_OK;
   }
 
@@ -1581,18 +1583,31 @@ PeerConnectionImpl::CreateOffer(const Js
         WrapRunnableNM(DeferredCreateOffer, mHandle, aOptions));
     STAMP_TIMECARD(mTimeCard, "Deferring CreateOffer (not ready)");
     return NS_OK;
   }
 
   CSFLogDebug(logTag, "CreateOffer()");
 
   nsresult nrv;
-  if (aOptions.mIceRestart.isSome() && *(aOptions.mIceRestart) &&
-      !mMedia->ice_ctx_hdlr()->IsRestarting()) {
+  if (restartIce) {
+    // If restart is requested and a restart is already in progress, we
+    // need to make room for the restart request so we either rollback
+    // or finalize to "clear" the previous restart.
+    if (mMedia->GetIceRestartState() ==
+            PeerConnectionMedia::ICE_RESTART_PROVISIONAL) {
+      // we're mid-restart and can rollback
+      RollbackIceRestart();
+    } else if (mMedia->GetIceRestartState() ==
+                   PeerConnectionMedia::ICE_RESTART_COMMITTED) {
+      // we're mid-restart and can't rollback, finalize restart even
+      // though we're not really ready yet
+      FinalizeIceRestart();
+    }
+
     CSFLogInfo(logTag, "Offerer restarting ice");
     nrv = SetupIceRestart();
     if (NS_FAILED(nrv)) {
       CSFLogError(logTag, "%s: SetupIceRestart failed, res=%u",
                            __FUNCTION__,
                            static_cast<unsigned>(nrv));
       return nrv;
     }
@@ -1641,23 +1656,28 @@ PeerConnectionImpl::CreateAnswer()
   if (!pco) {
     return NS_OK;
   }
 
   CSFLogDebug(logTag, "CreateAnswer()");
 
   nsresult nrv;
   if (mJsepSession->RemoteIceIsRestarting()) {
-    CSFLogInfo(logTag, "Answerer restarting ice");
-    nrv = SetupIceRestart();
-    if (NS_FAILED(nrv)) {
-      CSFLogError(logTag, "%s: SetupIceRestart failed, res=%u",
-                           __FUNCTION__,
-                           static_cast<unsigned>(nrv));
-      return nrv;
+    if (mMedia->GetIceRestartState() ==
+            PeerConnectionMedia::ICE_RESTART_COMMITTED) {
+      FinalizeIceRestart();
+    } else if (!mMedia->IsIceRestarting()) {
+      CSFLogInfo(logTag, "Answerer restarting ice");
+      nrv = SetupIceRestart();
+      if (NS_FAILED(nrv)) {
+        CSFLogError(logTag, "%s: SetupIceRestart failed, res=%u",
+                             __FUNCTION__,
+                             static_cast<unsigned>(nrv));
+        return nrv;
+      }
     }
   }
 
   STAMP_TIMECARD(mTimeCard, "Create Answer");
   // TODO(bug 1098015): Once RTCAnswerOptions is standardized, we'll need to
   // add it as a param to CreateAnswer, and convert it here.
   JsepAnswerOptions options;
   std::string answer;
@@ -1685,17 +1705,17 @@ PeerConnectionImpl::CreateAnswer()
   UpdateSignalingState();
 
   return NS_OK;
 }
 
 nsresult
 PeerConnectionImpl::SetupIceRestart()
 {
-  if (mMedia->ice_ctx_hdlr()->IsRestarting()) {
+  if (mMedia->IsIceRestarting()) {
     CSFLogError(logTag, "%s: ICE already restarting",
                          __FUNCTION__);
     return NS_ERROR_UNEXPECTED;
   }
 
   std::string ufrag = mMedia->ice_ctx()->GetNewUfrag();
   std::string pwd = mMedia->ice_ctx()->GetNewPwd();
   if (ufrag.empty() || pwd.empty()) {
@@ -1735,16 +1755,25 @@ PeerConnectionImpl::RollbackIceRestart()
     return nrv;
   }
   mPreviousIceUfrag = "";
   mPreviousIcePwd = "";
 
   return NS_OK;
 }
 
+void
+PeerConnectionImpl::FinalizeIceRestart()
+{
+  mMedia->FinalizeIceRestart();
+  // clear the previous ice creds since they are no longer needed
+  mPreviousIceUfrag = "";
+  mPreviousIcePwd = "";
+}
+
 NS_IMETHODIMP
 PeerConnectionImpl::SetLocalDescription(int32_t aAction, const char* aSDP)
 {
   PC_AUTO_ENTER_API_CALL(true);
 
   if (!aSDP) {
     CSFLogError(logTag, "%s - aSDP is NULL", __FUNCTION__);
     return NS_ERROR_FAILURE;
@@ -2868,18 +2897,23 @@ PeerConnectionImpl::SetSignalingState_m(
        !rollback)) {
     mMedia->EnsureTransports(*mJsepSession);
   }
 
   mSignalingState = aSignalingState;
 
   bool fireNegotiationNeeded = false;
   if (mSignalingState == PCImplSignalingState::SignalingStable) {
-    if (rollback && mMedia->ice_ctx_hdlr()->IsRestarting()) {
-      RollbackIceRestart();
+    if (mMedia->GetIceRestartState() ==
+            PeerConnectionMedia::ICE_RESTART_PROVISIONAL) {
+      if (rollback) {
+        RollbackIceRestart();
+      } else {
+        mMedia->CommitIceRestart();
+      }
     }
 
     // Either negotiation is done, or we've rolled back. In either case, we
     // need to re-evaluate whether further negotiation is required.
     mNegotiationNeeded = false;
     // If we're rolling back a local offer, we might need to remove some
     // transports, but nothing further needs to be done.
     mMedia->ActivateOrRemoveTransports(*mJsepSession);
@@ -3166,21 +3200,18 @@ void PeerConnectionImpl::IceConnectionSt
   }
 #endif
 
   mIceConnectionState = domState;
 
   if (mIceConnectionState == PCImplIceConnectionState::Connected ||
       mIceConnectionState == PCImplIceConnectionState::Completed ||
       mIceConnectionState == PCImplIceConnectionState::Failed) {
-    if (mMedia->ice_ctx_hdlr()->IsRestarting()) {
-      mMedia->FinalizeIceRestart();
-      // clear the previous ice creds since they are no longer needed
-      mPreviousIceUfrag = "";
-      mPreviousIcePwd = "";
+    if (mMedia->IsIceRestarting()) {
+      FinalizeIceRestart();
     }
   }
 
   // Would be nice if we had a means of converting one of these dom enums
   // to a string that wasn't almost as much text as this switch statement...
   switch (mIceConnectionState) {
     case PCImplIceConnectionState::New:
       STAMP_TIMECARD(mTimeCard, "Ice state: new");
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ -708,16 +708,17 @@ private:
                                             const std::string& trackId);
 
   nsresult AddTrackToJsepSession(SdpMediaSection::MediaType type,
                                  const std::string& streamId,
                                  const std::string& trackId);
 
   nsresult SetupIceRestart();
   nsresult RollbackIceRestart();
+  void FinalizeIceRestart();
 
 #if !defined(MOZILLA_EXTERNAL_LINKAGE)
   static void GetStatsForPCObserver_s(
       const std::string& pcHandle,
       nsAutoPtr<RTCStatsQuery> query);
 
   // Sends an RTCStatsReport to JS. Must run on main thread.
   static void DeliverStatsReportToPCObserver_m(
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
@@ -223,17 +223,18 @@ PeerConnectionMedia::PeerConnectionMedia
     : mParent(parent),
       mParentHandle(parent->GetHandle()),
       mParentName(parent->GetName()),
       mIceCtxHdlr(nullptr),
       mDNSResolver(new NrIceResolver()),
       mUuidGen(MakeUnique<PCUuidGenerator>()),
       mMainThread(mParent->GetMainThread()),
       mSTSThread(mParent->GetSTSThread()),
-      mProxyResolveCompleted(false) {
+      mProxyResolveCompleted(false),
+      mIceRestartState(ICE_RESTART_NONE) {
 }
 
 nsresult PeerConnectionMedia::Init(const std::vector<NrIceStunServer>& stun_servers,
                                    const std::vector<NrIceTurnServer>& turn_servers,
                                    NrIceCtx::Policy policy)
 {
   nsresult rv;
   nsCOMPtr<nsIProtocolProxyService> pps =
@@ -570,92 +571,150 @@ PeerConnectionMedia::StartIceChecks_s(
 
   mIceCtxHdlr->ctx()->SetControlling(aIsControlling ?
                                      NrIceCtx::ICE_CONTROLLING :
                                      NrIceCtx::ICE_CONTROLLED);
 
   mIceCtxHdlr->ctx()->StartChecks();
 }
 
+bool
+PeerConnectionMedia::IsIceRestarting() const
+{
+  ASSERT_ON_THREAD(mMainThread);
+
+  return (mIceRestartState != ICE_RESTART_NONE);
+}
+
+PeerConnectionMedia::IceRestartState
+PeerConnectionMedia::GetIceRestartState() const
+{
+  ASSERT_ON_THREAD(mMainThread);
+
+  return mIceRestartState;
+}
+
 void
 PeerConnectionMedia::BeginIceRestart(const std::string& ufrag,
                                      const std::string& pwd)
 {
   ASSERT_ON_THREAD(mMainThread);
+  if (IsIceRestarting()) {
+    return;
+  }
 
   bool default_address_only = GetPrefDefaultAddressOnly();
   RefPtr<NrIceCtx> new_ctx = mIceCtxHdlr->CreateCtx(ufrag,
                                                     pwd,
                                                     default_address_only);
 
   RUN_ON_THREAD(GetSTSThread(),
                 WrapRunnable(
                     RefPtr<PeerConnectionMedia>(this),
                     &PeerConnectionMedia::BeginIceRestart_s,
                     new_ctx),
                 NS_DISPATCH_NORMAL);
+
+  mIceRestartState = ICE_RESTART_PROVISIONAL;
 }
 
 void
 PeerConnectionMedia::BeginIceRestart_s(RefPtr<NrIceCtx> new_ctx)
 {
   ASSERT_ON_THREAD(mSTSThread);
 
   // hold the original context so we can disconnect signals if needed
   RefPtr<NrIceCtx> originalCtx = mIceCtxHdlr->ctx();
 
-  mIceCtxHdlr->BeginIceRestart(new_ctx);
-  if (mIceCtxHdlr->IsRestarting()) {
+  if (mIceCtxHdlr->BeginIceRestart(new_ctx)) {
     ConnectSignals(mIceCtxHdlr->ctx().get(), originalCtx.get());
   }
 }
 
 void
+PeerConnectionMedia::CommitIceRestart()
+{
+  ASSERT_ON_THREAD(mMainThread);
+  if (mIceRestartState != ICE_RESTART_PROVISIONAL) {
+    return;
+  }
+
+  mIceRestartState = ICE_RESTART_COMMITTED;
+}
+
+void
 PeerConnectionMedia::FinalizeIceRestart()
 {
   ASSERT_ON_THREAD(mMainThread);
+  if (!IsIceRestarting()) {
+    return;
+  }
 
   RUN_ON_THREAD(GetSTSThread(),
                 WrapRunnable(
                     RefPtr<PeerConnectionMedia>(this),
                     &PeerConnectionMedia::FinalizeIceRestart_s),
                 NS_DISPATCH_NORMAL);
+
+  mIceRestartState = ICE_RESTART_NONE;
 }
 
 void
 PeerConnectionMedia::FinalizeIceRestart_s()
 {
   ASSERT_ON_THREAD(mSTSThread);
 
+  // reset old streams since we don't need them anymore
+  for (auto i = mTransportFlows.begin();
+       i != mTransportFlows.end();
+       ++i) {
+    RefPtr<TransportFlow> aFlow = i->second;
+    if (!aFlow) continue;
+    TransportLayerIce* ice =
+      static_cast<TransportLayerIce*>(aFlow->GetLayer(TransportLayerIce::ID()));
+    ice->ResetOldStream();
+  }
+
   mIceCtxHdlr->FinalizeIceRestart();
 }
 
 void
 PeerConnectionMedia::RollbackIceRestart()
 {
   ASSERT_ON_THREAD(mMainThread);
+  if (mIceRestartState != ICE_RESTART_PROVISIONAL) {
+    return;
+  }
 
   RUN_ON_THREAD(GetSTSThread(),
                 WrapRunnable(
                     RefPtr<PeerConnectionMedia>(this),
                     &PeerConnectionMedia::RollbackIceRestart_s),
                 NS_DISPATCH_NORMAL);
 }
 
 void
 PeerConnectionMedia::RollbackIceRestart_s()
 {
   ASSERT_ON_THREAD(mSTSThread);
-  if (!mIceCtxHdlr->IsRestarting()) {
-    return;
-  }
 
   // hold the restart context so we can disconnect signals
   RefPtr<NrIceCtx> restartCtx = mIceCtxHdlr->ctx();
 
+  // restore old streams since we're rolling back
+  for (auto i = mTransportFlows.begin();
+       i != mTransportFlows.end();
+       ++i) {
+    RefPtr<TransportFlow> aFlow = i->second;
+    if (!aFlow) continue;
+    TransportLayerIce* ice =
+      static_cast<TransportLayerIce*>(aFlow->GetLayer(TransportLayerIce::ID()));
+    ice->RestoreOldStream();
+  }
+
   mIceCtxHdlr->RollbackIceRestart();
   ConnectSignals(mIceCtxHdlr->ctx().get(), restartCtx.get());
 }
 
 bool
 PeerConnectionMedia::GetPrefDefaultAddressOnly() const
 {
   ASSERT_ON_THREAD(mMainThread); // will crash on STS thread
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
@@ -222,16 +222,21 @@ class PeerConnectionMedia : public sigsl
   ~PeerConnectionMedia()
   {
     MOZ_RELEASE_ASSERT(!mMainThread);
   }
 
  public:
   explicit PeerConnectionMedia(PeerConnectionImpl *parent);
 
+  enum IceRestartState { ICE_RESTART_NONE,
+                         ICE_RESTART_PROVISIONAL,
+                         ICE_RESTART_COMMITTED
+  };
+
   PeerConnectionImpl* GetPC() { return mParent; }
   nsresult Init(const std::vector<NrIceStunServer>& stun_servers,
                 const std::vector<NrIceTurnServer>& turn_servers,
                 NrIceCtx::Policy policy);
   // WARNING: This destroys the object!
   void SelfDestruct();
 
   RefPtr<NrIceCtxHandler> ice_ctx_hdlr() const { return mIceCtxHdlr; }
@@ -250,19 +255,24 @@ class PeerConnectionMedia : public sigsl
 
   // Activate or remove ICE transports at the conclusion of offer/answer,
   // or when rollback occurs.
   void ActivateOrRemoveTransports(const JsepSession& aSession);
 
   // Start ICE checks.
   void StartIceChecks(const JsepSession& session);
 
+  bool IsIceRestarting() const;
+  IceRestartState GetIceRestartState() const;
+
   // Begin ICE restart
   void BeginIceRestart(const std::string& ufrag,
                        const std::string& pwd);
+  // Commit ICE Restart - offer/answer complete, no rollback possible
+  void CommitIceRestart();
   // Finalize ICE restart
   void FinalizeIceRestart();
   // Abort ICE restart
   void RollbackIceRestart();
 
   // Process a trickle ICE candidate.
   void AddIceCandidate(const std::string& candidate, const std::string& mid,
                        uint32_t aMLine);
@@ -542,14 +552,17 @@ class PeerConnectionMedia : public sigsl
   nsCOMPtr<nsICancelable> mProxyRequest;
 
   // Used to track the state of the request.
   bool mProxyResolveCompleted;
 
   // Used to store the result of the request.
   UniquePtr<NrIceProxyServer> mProxyServer;
 
+  // Used to track the state of ice restart
+  IceRestartState mIceRestartState;
+
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(PeerConnectionMedia)
 };
 
 } // namespace mozilla
 
 #endif