1478837 - Reinstate AudioParam tests removed by Google

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P2)

Description

[Karl Tomlinson (:karlt)]

Some AudioParam web platform tests written by Mozilla, which did not pass in Blink, were replaced with tests that do not run in Gecko for bug 1438319.

Perhaps it is possible to adjust the harness to use either ScriptProcessor or offline suspend according to feature detection.  If this is difficult, or as a stopgap, the old tests could be reinstated to dom/media/webaudio/test where they are under our control.  There should be copies there anyway to run on asan and android.

Comment 1

[Paul Adenot (:padenot)]

I'll sort this out, thanks for noticing.

Comment 2

[Paul Adenot (:padenot)]

I'm going to reimport the tests as mochitests or mozilla-only wpt (if that's a thing). I think having an hybrid test harness is too much work for not enough value.

In the longer run, it's a bit unclear whether Gecko's guarantees about the MSG messages that originate from the same event loop task are going to be run together is going to be true (it's something that we've been talking about at a spec level, but not deeply), or whether, for example, AudioContext.currentTime can change during a script run (for now in Gecko, it can't, but we're alone with this behaviour).

Last time I was thinking about this, I think I came to the conclusion that not having those guarantees had some benefits in terms of interactivity (interaction-to-sound-output in particular), but it's not so clear in my head right now.

Comment 3

[Karl Tomlinson (:karlt)]

Thank you, Paul.

I hadn't noticed that the changes for bug 1308433 meant that these tests depended on multiple control events being processed in a single transaction.

There is a mozilla-only wpt directory, but I don't see much value in using that place unless there is no existing place for tests.  The same tests can run without modification from dom/media/webaudio/test.  Using the existing mochitest directory has the advantage that there are fewer places from which to remember to run tests.  testing/web-platform/mozilla/tests is not upstreamed AFAICT.

https://developer.mozilla.org/en-US/docs/Mozilla/QA/web-platform-tests#Setting_prefs#Where_to_put_new_tests

Comment 4

[Paul Adenot (:padenot)]

Attachment: reinstate-tests.patch

Karl, you don't seem to have a phabricator account yet, and MozReview is now closed, so I'm doing this one the old way.

This is simply re-importing the tests as they were right before bug 1438319 landed, renaming the file so that they start with "test_" otherwise mach seems unhappy.

Comment 5

[Karl Tomlinson (:karlt)]

Thank you!

https://bugs.chromium.org/p/chromium/issues/detail?id=812285#c2 claims there was a bug in the setTargetAtTime test.
It "wasn't actually testing setTargetAtTime because a
    linearRampToValueAtTime event was called at the same time,
    effectively replacing the setTargetAtTime event"

But that's best dealt with separately.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f35a649bd10f
Reinstate a few Web Audio API tests in the mochitest directory. r=karlt

Comment 7

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/f35a649bd10f