Backed out changesets 8ed054e5853e,d56adef9c8e0,408df9f84697,7092e498ac3a,29dce05596c6,ae1dc75009e7 (bug 1231975) for breaking local mochitests a=backout
authorWes Kocher <wkocher@mozilla.com>
Wed, 27 Apr 2016 10:20:42 -0700
changeset 334048 a8d0cda0ef764a00b9ca133878581109698c295f
parent 334047 ab0044bfa1df858919797bcd6a9aef76a668cd4a
child 334049 86730d0a82093d705e44f33a34973d28b269f1ea
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1231975
milestone49.0a1
backs out8ed054e5853e06a05e1da980627253c2cabd319a
d56adef9c8e00ba6d07299941df1e5b3ff03ba01
408df9f8469753cb2fcc78a2cf5ef4a604101d77
7092e498ac3af20998b35a7cadfff4c03d396378
29dce05596c6c854af6f2e71fdfde138033ac1a3
ae1dc75009e73744bc3686a427d985e4e59dd24f
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
Backed out changesets 8ed054e5853e,d56adef9c8e0,408df9f84697,7092e498ac3a,29dce05596c6,ae1dc75009e7 (bug 1231975) for breaking local mochitests a=backout MozReview-Commit-ID: 17mJDyZMVju
dom/media/tests/mochitest/mochitest.ini
dom/media/tests/mochitest/pc.js
dom/media/tests/mochitest/test_peerConnection_basicAudioNATRelay.html
dom/media/tests/mochitest/test_peerConnection_basicAudioNATRelayTCP.html
dom/media/tests/mochitest/test_peerConnection_basicAudioNATSrflx.html
dom/network/TCPSocketChild.cpp
dom/network/TCPSocketParent.cpp
media/mtransport/nr_socket_prsock.cpp
media/mtransport/nricectx.cpp
media/mtransport/nriceresolver.cpp
media/mtransport/nriceresolver.h
media/mtransport/test_nr_socket.cpp
media/mtransport/test_nr_socket.h
--- a/dom/media/tests/mochitest/mochitest.ini
+++ b/dom/media/tests/mochitest/mochitest.ini
@@ -79,22 +79,16 @@ skip-if = (toolkit == 'gonk' || buildapp
 [test_getUserMedia_stopVideoStream.html]
 [test_getUserMedia_stopVideoStreamWithFollowupVideo.html]
 [test_getUserMedia_peerIdentity.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 1021776, too --ing slow on b2g)
 [test_peerConnection_addIceCandidate.html]
 [test_peerConnection_addTrack.html]
 [test_peerConnection_basicAudio.html]
 skip-if = toolkit == 'gonk' # B2G emulator is too slow to handle a two-way audio call reliably
-[test_peerConnection_basicAudioNATSrflx.html]
-skip-if = toolkit == 'gonk' || toolkit == 'android' # B2G emulator is too slow to handle a two-way audio call reliably, websockets don't work on android (bug 1266217)
-[test_peerConnection_basicAudioNATRelay.html]
-skip-if = toolkit == 'gonk' || toolkit == 'android' # B2G emulator is too slow to handle a two-way audio call reliably, websockets don't work on android (bug 1266217)
-[test_peerConnection_basicAudioNATRelayTCP.html]
-skip-if = toolkit == 'gonk' || toolkit == 'android' # B2G emulator is too slow to handle a two-way audio call reliably, websockets don't work on android (bug 1266217)
 [test_peerConnection_basicAudioRequireEOC.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867)
 [test_peerConnection_basicAudioPcmaPcmuOnly.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867)
 [test_peerConnection_basicAudioDynamicPtMissingRtpmap.html]
 skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867)
 [test_peerConnection_basicAudioVideo.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)
