1651703 - Update AudioParamDescriptor webidl min and max values

Status: RESOLVED FIXED

(Core :: Web Audio, enhancement)

Description

[Arnaud Bienner]

Follow-up of bug 1501709. This is about fixing the FIXME comment in AudioParamDescriptor.webidl. Now MSVC support has been dropped (bug 1512504), we can remove this workaround.

Comment 1

[Arnaud Bienner]

Attachment: Bug 1651703: fix min and max values in AudioParamDescriptor now we don't need this workaround to support MSVC. r=karlt

Comment 2

[Arnaud Bienner]

Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca760f4a10d9abf318e4269d9b8a7e234254e881

Comment 3

[Arnaud Bienner]

Karl: I'm not sure which try jobs I should start. It looks like selecting jobs to run on try server is more complicated now than it used to be, and unfortunately "auto" (which looked promising) doesn't work.
Also, I'm not sure how to request this to be landed: I can't find the "checkin-needed" keyword, and it looks like I don't have access to Lando.

Comment 4

[Karl Tomlinson (:karlt)]

This is the changeset that I add when ./mach try auto fails:
https://hg.mozilla.org/try/rev/096f332daa2c77188b9390482817412d9b8f9488

For landing, I think the process is
"Edit Revision" -> "Tags" -> type "Check-in Needed".

Comment 5

[Arnaud Bienner]

Thanks Karl!
I missed the fact you can still use the try syntax.

I pushed to try, building on Linux macOS and Windows (since there was a compiler bug, I want to be sure all platforms behave correctly now, though I think all of them are using clang now): https://treeherder.mozilla.org/#/jobs?repo=try&revision=eba1ebbeed05cb58ff4351fb00913aad5a5d92d7
Almost green. I don't think the issue I see are related to this change, so I will request checkin.

Comment 6

[Narcis Beleuzu [:NarcisB]]

Arnaud, land failed with:
"Details: hg error in cmd: hg push -r tip upstream: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote:
remote: ******************************* ERROR *******************************
remote: Changeset 775a97906327 alters WebIDL file(s) without DOM peer review:
remote: dom/webidl/AudioParamDescriptor.webidl
remote:
remote: Please, request review from the #webidl reviewer group or either:
remote: - Andrea Marchesini (:baku)
remote: - Andreas Farre (:farre)
remote: - Andrew McCreight (:mccr8)
remote: - Bobby Holley (:bholley)
remote: - Ehsan Akhgari (:ehsan)
remote: - Emilio Cobos (:emilio)
remote: - Henri Sivonen (:hsivonen)
remote: - Nika Layzell (:mystor)
remote: - Olli Pettay (:smaug)
remote: - Peter Van der Beken (:peterv)
remote: *********************************************************************
remote:
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.mozhooks hook failed
abort: push failed on remote"

Comment 7

[Arnaud Bienner]

I've asked Andrea to review the change, since he reviewed the original file when it was created.

Comment 8

[Arnaud Bienner]

Andrea, could you please review (and approve :)) this WebIDL change?
Looks like I need DOM peer review for this to be landed, and you reviewed this file when I created it two years ago (bug 1501709 https://hg.mozilla.org/mozilla-central/rev/fc6fa6ebad86)
This change is about updating the WebIDL file so it matches the spec.
We couldn't use the values used in the spec at that time because of a compiler error with MSVC, which is not supported anymore (more details above and on bug 1501709).

Comment 9

[Andrea Marchesini [:baku]]

Hi Arnaud. Sorry for the delay, I haven't seen your patch in my review queue. R+

Comment 10

[Arnaud Bienner]

No worries :) Thanks for the review!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c5cd63fa29c
fix min and max values in AudioParamDescriptor now we don't need this workaround to support MSVC. r=karlt,baku

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5c5cd63fa29c