Bug 1304269: lets promise ice from now on. r=bwc draft
authorNils Ohlmeier [:drno] <drno@ohlmeier.org>
Wed, 21 Sep 2016 16:42:22 -0700
changeset 416673 c3bf738d323f8386bbf25d63e04472cc78a29539
parent 416020 560b2c805bf7bebeb3ceebc495a81b2aa4c0c755
child 531921 0a051f672af633825d0c3640567a15865d990007
push id30215
push userdrno@ohlmeier.org
push dateThu, 22 Sep 2016 19:44:37 +0000
reviewersbwc
bugs1304269
milestone52.0a1
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();
   });