--- a/dom/media/tests/mochitest/pc.js
+++ b/dom/media/tests/mochitest/pc.js
@@ -60,24 +60,20 @@ function PeerConnectionTest(options) {
   options.is_remote = "is_remote" in options ? options.is_remote : true;
 
   options.h264 = "h264" in options ? options.h264 : false;
   options.bundle = "bundle" in options ? options.bundle : true;
   options.rtcpmux = "rtcpmux" in options ? options.rtcpmux : true;
   options.opus = "opus" in options ? options.opus : true;
 
   if (iceServersArray.length) {
-    if (!options.turn_disabled_local) {
-      options.config_local = options.config_local || {}
-      options.config_local.iceServers = iceServersArray;
-    }
-    if (!options.turn_disabled_remote) {
-      options.config_remote = options.config_remote || {}
-      options.config_remote.iceServers = iceServersArray;
-    }
+    options.config_remote = options.config_remote || {}
+    options.config_local = options.config_local || {}
+    options.config_remote.iceServers = iceServersArray;
+    options.config_local.iceServers = iceServersArray;
   }
   else if (typeof turnServers !== "undefined") {
     if ((!options.turn_disabled_local) && (turnServers.local)) {
       if (!options.hasOwnProperty("config_local")) {
         options.config_local = {};
       }
       if (!options.config_local.hasOwnProperty("iceServers")) {
         options.config_local.iceServers = turnServers.local.iceServers;
deleted file mode 100644
--- a/dom/media/tests/mochitest/test_peerConnection_basicAudioNATRelay.html
+++ /dev/null
@@ -1,37 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <script type="application/javascript" src="pc.js"></script>
-</head>
-<body>
-<pre id="test">
-<script type="application/javascript">
-  createHTML({
-    bug: "1231975",
-    title: "Basic audio-only peer connection with port dependent NAT"
-  });
-
-  var test;
-  runNetworkTest(options => {
-    SpecialPowers.pushPrefEnv(
-      {
-        'set': [
-          ['media.peerconnection.nat_simulator.filtering_type', 'PORT_DEPENDENT'],
-          ['media.peerconnection.nat_simulator.mapping_type', 'PORT_DEPENDENT']
-        ]
-      }, function (options) {
-        options = options || {};
-        options.expectedLocalCandidateType = "serverreflexive";
-        options.expectedRemoteCandidateType = "relayed";
-        // If both have TURN, it is a toss-up which one will end up using a
-        // relay.
-        options.turn_disabled_local = true;
-        test = new PeerConnectionTest(options);
-        test.setMediaConstraints([{audio: true}], [{audio: true}]);
-        test.run();
-      })
-  }, { useIceServer: true });
-</script>
-</pre>
-</body>
-</html>
deleted file mode 100644
--- a/dom/media/tests/mochitest/test_peerConnection_basicAudioNATRelayTCP.html
+++ /dev/null
@@ -1,35 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <script type="application/javascript" src="pc.js"></script>
-</head>
-<body>
-<pre id="test">
-<script type="application/javascript">
-  createHTML({
-    bug: "1231975",
-    title: "Basic audio-only peer connection with port dependent NAT that blocks UDP"
-  });
-
-  var test;
-  runNetworkTest(options => {
-    SpecialPowers.pushPrefEnv(
-      {
-        'set': [
-          ['media.peerconnection.nat_simulator.filtering_type', 'PORT_DEPENDENT'],
-          ['media.peerconnection.nat_simulator.mapping_type', 'PORT_DEPENDENT'],
-          ['media.peerconnection.nat_simulator.block_udp', true]
-        ]
-      }, function (options) {
-        options = options || {};
-        options.expectedLocalCandidateType = "relayed-tcp";
-        options.expectedRemoteCandidateType = "relayed-tcp";
-        test = new PeerConnectionTest(options);
-        test.setMediaConstraints([{audio: true}], [{audio: true}]);
-        test.run();
-      })
-  }, { useIceServer: true });
-</script>
-</pre>
-</body>
-</html>
deleted file mode 100644
--- a/dom/media/tests/mochitest/test_peerConnection_basicAudioNATSrflx.html
+++ /dev/null
@@ -1,34 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <script type="application/javascript" src="pc.js"></script>
-</head>
-<body>
-<pre id="test">
-<script type="application/javascript">
-  createHTML({
-    bug: "1231975",
-    title: "Basic audio-only peer connection with endpoint independent NAT"
-  });
-
-  var test;
-  runNetworkTest(options => {
-    SpecialPowers.pushPrefEnv(
-      {
-        'set': [
-          ['media.peerconnection.nat_simulator.filtering_type', 'ENDPOINT_INDEPENDENT'],
-          ['media.peerconnection.nat_simulator.mapping_type', 'ENDPOINT_INDEPENDENT']
-        ]
-      }, function (options) {
-        options = options || {};
-        options.expectedLocalCandidateType = "serverreflexive";
-        options.expectedRemoteCandidateType = "serverreflexive";
-        test = new PeerConnectionTest(options);
-        test.setMediaConstraints([{audio: true}], [{audio: true}]);
-        test.run();
-      })
-  }, { useIceServer: true });
-</script>
-</pre>
-</body>
-</html>
--- a/dom/network/TCPSocketChild.cpp
+++ b/dom/network/TCPSocketChild.cpp
@@ -122,17 +122,16 @@ TCPSocketChild::SendWindowlessOpenBind(n
                                 aUseSSL, true, mFilterName);
 }
 
 void
 TCPSocketChildBase::ReleaseIPDLReference()
 {
   MOZ_ASSERT(mIPCOpen);
   mIPCOpen = false;
-  mSocket = nullptr;
   this->Release();
 }
 
 void
 TCPSocketChildBase::AddIPDLReference()
 {
   MOZ_ASSERT(!mIPCOpen);
   mIPCOpen = true;
--- a/dom/network/TCPSocketParent.cpp
+++ b/dom/network/TCPSocketParent.cpp
@@ -335,17 +335,17 @@ TCPSocketParent::RecvData(const Sendable
       const nsCString& strData = aData.get_nsCString();
       mSocket->SendWithTrackingNumber(strData, aTrackingNumber, rv);
       break;
     }
 
     default:
       MOZ_CRASH("unexpected SendableData type");
   }
-  NS_ENSURE_SUCCESS(rv.StealNSResult(), true);
+  NS_ENSURE_FALSE(rv.Failed(), true);
   return true;
 }
 
 bool
 TCPSocketParent::RecvClose()
 {
   NS_ENSURE_TRUE(mSocket, true);
   mSocket->Close();
--- a/media/mtransport/nr_socket_prsock.cpp
+++ b/media/mtransport/nr_socket_prsock.cpp
@@ -733,18 +733,17 @@ int NrSocket::create(nr_transport_addr *
   // Remember our thread.
   ststhread_ = do_QueryInterface(stservice, &rv);
   if (!NS_SUCCEEDED(rv))
     ABORT(R_INTERNAL);
 
   // Finally, register with the STS
   rv = stservice->AttachSocket(fd_, this);
   if (!NS_SUCCEEDED(rv)) {
-    r_log(LOG_GENERIC, LOG_CRIT, "Couldn't attach socket to STS, rv=%u",
-          static_cast<unsigned>(rv));
+    r_log(LOG_GENERIC, LOG_CRIT, "Couldn't attach socket to STS");
     ABORT(R_INTERNAL);
   }
 
   _status = 0;
 
 abort:
   return(_status);
 }
--- a/media/mtransport/nricectx.cpp
+++ b/media/mtransport/nricectx.cpp
@@ -571,46 +571,46 @@ NrIceCtx::Initialize(bool hide_non_defau
   if (generating_trickle()) {
     r = nr_ice_ctx_set_trickle_cb(ctx_, &NrIceCtx::trickle_cb, this);
     if (r) {
       MOZ_MTLOG(ML_ERROR, "Couldn't set trickle cb for '" << name_ << "'");
       return false;
     }
   }
 
-  nsCString mapping_type;
-  nsCString filtering_type;
+  char* mapping_type = nullptr;
+  char* filtering_type = nullptr;
   bool block_udp = false;
 
   nsresult rv;
   nsCOMPtr<nsIPrefService> pref_service =
     do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
 
   if (NS_SUCCEEDED(rv)) {
     nsCOMPtr<nsIPrefBranch> pref_branch;
     rv = pref_service->GetBranch(nullptr, getter_AddRefs(pref_branch));
     if (NS_SUCCEEDED(rv)) {
       rv = pref_branch->GetCharPref(
           "media.peerconnection.nat_simulator.mapping_type",
-          getter_Copies(mapping_type));
+          &mapping_type);
       rv = pref_branch->GetCharPref(
           "media.peerconnection.nat_simulator.filtering_type",
-          getter_Copies(filtering_type));
+          &filtering_type);
       rv = pref_branch->GetBoolPref(
           "media.peerconnection.nat_simulator.block_udp",
           &block_udp);
     }
   }
 
-  if (!mapping_type.IsEmpty() && !filtering_type.IsEmpty()) {
-    MOZ_MTLOG(ML_DEBUG, "NAT filtering type: " << filtering_type.get());
-    MOZ_MTLOG(ML_DEBUG, "NAT mapping type: " << mapping_type.get());
+  if (mapping_type && filtering_type) {
+    MOZ_MTLOG(ML_DEBUG, "NAT filtering type: " << filtering_type);
+    MOZ_MTLOG(ML_DEBUG, "NAT mapping type: " << mapping_type);
     TestNat* test_nat = new TestNat;
-    test_nat->filtering_type_ = TestNat::ToNatBehavior(filtering_type.get());
-    test_nat->mapping_type_ = TestNat::ToNatBehavior(mapping_type.get());
+    test_nat->filtering_type_ = TestNat::ToNatBehavior(filtering_type);
+    test_nat->mapping_type_ = TestNat::ToNatBehavior(mapping_type);
     test_nat->block_udp_ = block_udp;
     test_nat->enabled_ = true;
     SetNat(test_nat);
   }
 
   // Create the handler objects
   ice_handler_vtbl_ = new nr_ice_handler_vtbl();
   ice_handler_vtbl_->select_pair = &NrIceCtx::select_pair;
--- a/media/mtransport/nriceresolver.cpp
+++ b/media/mtransport/nriceresolver.cpp
@@ -194,43 +194,42 @@ abort:
   return _status;
 }
 
 nsresult NrIceResolver::PendingResolution::OnLookupComplete(
     nsICancelable *request, nsIDNSRecord *record, nsresult status) {
   ASSERT_ON_THREAD(thread_);
   // First check if we've been canceled. This is single-threaded on the STS
   // thread, but cancel() cannot guarantee this event isn't on the queue.
-  if (request_) {
+  if (!canceled_) {
     nr_transport_addr *cb_addr = nullptr;
     nr_transport_addr ta;
     // TODO(jib@mozilla.com): Revisit when we do TURN.
     if (NS_SUCCEEDED(status)) {
       net::NetAddr na;
       if (NS_SUCCEEDED(record->GetNextAddr(port_, &na))) {
         MOZ_ALWAYS_TRUE (nr_netaddr_to_transport_addr(&na, &ta,
                                                       transport_) == 0);
         cb_addr = &ta;
       }
     }
     cb_(cb_arg_, cb_addr);
-    request_ = nullptr;
     Release();
   }
   return NS_OK;
 }
 
 int NrIceResolver::cancel(void *obj, void *handle) {
   MOZ_ALWAYS_TRUE(obj);
   MOZ_ASSERT(handle);
   ASSERT_ON_THREAD(static_cast<NrIceResolver *>(obj)->sts_thread_);
   return static_cast<PendingResolution *>(handle)->cancel();
 }
 
 int NrIceResolver::PendingResolution::cancel() {
   request_->Cancel (NS_ERROR_ABORT);
-  request_ = nullptr;
+  canceled_ = true; // in case OnLookupComplete is already on event queue.
   Release();
   return 0;
 }
 
 NS_IMPL_ISUPPORTS(NrIceResolver::PendingResolution, nsIDNSListener);
 }  // End of namespace mozilla
--- a/media/mtransport/nriceresolver.h
+++ b/media/mtransport/nriceresolver.h
@@ -90,30 +90,32 @@ class NrIceResolver
     PendingResolution(nsIEventTarget *thread,
                       uint16_t port,
                       int transport,
                       int (*cb)(void *cb_arg, nr_transport_addr *addr),
                       void *cb_arg) :
         thread_(thread),
         port_(port),
         transport_(transport),
-        cb_(cb), cb_arg_(cb_arg) {}
+        cb_(cb), cb_arg_(cb_arg),
+        canceled_ (false) {}
     NS_IMETHOD OnLookupComplete(nsICancelable *request, nsIDNSRecord *record,
                                 nsresult status) override;
     int cancel();
     nsCOMPtr<nsICancelable> request_;
     NS_DECL_THREADSAFE_ISUPPORTS
 
    private:
     virtual ~PendingResolution(){};
     nsCOMPtr<nsIEventTarget> thread_;
     uint16_t port_;
     int transport_;
     int (*cb_)(void *cb_arg, nr_transport_addr *addr);
     void *cb_arg_;
+    bool canceled_;
   };
 
   nr_resolver_vtbl* vtbl_;
   nsCOMPtr<nsIEventTarget> sts_thread_;
   nsCOMPtr<nsIDNSService> dns_;
 #ifdef DEBUG
   int allocated_resolvers_;
 #endif
--- a/media/mtransport/test_nr_socket.cpp
+++ b/media/mtransport/test_nr_socket.cpp
@@ -253,20 +253,18 @@ int TestNrSocket::getaddr(nr_transport_a
   return internal_socket_->getaddr(addrp);
 }
 
 void TestNrSocket::close() {
   if (timer_handle_) {
     NR_async_timer_cancel(timer_handle_);
     timer_handle_ = 0;
   }
+  // TODO: close port mappings too?
   internal_socket_->close();
-  for (RefPtr<PortMapping>& port_mapping : port_mappings_) {
-    port_mapping->external_socket_->close();
-  }
 }
 
 int TestNrSocket::listen(int backlog) {
   MOZ_ASSERT(internal_socket_->my_addr().protocol == IPPROTO_TCP);
   r_log(LOG_GENERIC, LOG_DEBUG,
         "TestNrSocket %s listening",
         internal_socket_->my_addr().as_string);
 
@@ -381,45 +379,35 @@ int TestNrSocket::recvfrom(void *buf, si
 
   if (readable_socket_) {
     // If any of the external sockets got data, see if it will be passed through
     r = readable_socket_->recvfrom(buf, maxlen, len, 0, from);
     readable_socket_ = nullptr;
     if (!r) {
       PortMapping *port_mapping_used;
       ingress_allowed = allow_ingress(*from, &port_mapping_used);
-      if (ingress_allowed) {
-        r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s received from %s via %s",
-              internal_socket_->my_addr().as_string,
-              from->as_string,
-              port_mapping_used->external_socket_->my_addr().as_string);
-        if (nat_->refresh_on_ingress_) {
-          port_mapping_used->last_used_ = PR_IntervalNow();
-        }
+      if (ingress_allowed && nat_->refresh_on_ingress_ && port_mapping_used) {
+        port_mapping_used->last_used_ = PR_IntervalNow();
       }
     }
   } else {
     // If no external socket has data, see if there's any data that was sent
     // directly to the TestNrSocket, and eat it if it isn't supposed to get
     // through.
     r = internal_socket_->recvfrom(buf, maxlen, len, flags, from);
     if (!r) {
       // We do not use allow_ingress() here because that only handles traffic
       // landing on an external port.
       ingress_allowed = (!nat_->enabled_ ||
                          nat_->is_an_internal_tuple(*from));
       if (!ingress_allowed) {
         r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s denying ingress from %s: "
-              "Not behind the same NAT",
-              internal_socket_->my_addr().as_string,
-              from->as_string);
-      } else {
-        r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s received from %s",
-              internal_socket_->my_addr().as_string,
-              from->as_string);
+                                     "Not behind the same NAT",
+                                     internal_socket_->my_addr().as_string,
+                                     from->as_string);
       }
     }
   }
 
   // Kinda lame that we are forced to give the app a readable callback and then
   // say "Oh, never mind...", but the alternative is to totally decouple the
   // callbacks from STS and the callbacks the app sets. On the bright side, this
   // speeds up unit tests where we are verifying that ingress is forbidden,
