Bug 1304269: lets promise ice from now on. r=bwc
authorNils Ohlmeier [:drno] <drno@ohlmeier.org>
Wed, 21 Sep 2016 16:42:22 -0700
changeset 314921 552477d11cd0dc96218a62a7562dd5fde5821cf9
parent 314920 cf4dc96168558fc9fef2c59e03a7daeb5dd7c069
child 314922 a2cbbd85a818137bba6fee45d2c997e9193201ef
push id30737
push usercbook@mozilla.com
push dateFri, 23 Sep 2016 09:58:27 +0000
treeherdermozilla-central@ce34596c89a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1304269
milestone52.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 1304269: lets promise ice from now on. r=bwc MozReview-Commit-ID: 7Oi4rDbGv09
dom/media/tests/mochitest/pc.js
dom/media/tests/mochitest/templates.js
dom/media/tests/mochitest/test_peerConnection_addDataChannelNoBundle.html
dom/media/tests/mochitest/test_peerConnection_addSecondAudioStreamNoBundle.html
dom/media/tests/mochitest/test_peerConnection_addSecondVideoStreamNoBundle.html
dom/media/tests/mochitest/test_peerConnection_restartIce.html
dom/media/tests/mochitest/test_peerConnection_restartIceLocalAndRemoteRollback.html
dom/media/tests/mochitest/test_peerConnection_restartIceLocalRollback.html
dom/media/tests/mochitest/test_peerConnection_restartIceNoBundle.html
dom/media/tests/mochitest/test_peerConnection_restartIceNoBundleNoRtcpMux.html
dom/media/tests/mochitest/test_peerConnection_restartIceNoRtcpMux.html
--- a/dom/media/tests/mochitest/pc.js
+++ b/dom/media/tests/mochitest/pc.js
@@ -753,35 +753,47 @@ function PeerConnectionWrapper(label, co
   this.audioElementsOnly = false;
 
   this.expectedLocalTrackInfoById = {};
   this.expectedRemoteTrackInfoById = {};
   this.observedRemoteTrackInfoById = {};
 
   this.disableRtpCountChecking = false;
 
+  this.iceConnectedResolve;
+  this.iceConnectedReject;
+  this.iceConnected = new Promise((resolve, reject) => {
+    this.iceConnectedResolve = resolve;
+    this.iceConnectedReject = reject;
+  });
   this.iceCheckingRestartExpected = false;
   this.iceCheckingIceRollbackExpected = false;
 
   info("Creating " + this);
   this._pc = new RTCPeerConnection(this.configuration);
 
   /**
    * Setup callback handlers
    */
   // This allows test to register their own callbacks for ICE connection state changes
   this.ice_connection_callbacks = {};
 
   this._pc.oniceconnectionstatechange = e => {
     isnot(typeof this._pc.iceConnectionState, "undefined",
           "iceConnectionState should not be undefined");
-    info(this + ": oniceconnectionstatechange fired, new state is: " + this._pc.iceConnectionState);
+    var iceState = this._pc.iceConnectionState;
+    info(this + ": oniceconnectionstatechange fired, new state is: " + iceState);
     Object.keys(this.ice_connection_callbacks).forEach(name => {
       this.ice_connection_callbacks[name]();
     });
+    if (iceState === "connected") {
+      this.iceConnectedResolve();
+    } else if (iceState === "failed") {
+      this.iceConnectedReject();
+    }
   };
 
   createOneShotEventWrapper(this, this._pc, 'datachannel');
   this._pc.addEventListener('datachannel', e => {
     var wrapper = new DataChannelWrapper(e.channel, this);
     this.dataChannels.push(wrapper);
   });
 
@@ -1197,55 +1209,16 @@ PeerConnectionWrapper.prototype = {
       // race of the Promises around our test steps.
       // Note: as long as we are queuing ICE candidates until the success
       //       of sRD() this should never ever happen.
       ok(false, this + " adding ICE candidate failed with: " + e.message)
     );
   },
 
   /**
-   * Returns if the ICE the connection state is "connected".
-   *
-   * @returns {boolean} True if the connection state is "connected", otherwise false.
-   */
-  isIceConnected : function() {
-    info(this + ": iceConnectionState = " + this.iceConnectionState);
-    return this.iceConnectionState === "connected";
-  },
-
-  /**
-   * Returns if the ICE the connection state is "checking".
-   *
-   * @returns {boolean} True if the connection state is "checking", otherwise false.
-   */
-  isIceChecking : function() {
-    return this.iceConnectionState === "checking";
-  },
-
-  /**
-   * Returns if the ICE the connection state is "new".
-   *
-   * @returns {boolean} True if the connection state is "new", otherwise false.
-   */
-  isIceNew : function() {
-    return this.iceConnectionState === "new";
-  },
-
-  /**
-   * Checks if the ICE connection state still waits for a connection to get
-   * established.
-   *
-   * @returns {boolean} True if the connection state is "checking" or "new",
-   *  otherwise false.
-   */
-  isIceConnectionPending : function() {
-    return (this.isIceChecking() || this.isIceNew());
-  },
-
-  /**
    * Registers a callback for the ICE connection state change and
    * appends the new state to an array for logging it later.
    */
   logIceConnectionState: function() {
     this.iceConnectionLog = [this._pc.iceConnectionState];
     this.ice_connection_callbacks.logIceStatus = () => {
       var newstate = this._pc.iceConnectionState;
       var oldstate = this.iceConnectionLog[this.iceConnectionLog.length - 1]
@@ -1266,34 +1239,35 @@ PeerConnectionWrapper.prototype = {
       } else {
         ok(false, this + ": old ICE state " + oldstate + " missing in ICE transition array");
       }
       this.iceConnectionLog.push(newstate);
     };
   },
 
   /**
-   * Registers a callback for the ICE connection state change and
-   * reports success (=connected) or failure via the callbacks.
-   * States "new" and "checking" are ignored.
+   * Resets the ICE connected Promise and allows ICE connection state monitoring
+   * to go backwards to 'checking'.
+   */
+  expectIceChecking : function() {
+    this.iceCheckingRestartExpected = true;
+    this.iceConnected = new Promise((resolve, reject) => {
+      this.iceConnectedResolve = resolve;
+      this.iceConnectedReject = reject;
+    });
+  },
+
+  /**
+   * Waits for ICE to either connect or fail.
    *
    * @returns {Promise}
    *          resolves when connected, rejects on failure
    */
   waitForIceConnected : function() {
-    return new Promise((resolve, reject) =>
-        this.ice_connection_callbacks.waitForIceConnected = () => {
-      if (this.isIceConnected()) {
-        delete this.ice_connection_callbacks.waitForIceConnected;
-        resolve();
-      } else if (!this.isIceConnectionPending()) {
-        delete this.ice_connection_callbacks.waitForIceConnected;
-        reject(new Error('ICE failed'));
-      }
-    });
+    return this.iceConnected;
   },
 
   /**
    * Setup a onicecandidate handler
    *
    * @param {object} test
    *        A PeerConnectionTest object to which the ice candidates gets
    *        forwarded.
--- a/dom/media/tests/mochitest/templates.js
+++ b/dom/media/tests/mochitest/templates.js
@@ -57,39 +57,16 @@ function dumpSdp(test) {
   if ((test.pcLocal) && (test.pcRemote) &&
     (typeof test.pcRemote.setLocalDescDate !== 'undefined') &&
     (typeof test.pcRemote.setLocalDescStableEventDate !== 'undefined')) {
     var delta = deltaSeconds(test.pcRemote.setLocalDescDate, test.pcRemote.setLocalDescStableEventDate);
     dump("Delay between pcRemote.setLocal <-> pcRemote.signalingStateStable: " + delta + "\n");
   }
 }
 
-function waitForIceConnected(test, pc) {
-  if (!pc.iceCheckingRestartExpected) {
-    if (pc.isIceConnected()) {
-      info(pc + ": ICE connection state log: " + pc.iceConnectionLog);
-      ok(true, pc + ": ICE is in connected state");
-      return Promise.resolve();
-    }
-
-    if (!pc.isIceConnectionPending()) {
-      dumpSdp(test);
-      var details = pc + ": ICE is already in bad state: " + pc.iceConnectionState;
-      ok(false, details);
-      return Promise.reject(new Error(details));
-    }
-  }
-
-  return pc.waitForIceConnected()
-    .then(() => {
-      info(pc + ": ICE connection state log: " + pc.iceConnectionLog);
-      ok(pc.isIceConnected(), pc + ": ICE switched to 'connected' state");
-    });
-}
-
 // We need to verify that at least one candidate has been (or will be) gathered.
 function waitForAnIceCandidate(pc) {
   return new Promise(resolve => {
     if (!pc.localRequiresTrickleIce ||
         pc._local_ice_candidates.length > 0) {
       resolve();
     } else {
       // In some circumstances, especially when both PCs are on the same
@@ -394,21 +371,27 @@ var commandsPeerConnectionOfferAnswer = 
   },
 
   function PC_LOCAL_CHECK_CAN_TRICKLE_SYNC(test) {
     is(test.pcLocal._pc.canTrickleIceCandidates, true,
        "Local thinks that remote can trickle");
   },
 
   function PC_LOCAL_WAIT_FOR_ICE_CONNECTED(test) {
-    return waitForIceConnected(test, test.pcLocal);
+    return test.pcLocal.waitForIceConnected()
+    .then(() => {
+      info(test.pcLocal + ": ICE connection state log: " + test.pcLocal.iceConnectionLog);
+    });
   },
 
   function PC_REMOTE_WAIT_FOR_ICE_CONNECTED(test) {
-    return waitForIceConnected(test, test.pcRemote);
+    return test.pcRemote.waitForIceConnected()
+    .then(() => {
+      info(test.pcRemote + ": ICE connection state log: " + test.pcRemote.iceConnectionLog);
+    });
   },
 
   function PC_LOCAL_VERIFY_ICE_GATHERING(test) {
     return waitForAnIceCandidate(test.pcLocal);
   },
 
   function PC_REMOTE_VERIFY_ICE_GATHERING(test) {
     return waitForAnIceCandidate(test.pcRemote);
--- a/dom/media/tests/mochitest/test_peerConnection_addDataChannelNoBundle.html
+++ b/dom/media/tests/mochitest/test_peerConnection_addDataChannelNoBundle.html
@@ -15,20 +15,20 @@
   runNetworkTest(function (options) {
     options = options || { };
     options.bundle = false;
     test = new PeerConnectionTest(options);
     addRenegotiation(test.chain,
                      commandsCreateDataChannel.concat(
                        [
                          function PC_LOCAL_EXPECT_ICE_CHECKING(test) {
-                           test.pcLocal.iceCheckingRestartExpected = true;
+                           test.pcLocal.expectIceChecking();
                          },
                          function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-                           test.pcRemote.iceCheckingRestartExpected = true;
+                           test.pcRemote.expectIceChecking();
                          },
                        ]
                       ),
                      commandsCheckDataChannel);
 
     // Insert before the second PC_LOCAL_WAIT_FOR_MEDIA_FLOW
     test.chain.insertBefore('PC_LOCAL_WAIT_FOR_MEDIA_FLOW',
                             commandsWaitForDataChannel,
--- a/dom/media/tests/mochitest/test_peerConnection_addSecondAudioStreamNoBundle.html
+++ b/dom/media/tests/mochitest/test_peerConnection_addSecondAudioStreamNoBundle.html
@@ -18,21 +18,21 @@
     test = new PeerConnectionTest(options);
     addRenegotiation(test.chain,
       [
         function PC_LOCAL_ADD_SECOND_STREAM(test) {
           test.setMediaConstraints([{audio: true}, {audio: true}],
                                    [{audio: true}]);
           // Since this is a NoBundle variant, adding a track will cause us to
           // go back to checking.
-          test.pcLocal.iceCheckingRestartExpected = true;
+          test.pcLocal.expectIceChecking();
           return test.pcLocal.getAllUserMedia([{audio: true}]);
         },
         function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-          test.pcRemote.iceCheckingRestartExpected = true;
+          test.pcRemote.expectIceChecking();
         },
       ]
     );
 
     // TODO(bug 1093835): figure out how to verify if media flows through the new stream
     test.setMediaConstraints([{audio: true}], [{audio: true}]);
     test.run();
   });
--- a/dom/media/tests/mochitest/test_peerConnection_addSecondVideoStreamNoBundle.html
+++ b/dom/media/tests/mochitest/test_peerConnection_addSecondVideoStreamNoBundle.html
@@ -18,21 +18,21 @@
     test = new PeerConnectionTest(options);
     addRenegotiation(test.chain,
       [
         function PC_LOCAL_ADD_SECOND_STREAM(test) {
           test.setMediaConstraints([{video: true}, {video: true}],
                                    [{video: true}]);
           // Since this is a NoBundle variant, adding a track will cause us to
           // go back to checking.
-          test.pcLocal.iceCheckingRestartExpected = true;
+          test.pcLocal.expectIceChecking();
           return test.pcLocal.getAllUserMedia([{video: true}]);
         },
         function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-          test.pcRemote.iceCheckingRestartExpected = true;
+          test.pcRemote.expectIceChecking();
         },
       ]
     );
 
     // TODO(bug 1093835): figure out how to verify if media flows through the new stream
     test.setMediaConstraints([{video: true}], [{video: true}]);
     test.run();
   });
--- a/dom/media/tests/mochitest/test_peerConnection_restartIce.html
+++ b/dom/media/tests/mochitest/test_peerConnection_restartIce.html
@@ -17,20 +17,20 @@
 
     addRenegotiation(test.chain,
       [
         // causes a full, normal ice restart
         function PC_LOCAL_SET_OFFER_OPTION(test) {
           test.setOfferOptions({ iceRestart: true });
         },
         function PC_LOCAL_EXPECT_ICE_CHECKING(test) {
-          test.pcLocal.iceCheckingRestartExpected = true;
+          test.pcLocal.expectIceChecking();
         },
         function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-          test.pcRemote.iceCheckingRestartExpected = true;
+          test.pcRemote.expectIceChecking();
         }
       ]
     );
 
     test.setMediaConstraints([{audio: true}, {video: true}],
                              [{audio: true}, {video: true}]);
     test.run();
   });
--- a/dom/media/tests/mochitest/test_peerConnection_restartIceLocalAndRemoteRollback.html
+++ b/dom/media/tests/mochitest/test_peerConnection_restartIceLocalAndRemoteRollback.html
@@ -70,20 +70,20 @@
         function PC_LOCAL_WAIT_FOR_END_OF_TRICKLE(test) {
           return test.pcLocal.endOfTrickleIce;
         },
         function PC_REMOTE_WAIT_FOR_END_OF_TRICKLE(test) {
           return test.pcRemote.endOfTrickleIce;
         },
 
         function PC_LOCAL_EXPECT_ICE_CHECKING(test) {
-          test.pcLocal.iceCheckingRestartExpected = true;
+          test.pcLocal.expectIceChecking();
         },
         function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-          test.pcRemote.iceCheckingRestartExpected = true;
+          test.pcRemote.expectIceChecking();
         }
       ],
       1 // Replaces after second PC_REMOTE_CREATE_ANSWER
     );
     test.chain.append(commandsPeerConnectionOfferAnswer);
 
     // for now, only use one stream, because rollback doesn't seem to
     // like multiple streams.  See bug 1259465.
--- a/dom/media/tests/mochitest/test_peerConnection_restartIceLocalRollback.html
+++ b/dom/media/tests/mochitest/test_peerConnection_restartIceLocalRollback.html
@@ -48,20 +48,20 @@
                                           sdp: ""}),
               STABLE);
         },
         // Rolling back should shut down gathering
         function PC_LOCAL_WAIT_FOR_END_OF_TRICKLE(test) {
           return test.pcLocal.endOfTrickleIce;
         },
         function PC_LOCAL_EXPECT_ICE_CHECKING(test) {
-          test.pcLocal.iceCheckingRestartExpected = true;
+          test.pcLocal.expectIceChecking();
         },
         function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-          test.pcRemote.iceCheckingRestartExpected = true;
+          test.pcRemote.expectIceChecking();
         }
       ]
     );
 
     // for now, only use one stream, because rollback doesn't seem to
     // like multiple streams.  See bug 1259465.
     test.setMediaConstraints([{audio: true}],
                              [{audio: true}]);
--- a/dom/media/tests/mochitest/test_peerConnection_restartIceNoBundle.html
+++ b/dom/media/tests/mochitest/test_peerConnection_restartIceNoBundle.html
@@ -19,20 +19,20 @@
 
     addRenegotiation(test.chain,
       [
         // causes a full, normal ice restart
         function PC_LOCAL_SET_OFFER_OPTION(test) {
           test.setOfferOptions({ iceRestart: true });
         },
         function PC_LOCAL_EXPECT_ICE_CHECKING(test) {
-          test.pcLocal.iceCheckingRestartExpected = true;
+          test.pcLocal.expectIceChecking();
         },
         function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-          test.pcRemote.iceCheckingRestartExpected = true;
+          test.pcRemote.expectIceChecking();
         }
       ]
     );
 
     test.setMediaConstraints([{audio: true}, {video: true}],
                              [{audio: true}, {video: true}]);
     test.run();
   });
--- a/dom/media/tests/mochitest/test_peerConnection_restartIceNoBundleNoRtcpMux.html
+++ b/dom/media/tests/mochitest/test_peerConnection_restartIceNoBundleNoRtcpMux.html
@@ -20,20 +20,20 @@
 
     addRenegotiation(test.chain,
       [
         // causes a full, normal ice restart
         function PC_LOCAL_SET_OFFER_OPTION(test) {
           test.setOfferOptions({ iceRestart: true });
         },
         function PC_LOCAL_EXPECT_ICE_CHECKING(test) {
-          test.pcLocal.iceCheckingRestartExpected = true;
+          test.pcLocal.expectIceChecking();
         },
         function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-          test.pcRemote.iceCheckingRestartExpected = true;
+          test.pcRemote.expectIceChecking();
         }
       ]
     );
 
     test.setMediaConstraints([{audio: true}, {video: true}],
                              [{audio: true}, {video: true}]);
     test.run();
   });
--- a/dom/media/tests/mochitest/test_peerConnection_restartIceNoRtcpMux.html
+++ b/dom/media/tests/mochitest/test_peerConnection_restartIceNoRtcpMux.html
@@ -19,20 +19,20 @@
 
     addRenegotiation(test.chain,
       [
         // causes a full, normal ice restart
         function PC_LOCAL_SET_OFFER_OPTION(test) {
           test.setOfferOptions({ iceRestart: true });
         },
         function PC_LOCAL_EXPECT_ICE_CHECKING(test) {
-          test.pcLocal.iceCheckingRestartExpected = true;
+          test.pcLocal.expectIceChecking();
         },
         function PC_REMOTE_EXPECT_ICE_CHECKING(test) {
-          test.pcRemote.iceCheckingRestartExpected = true;
+          test.pcRemote.expectIceChecking();
         }
       ]
     );
 
     test.setMediaConstraints([{audio: true}, {video: true}],
                              [{audio: true}, {video: true}]);
     test.run();
   });