Bug 1654511 [wpt PR 24606] - [Sheriff] Revert "Deflake preservesPitch tests", a=testonly

Automatic update from web-platform-tests
[Sheriff] Revert "Deflake preservesPitch tests"

This reverts commit 7b6c4007896853f91f32bd69b2c770f0210c66e5.

Reason for revert:

Post this change, preservesPitch seems to consistently fail.

https://ci.chromium.org/p/chromium/builders/ci/Mac10.14%20Tests

Original change's description:
> Deflake preservesPitch tests
>
> This CL changes preservesPitch tests to wait until the audioElement's
> currentTime is at least 0.5 seconds before trying to detect the pitch.
> Without this check, the test can try to detect the pitch before the
> audio has played enough, which can return a dominant frequency of 0Hz
> and fail the test.
>
> The CL also makes a few stylistic changes, and fixes an off-by-one error
> in the number of increments used to calculate frequencies.
>
> Bug: 1096238
> Change-Id: I6e98e172862a47bea1c4026737138293914f7906
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2298281
> Auto-Submit: Thomas Guilbert <tguilbert@chromium.org>
> Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
> Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#788535}

TBR=tguilbert@chromium.org,foolip@chromium.org

Change-Id: I0042f73ca9c5de7d82be1200794c45b46a003f9b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1096238
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2299183
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Commit-Queue: Roger McFarlane <rogerm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788626}

--
Reland preservesPitch test deflake, fix timeouts

This CL relands "Deflake preservesPitch tests", which was reverted by:
commit f19f3404371bba3cac38bf1bcf672ef5f3e96558.

It also attempts to incorporate some of the WPT changes made upstream,
but not yet landed, in:
https://github.com/web-platform-tests/wpt/pull/24599

Specifically, it fixes typos in the Safari prefixes, and explicitly
starts the audio context. However, it is different from the PR, since
it reuses the same Audio element, MediaElementAudioSourceNode and
AudioContext. This is an attempt to cut down on overhead costs and
fix test timeouts. We instead reset the audio.currentTime to 0, and
only create a new analyser node for each test.

Bug: 1105877, 1096238
Change-Id: Ie528ec0b7c38d9df59fcb04696c810e6d1c232f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2300929
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Auto-Submit: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789262}

--

wpt-commits: 9abf97af00b4a3f55ecba875bf1cdbecb4a1a1ee, 9a67b791ec7d9c6538411430677ac25eb6ef60b0
wpt-pr: 24606

testing/web-platform/tests/html/semantics/embedded-content/media-elements/pitch-detector.js

@@ -4,11 +4,14 @@ window.AudioContext = window.AudioContext || window.webkitAudioContext;
 
 var FFT_SIZE = 2048;
 
