Bug 906986 - Fixing more review issues. draft
authorMichael Froman <mfroman@mozilla.com>
Wed, 30 Mar 2016 13:24:36 -0500
changeset 346086 99316c069863bff12b926bd2d3c3565bd5270482
parent 345976 5c4919e83307e739f1d76f8849cbf9435f94380f
child 517318 312c0dbaba81c0cb37ddd5236282c4a03fec2cd0
push id14233
push usermfroman@nostrum.com
push dateWed, 30 Mar 2016 21:31:32 +0000
bugs906986
milestone47.0a1
Bug 906986 - Fixing more review issues. MozReview-Commit-ID: GX2M1nZxO6N
dom/media/tests/mochitest/mochitest.ini
media/mtransport/nricectxhandler.h
media/mtransport/test/ice_unittest.cpp
media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
media/mtransport/transportlayerice.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
--- a/dom/media/tests/mochitest/mochitest.ini
+++ b/dom/media/tests/mochitest/mochitest.ini
@@ -134,25 +134,25 @@ skip-if = toolkit == 'gonk' # B2G emulat
 [test_peerConnection_offerRequiresReceiveAudio.html]
 [test_peerConnection_offerRequiresReceiveVideo.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
 [test_peerConnection_offerRequiresReceiveVideoAudio.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' || (android_version == '18' && debug) # b2g(Bug 960442, video support for WebRTC is disabled on b2g), android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_promiseSendOnly.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' || (android_version == '18' && debug) # b2g(Bug 960442, video support for WebRTC is disabled on b2g), android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_restartIce.html]
-skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867)
+skip-if = toolkit == 'gonk' || buildapp == 'mulet' || android_version # b2g (Bug 1059867)
 [test_peerConnection_restartIceNoBundle.html]
-skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867)
+skip-if = toolkit == 'gonk' || buildapp == 'mulet' || android_version # b2g (Bug 1059867)
 [test_peerConnection_restartIceNoRtcpMux.html]
-skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867)
+skip-if = toolkit == 'gonk' || buildapp == 'mulet' || android_version # b2g (Bug 1059867)
 [test_peerConnection_restartIceLocalRollback.html]
-skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867)
+skip-if = toolkit == 'gonk' || buildapp == 'mulet' || android_version # b2g (Bug 1059867)
 [test_peerConnection_restartIceLocalAndRemoteRollback.html]
-skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867)
+skip-if = toolkit == 'gonk' || buildapp == 'mulet' || android_version # b2g (Bug 1059867)
 [test_peerConnection_simulcastOffer.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' || android_version  || (os == 'linux' && debug) # b2g(Bug 960442, video support for WebRTC is disabled on b2g), no simulcast support on android, linux (bug 1242058)
 #[test_peerConnection_relayOnly.html]
 #skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video support for WebRTC is disabled on b2g)
 [test_peerConnection_callbacks.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' || (android_version == '18' && debug) # b2g(Bug 960442, video support for WebRTC is disabled on b2g), android(Bug 1189784, timeouts on 4.3 emulator)
 [test_peerConnection_replaceTrack.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' || android_version == '18' # b2g(Bug 960442, video support for WebRTC is disabled on b2g), android(Bug 1189784, timeouts on 4.3 emulator)
--- a/media/mtransport/nricectxhandler.h
+++ b/media/mtransport/nricectxhandler.h
@@ -9,17 +9,17 @@ class NrIceCtxHandler {
 public:
   // TODO(ekr@rtfm.com): Too many bools here. Bug 1193437.
   static RefPtr<NrIceCtxHandler> Create(const std::string& name,
                                  bool offerer,
                                  bool allow_loopback = false,
                                  bool tcp_enabled = true,
                                  bool allow_link_local = false,
                                  bool hide_non_default = false,
-                                 NrIceCtx::Policy policy = 
+                                 NrIceCtx::Policy policy =
                                    NrIceCtx::ICE_POLICY_ALL);
 
   RefPtr<NrIceMediaStream> CreateStream(const std::string& name,
                                         int components);
 
   RefPtr<NrIceCtx> ctx() { return current_ctx; }
 
   bool BeginIceRestart(); // used in testing
--- a/media/mtransport/test/ice_unittest.cpp
+++ b/media/mtransport/test/ice_unittest.cpp
@@ -284,17 +284,17 @@ class IceTestPeer : public sigslot::has_
  public:
   // TODO(ekr@rtfm.com): Convert to flags when NrIceCtx::Create() does.
   // Bug 1193437.
   IceTestPeer(const std::string& name, bool offerer,
               bool allow_loopback = false, bool enable_tcp = true,
               bool allow_link_local = false, bool hide_non_default = false) :
       name_(name),
       ice_ctx_(NrIceCtxHandler::Create(name, offerer, allow_loopback,
-                                       enable_tcp, allow_link_local, 
+                                       enable_tcp, allow_link_local,
                                        hide_non_default)),
       candidates_(),
       gathering_complete_(false),
       ready_ct_(0),
       ice_complete_(false),
       ice_reached_checking_(false),
       received_(0),
       sent_(0),
@@ -1158,17 +1158,17 @@ class IceTestPeer : public sigslot::has_
                         &IceTestPeer::ParseCandidate_s,
                         i,
                         candidate),
         NS_DISPATCH_SYNC);
   }
 
   void DisableComponent_s(size_t stream, int component_id) {
     ASSERT_LT(stream, ice_ctx_->ctx()->GetStreamCount());
-    ASSERT_TRUE(ice_ctx_->ctx()->GetStream(stream).get()) << "No such stream " 
+    ASSERT_TRUE(ice_ctx_->ctx()->GetStream(stream).get()) << "No such stream "
                                                           << stream;
     nsresult res =
       ice_ctx_->ctx()->GetStream(stream)->DisableComponent(component_id);
     ASSERT_TRUE(NS_SUCCEEDED(res));
   }
 
   void DisableComponent(size_t stream, int component_id)
   {
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
@@ -52,16 +52,18 @@ static char *RCSSTRING __UNUSED__="$Id: 
 #include "stun.h"
 #include "ice_ctx.h"
 #include "ice_reg.h"
 #include "nr_crypto.h"
 #include "async_timer.h"
 #include "util.h"
 #include "nr_socket_local.h"
 
+#define ICE_UFRAG_LEN 8
+#define ICE_PWD_LEN 32
 
 int LOG_ICE = 0;
 
 static int nr_ice_random_string(char *str, int len);
 static int nr_ice_fetch_stun_servers(int ct, nr_ice_stun_server **out);
 #ifdef USE_TURN
 static int nr_ice_fetch_turn_servers(int ct, nr_ice_turn_server **out);
 #endif /* USE_TURN */
@@ -968,17 +970,16 @@ int nr_ice_ctx_hide_candidate(nr_ice_ctx
     if (ctx->flags & NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS) {
       if (cand->type == HOST)
         return 1;
     }
 
     return 0;
   }
 
-#define ICE_UFRAG_LEN 8
 int nr_ice_get_new_ice_ufrag(char** ufrag)
   {
     int r,_status;
     char buf[ICE_UFRAG_LEN+1];
 
     if(r=nr_ice_random_string(buf,ICE_UFRAG_LEN))
       ABORT(r);
     if(!(*ufrag=r_strdup(buf)))
@@ -988,17 +989,16 @@ int nr_ice_get_new_ice_ufrag(char** ufra
   abort:
     if(_status) {
       RFREE(*ufrag);
       *ufrag = 0;
     }
     return(_status);
   }
 
-#define ICE_PWD_LEN 32
 int nr_ice_get_new_ice_pwd(char** pwd)
   {
     int r,_status;
     char buf[ICE_PWD_LEN+1];
 
     if(r=nr_ice_random_string(buf,ICE_PWD_LEN))
       ABORT(r);
     if(!(*pwd=r_strdup(buf)))
--- a/media/mtransport/transportlayerice.cpp
+++ b/media/mtransport/transportlayerice.cpp
@@ -159,16 +159,19 @@ void TransportLayerIce::RestoreOldStream
   stream_ = old_stream_;
   old_stream_ = nullptr;
 
   if (stream_->state() == NrIceMediaStream::ICE_OPEN) {
     IceReady(stream_);
   } else if (stream_->state() == NrIceMediaStream::ICE_CLOSED) {
     IceFailed(stream_);
   }
+  // No events are fired when the stream is ICE_CONNECTING.  If the
+  // restored stream is ICE_CONNECTING, IceReady/IceFailed will fire
+  // later.
 }
 
 TransportResult TransportLayerIce::SendPacket(const unsigned char *data,
                                               size_t len) {
   CheckThread();
   // use old_stream_ until stream_ is ready
   nsresult res = (old_stream_?old_stream_:stream_)->SendPacket(component_,
                                                                data,
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -1523,16 +1523,20 @@ 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()) {
+    RollbackIceRestart();
+  }
 
   RefPtr<PeerConnectionObserver> pco = do_QueryObjectReferent(mPCObserver);
   if (!pco) {
     return NS_OK;
   }
 
   if (!PeerConnectionCtx::GetInstance()->isReady()) {
     // Uh oh. We're not ready yet. Enqueue this operation.
@@ -1540,17 +1544,18 @@ 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)) {
+  if (aOptions.mIceRestart.isSome() && *(aOptions.mIceRestart) &&
+      !mMedia->ice_ctx_hdlr()->IsRestarting()) {
     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;
     }
@@ -1674,16 +1679,35 @@ PeerConnectionImpl::SetupIceRestart()
                          __FUNCTION__,
                          static_cast<unsigned>(nrv));
     return nrv;
   }
 
   return NS_OK;
 }
 
+nsresult
+PeerConnectionImpl::RollbackIceRestart()
+{
+  mMedia->RollbackIceRestart();
+  // put back the previous ice creds
+  nsresult nrv = mJsepSession->SetIceCredentials(mPreviousIceUfrag,
+                                                 mPreviousIcePwd);
+  if (NS_FAILED(nrv)) {
+    CSFLogError(logTag, "%s: Couldn't set ICE credentials, res=%u",
+                         __FUNCTION__,
+                         static_cast<unsigned>(nrv));
+    return nrv;
+  }
+  mPreviousIceUfrag = "";
+  mPreviousIcePwd = "";
+
+  return NS_OK;
+}
+
 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;
@@ -2800,21 +2824,17 @@ PeerConnectionImpl::SetSignalingState_m(
     mMedia->EnsureTransports(*mJsepSession);
   }
 
   mSignalingState = aSignalingState;
 
   bool fireNegotiationNeeded = false;
   if (mSignalingState == PCImplSignalingState::SignalingStable) {
     if (rollback && mMedia->ice_ctx_hdlr()->IsRestarting()) {
-      mMedia->RollbackIceRestart();
-      // put back the previous ice creds
-      mJsepSession->SetIceCredentials(mPreviousIceUfrag, mPreviousIcePwd);
-      mPreviousIceUfrag = "";
-      mPreviousIcePwd = "";
+      RollbackIceRestart();
     }
 
     // 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);
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ -713,16 +713,17 @@ private:
                                             const std::string& streamId,
                                             const std::string& trackId);
 
   nsresult AddTrackToJsepSession(SdpMediaSection::MediaType type,
                                  const std::string& streamId,
                                  const std::string& trackId);
 
   nsresult SetupIceRestart();
+  nsresult RollbackIceRestart();
 
 #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
@@ -353,17 +353,17 @@ nsresult PeerConnectionMedia::Init(const
     }
   } else if (turn_servers.size() != 0) {
     CSFLogError(logTag, "%s: Setting turn servers disabled", __FUNCTION__);
   }
   if (NS_FAILED(rv = mDNSResolver->Init())) {
     CSFLogError(logTag, "%s: Failed to initialize dns resolver", __FUNCTION__);
     return rv;
   }
-  if (NS_FAILED(rv = 
+  if (NS_FAILED(rv =
       mIceCtxHdlr->ctx()->SetResolver(mDNSResolver->AllocateResolver()))) {
     CSFLogError(logTag, "%s: Failed to get dns resolver", __FUNCTION__);
     return rv;
   }
   ConnectSignals(mIceCtxHdlr->ctx().get());
 
   return NS_OK;
 }
@@ -393,17 +393,17 @@ PeerConnectionMedia::EnsureTransport_s(s
   if (!stream) {
     CSFLogDebug(logTag, "%s: Creating ICE media stream=%u components=%u",
                 mParentHandle.c_str(),
                 static_cast<unsigned>(aLevel),
                 static_cast<unsigned>(aComponentCount));
 
     std::ostringstream os;
     os << mParentName << " aLevel=" << aLevel;
-    RefPtr<NrIceMediaStream> stream = 
+    RefPtr<NrIceMediaStream> stream =
       mIceCtxHdlr->CreateStream(os.str().c_str(),
                                 aComponentCount);
 
     if (!stream) {
       CSFLogError(logTag, "Failed to create ICE stream.");
       return;
     }