Bug 1535100: Re-enable and de-flake the RTCDTMFSender-ontonechange tests. r=jib
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 15 Mar 2019 18:45:59 +0000
changeset 464426 d57f48d6e42bc87ccc20f3350f49bf925d2d8bc2
parent 464425 1f911d12fd0737a2107c8b0cc98cf3d62165d7db
child 464427 2e1a91cd78fe003468ab4c4c625e9d300ad1709d
push id35716
push useraciure@mozilla.com
push dateSun, 17 Mar 2019 09:42:17 +0000
treeherdermozilla-central@8ee97c045359 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib
bugs1535100
milestone67.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 1535100: Re-enable and de-flake the RTCDTMFSender-ontonechange tests. r=jib Differential Revision: https://phabricator.services.mozilla.com/D23417
dom/media/PeerConnection.jsm
testing/web-platform/meta/webrtc/RTCDTMFSender-ontonechange-long.https.html.ini
testing/web-platform/meta/webrtc/RTCDTMFSender-ontonechange.https.html.ini
testing/web-platform/tests/webrtc/RTCDTMFSender-helper.js
--- a/dom/media/PeerConnection.jsm
+++ b/dom/media/PeerConnection.jsm
@@ -1367,17 +1367,19 @@ class RTCPeerConnection {
     this._closed = true;
     this.changeIceConnectionState("closed");
     if (this._localIdp) {
         this._localIdp.close();
     }
     if (this._remoteIdp) {
         this._remoteIdp.close();
     }
-    this._transceivers.forEach(t => t.setStopped());
+    if (!this._suppressEvents) {
+      this._transceivers.forEach(t => t.setStopped());
+    }
     this._impl.close();
     this._suppressEvents = true;
     delete this._pc;
     delete this._observer;
   }
 
   getLocalStreams() {
     this._checkClosed();
deleted file mode 100644
--- a/testing/web-platform/meta/webrtc/RTCDTMFSender-ontonechange-long.https.html.ini
+++ /dev/null
@@ -1,10 +0,0 @@
-[RTCDTMFSender-ontonechange-long.https.html]
-  [insertDTMF with duration greater than 6000 should be clamped to 6000]
-    disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1420640
-    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1498538
-    expected:
-      if e10s and (os == "linux") and (processor == "x86"): FAIL
-      if not debug and e10s and (os == "linux") and (processor == "x86_64"): FAIL
-      if debug and webrender and (os == "linux"): FAIL
-      if debug and (os == "win") and (version == "6.1.7601"): FAIL
-
--- a/testing/web-platform/meta/webrtc/RTCDTMFSender-ontonechange.https.html.ini
+++ b/testing/web-platform/meta/webrtc/RTCDTMFSender-ontonechange.https.html.ini
@@ -1,11 +1,9 @@
 [RTCDTMFSender-ontonechange.https.html]
-  disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1498538
-  bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1498538
   [Calling insertDTMF() in the middle of tonechange events should cause future tonechanges to be updated to new tones]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1531825
 
   [Calling insertDTMF() multiple times in the middle of tonechange events should cause future tonechanges to be updated the last provided tones]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1531825
 
--- a/testing/web-platform/tests/webrtc/RTCDTMFSender-helper.js
+++ b/testing/web-platform/tests/webrtc/RTCDTMFSender-helper.js
@@ -73,75 +73,73 @@ function createDtmfSender(pc = new RTCPe
           The expected new value of dtmfSender.toneBuffer
         expectedDuration
           The rough time since beginning or last tonechange event
           was fired.
     desc
       Test description.
  */
 function test_tone_change_events(testFunc, toneChanges, desc) {
-  async_test(t => {
-    const pc = new RTCPeerConnection();
+  // Convert to cumulative time
+  let cumulativeTime = 0;
+  const cumulativeToneChanges = toneChanges.map(c => {
+    cumulativeTime += c[2];
+    return [c[0], c[1], cumulativeTime];
+  });
 
-    createDtmfSender(pc)
-    .then(dtmfSender => {
-      let lastEventTime = Date.now();
+  // Wait for same duration as last expected duration + 100ms
+  // before passing test in case there are new tone events fired,
+  // in which case the test should fail.
+  const lastWait = toneChanges.pop()[2] + 100;
 
+  promise_test(async t => {
+    const pc = new RTCPeerConnection();
+    const dtmfSender = await createDtmfSender(pc);
+    const start = Date.now();
+
+    const allEventsReceived = new Promise(resolve => {
       const onToneChange = t.step_func(ev => {
         assert_true(ev instanceof RTCDTMFToneChangeEvent,
           'Expect tone change event object to be an RTCDTMFToneChangeEvent');
 
         const { tone } = ev;
         assert_equals(typeof tone, 'string',
           'Expect event.tone to be the tone string');
 
-        assert_greater_than(toneChanges.length, 0,
+        assert_greater_than(cumulativeToneChanges.length, 0,
           'More tonechange event is fired than expected');
 
         const [
-          expectedTone, expectedToneBuffer, expectedDuration
-        ] = toneChanges.shift();
+          expectedTone, expectedToneBuffer, expectedTime
+        ] = cumulativeToneChanges.shift();
 
         assert_equals(tone, expectedTone,
           `Expect current event.tone to be ${expectedTone}`);
 
         assert_equals(dtmfSender.toneBuffer, expectedToneBuffer,
           `Expect dtmfSender.toneBuffer to be updated to ${expectedToneBuffer}`);
 
-        const now = Date.now();
-        const duration = now - lastEventTime;
-
-        // We check that the delay is at least the expected one, but
-        // system load may cause random delay, so we do not put any
-        // realistic upper bound on the timing of the event.
-        assert_between_inclusive(duration, expectedDuration,
-                                 expectedDuration + 4000,
-          `Expect tonechange event for "${tone}" to be fired approximately after ${expectedDuration} milliseconds`);
-
-        lastEventTime = now;
-
-        if (toneChanges.length === 0) {
-          // Wait for same duration as last expected duration + 100ms
-          // before passing test in case there are new tone events fired,
-          // in which case the test should fail.
-          t.step_timeout(
-            t.step_func(() => {
-              t.done();
-              pc.close();
-              pc.otherPc.close();
-            }), expectedDuration + 100);
+        // We check that the cumulative delay is at least the expected one, but
+        // system load may cause random delays, so we do not put any
+        // realistic upper bound on the timing of the events.
+        assert_between_inclusive(Date.now() - start, expectedTime,
+                                 expectedTime + 4000,
+          `Expect tonechange event for "${tone}" to be fired approximately after ${expectedTime} milliseconds`);
+        if (cumulativeToneChanges.length === 0) {
+          resolve();
         }
       });
 
       dtmfSender.addEventListener('tonechange', onToneChange);
-      testFunc(t, dtmfSender, pc);
-    })
-    .catch(t.step_func(err => {
-      assert_unreached(`Unexpected promise rejection: ${err}`);
-    }));
+    });
+
+    testFunc(t, dtmfSender, pc);
+    await allEventsReceived;
+    const wait = ms => new Promise(resolve => t.step_timeout(resolve, ms));
+    await wait(lastWait);
   }, desc);
 }
 
 // Get the one and only tranceiver from pc.getTransceivers().
 // Assumes that there is only one tranceiver in pc.
 function getTransceiver(pc) {
   const transceivers = pc.getTransceivers();
   assert_equals(transceivers.length, 1,