-function getPitchDetector(media, t) {
-  var audioContext = new AudioContext();
-  t.add_cleanup(() => audioContext.close());
+var audioContext;
+var sourceNode;
 
-  var sourceNode = audioContext.createMediaElementSource(media);
+function getPitchDetector(media) {
+  if(!audioContext) {
+    audioContext = new AudioContext();
+    sourceNode = audioContext.createMediaElementSource(media);
+  }
 
   var analyser = audioContext.createAnalyser();
   analyser.fftSize = FFT_SIZE;
@@ -16,13 +19,20 @@ function getPitchDetector(media, t) {
   sourceNode.connect(analyser);
   analyser.connect(audioContext.destination);
 
-  return () => getPitch(analyser);
+  return {
+      ensureStart() { return audioContext.resume(); },
+      detect() { return getPitch(analyser); },
+      cleanup() {
+        sourceNode.disconnect();
+        analyser.disconnect();
+      },
+  };
 }
 
 function getPitch(analyser) {
   // Returns the frequency value for the nth FFT bin.
   var binConverter = (bin) =>
-    (analyser.context.sampleRate/2)*((bin)/(analyser.frequencyBinCount-1));
+    (audioContext.sampleRate/2)*((bin)/(analyser.frequencyBinCount-1));
 
   var buf = new Uint8Array(analyser.frequencyBinCount);
   analyser.getByteFrequencyData(buf);

testing/web-platform/tests/html/semantics/embedded-content/media-elements/preserves-pitch.html

@@ -15,8 +15,8 @@ function getPreservesPitch(audio) {
     if ("mozPreservesPitch" in HTMLAudioElement.prototype) {
         return audio.mozPreservesPitch;
     }
-    if ("wekbitPreservesPitch" in HTMLAudioElement.prototype) {
-        return audio.wekbitPreservesPitch;
+    if ("webkitPreservesPitch" in HTMLAudioElement.prototype) {
+        return audio.webkitPreservesPitch;
     }
     return undefined;
 }
@@ -27,8 +27,8 @@ function setPreservesPitch(audio, value) {
         audio.preservesPitch = value;
     } else if ("mozPreservesPitch" in HTMLAudioElement.prototype) {
         audio.mozPreservesPitch = value;
-    } else if ("wekbitPreservesPitch" in HTMLAudioElement.prototype) {
-        audio.wekbitPreservesPitch = value;
+    } else if ("webkitPreservesPitch" in HTMLAudioElement.prototype) {
+        audio.webkitPreservesPitch = value;
     }
 }
 
@@ -37,21 +37,28 @@ test(function(t) {
 }, "Test that preservesPitch is present and unprefixed.");
 
 test(function(t) {
-    let audio = document.createElement('audio');
-    assert_true(getPreservesPitch(audio));
+    let defaultAudio = document.createElement('audio');
+    assert_true(getPreservesPitch(defaultAudio));
 
-    setPreservesPitch(audio, false);
-    assert_false(getPreservesPitch(audio));
-}, "Test that presevesPitch is on by default");
+    setPreservesPitch(defaultAudio, false);
+    assert_false(getPreservesPitch(defaultAudio));
+}, "Test that preservesPitch is on by default");
+
+
+var audio;
+
+function addTestCleanups(t, detector) {
+    t.add_cleanup(() => {
+        audio.pause();
+        audio.currentTime = 0;
+    });
+    t.add_cleanup(() => detector.cleanup());
+}
 
 function testPreservesPitch(preservesPitch, playbackRate, expectedPitch, description) {
     promise_test(async t => {
-        let audio = document.createElement('audio');
-
-        var detector = getPitchDetector(audio, t);
-
-        // This file contains 5 seconds of a 440hz sine wave.
-        audio.src = "/media/sine440.mp3";
+        let detector = getPitchDetector(audio);
+        addTestCleanups(t, detector);
 
         audio.playbackRate = playbackRate;
         setPreservesPitch(audio, preservesPitch);
@@ -66,13 +73,12 @@ function testPreservesPitch(preservesPitch, playbackRate, expectedPitch, descrip
             });
         }
 
-        await test_driver.bless("Play audio element", () => audio.play() )
+        // Wait until we have played some audio. Otherwise, the detector
+        // might return a pitch of 0Hz.
+        audio.play();
+        await waitUntil(0.25);
 
-        // Wait until we have at least played some audio. Otherwise, the
-        // detector might return a pitch of 0Hz.
-        await waitUntil(0.5);
-
-        var pitch = detector();
+        var pitch = detector.detect();
 
         // 25Hz is larger than the margin we get from 48kHz and 44.1kHz
         // audio being analyzed by a FFT of size 2048. If we get something
@@ -89,6 +95,24 @@ function testPreservesPitch(preservesPitch, playbackRate, expectedPitch, descrip
 
 var REFERENCE_PITCH = 440;
 
+promise_test(async t => {
+    // Create the audio element only once, in order to lower the chances of
+    // tests timing out.
+    audio = document.createElement('audio');
+
+    // This file contains 5 seconds of a 440hz sine wave.
+    audio.src = "/media/sine440.mp3";
+
+    let detector = getPitchDetector(audio);
+    addTestCleanups(t, detector);
+
+    // The first time we run the test, we need to interact with the
+    // AudioContext and Audio element via user gestures.
+    await test_driver.bless("Play audio element", () => {
+        return Promise.all([audio.play(), detector.ensureStart()]);
+    });
+}, "Setup Audio element and AudioContext")
+
 testPreservesPitch(true, 1.0, REFERENCE_PITCH,
     "The default playbackRate should not affect pitch")