@@ -430,19 +418,22 @@ int TestNrSocket::recvfrom(void *buf, si
     r = R_WOULDBLOCK;
   }
 
   return r;
 }
 
 bool TestNrSocket::allow_ingress(const nr_transport_addr &from,
                                  PortMapping **port_mapping_used) const {
-  // This is only called for traffic arriving at a port mapping
-  MOZ_ASSERT(nat_->enabled_);
-  MOZ_ASSERT(!nat_->is_an_internal_tuple(from));
+  *port_mapping_used = nullptr;
+  if (!nat_->enabled_)
+    return true;
+
+  if (nat_->is_an_internal_tuple(from))
+    return true;
 
   *port_mapping_used = get_port_mapping(from, nat_->filtering_type_);
   if (!(*port_mapping_used)) {
     r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s denying ingress from %s: "
                                  "Filtered",
                                  internal_socket_->my_addr().as_string,
                                  from.as_string);
     return false;
@@ -701,16 +692,20 @@ void TestNrSocket::on_socket_readable(Nr
     readable_socket_ = real_socket;
   }
 
   fire_readable_callback();
 }
 
 void TestNrSocket::fire_readable_callback() {
   MOZ_ASSERT(poll_flags() & PR_POLL_READ);
+  // Stop listening on all real sockets; we will start listening again
+  // if the app starts listening to us again.
+  cancel_port_mapping_async_wait(NR_ASYNC_WAIT_READ);
+  internal_socket_->cancel(NR_ASYNC_WAIT_READ);
   r_log(LOG_GENERIC, LOG_DEBUG, "TestNrSocket %s ready for read",
         internal_socket_->my_addr().as_string);
   fire_callback(NR_ASYNC_WAIT_READ);
 }
 
 void TestNrSocket::port_mapping_writeable_callback(void *ext_sock_v,
                                                    int how,
                                                    void *test_sock_v) {
--- a/media/mtransport/test_nr_socket.h
+++ b/media/mtransport/test_nr_socket.h
@@ -248,19 +248,17 @@ class TestNrSocket : public NrSocketBase
         NS_INLINE_DECL_THREADSAFE_REFCOUNTING(PortMapping);
 
         PRIntervalTime last_used_;
         RefPtr<NrSocketBase> external_socket_;
         // For non-symmetric, most of the data here doesn't matter
         nr_transport_addr remote_address_;
 
       private:
-        ~PortMapping() {
-          external_socket_->close();
-        }
+        ~PortMapping(){}
 
         // If external_socket_ returns E_WOULDBLOCK, we don't want to propagate
         // that to the code using the TestNrSocket. We can also perhaps use this
         // to help simulate things like latency.
         std::list<RefPtr<UdpPacket>> send_queue_;
     };
 
     struct DeferredPacket {