Auto merge of #23270 - Manishearth:audioparam-validation, r=ferjm

Add input validation for AudioParam methods

Waiting on a build to make WPT updates

Spec requires us to validate inputs, we aren't doing this here.

We don't need to check for things being finite since we already get the arguments as `Finite` values

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23270)
<!-- Reviewable:end -->

components/script/dom/audioparam.rs

@@ -7,6 +7,7 @@ use crate::dom::bindings::codegen::Bindings::AudioParamBinding;
 use crate::dom::bindings::codegen::Bindings::AudioParamBinding::{
     AudioParamMethods, AutomationRate,
 };
+use crate::dom::bindings::error::{Error, Fallible};
 use crate::dom::bindings::num::Finite;
 use crate::dom::bindings::reflector::{reflect_dom_object, Reflector};
 use crate::dom::bindings::root::{Dom, DomRoot};
@@ -142,12 +143,22 @@ impl AudioParamMethods for AudioParam {
     }
 
     // https://webaudio.github.io/web-audio-api/#dom-audioparam-setvalueattime
-    fn SetValueAtTime(&self, value: Finite<f32>, start_time: Finite<f64>) -> DomRoot<AudioParam> {
+    fn SetValueAtTime(
+        &self,
+        value: Finite<f32>,
+        start_time: Finite<f64>,
+    ) -> Fallible<DomRoot<AudioParam>> {
+        if *start_time < 0. {
+            return Err(Error::Range(format!(
+                "start time {} should not be negative",
+                *start_time
+            )));
+        }
         self.message_node(AudioNodeMessage::SetParam(
             self.param,
             UserAutomationEvent::SetValueAtTime(*value, *start_time),
         ));
-        DomRoot::from_ref(self)
+        Ok(DomRoot::from_ref(self))
     }
 
     // https://webaudio.github.io/web-audio-api/#dom-audioparam-linearramptovalueattime
@@ -155,12 +166,18 @@ impl AudioParamMethods for AudioParam {
         &self,
         value: Finite<f32>,
         end_time: Finite<f64>,
-    ) -> DomRoot<AudioParam> {
+    ) -> Fallible<DomRoot<AudioParam>> {
+        if *end_time < 0. {
+            return Err(Error::Range(format!(
+                "end time {} should not be negative",
+                *end_time
+            )));
+        }
         self.message_node(AudioNodeMessage::SetParam(
             self.param,
             UserAutomationEvent::RampToValueAtTime(RampKind::Linear, *value, *end_time),
         ));
-        DomRoot::from_ref(self)
+        Ok(DomRoot::from_ref(self))
     }
 
     // https://webaudio.github.io/web-audio-api/#dom-audioparam-exponentialramptovalueattime
@@ -168,12 +185,24 @@ impl AudioParamMethods for AudioParam {
         &self,
         value: Finite<f32>,
         end_time: Finite<f64>,
-    ) -> DomRoot<AudioParam> {
+    ) -> Fallible<DomRoot<AudioParam>> {
+        if *end_time < 0. {
+            return Err(Error::Range(format!(
+                "end time {} should not be negative",
+                *end_time
+            )));
+        }
+        if *value == 0. {
+            return Err(Error::Range(format!(
+                "target value {} should not be 0",
+                *value
+            )));
+        }
         self.message_node(AudioNodeMessage::SetParam(
             self.param,
             UserAutomationEvent::RampToValueAtTime(RampKind::Exponential, *value, *end_time),
         ));
-        DomRoot::from_ref(self)
+        Ok(DomRoot::from_ref(self))
     }
 
     // https://webaudio.github.io/web-audio-api/#dom-audioparam-settargetattime
@@ -182,30 +211,54 @@ impl AudioParamMethods for AudioParam {
         target: Finite<f32>,
         start_time: Finite<f64>,
         time_constant: Finite<f32>,
-    ) -> DomRoot<AudioParam> {
+    ) -> Fallible<DomRoot<AudioParam>> {
+        if *start_time < 0. {
+            return Err(Error::Range(format!(
+                "start time {} should not be negative",
+                *start_time
+            )));
+        }
+        if *time_constant < 0. {
+            return Err(Error::Range(format!(
+                "time constant {} should not be negative",
+                *time_constant
+            )));
+        }
         self.message_node(AudioNodeMessage::SetParam(
             self.param,
             UserAutomationEvent::SetTargetAtTime(*target, *start_time, (*time_constant).into()),
         ));
-        DomRoot::from_ref(self)
+        Ok(DomRoot::from_ref(self))
     }
 
     // https://webaudio.github.io/web-audio-api/#dom-audioparam-cancelscheduledvalues
-    fn CancelScheduledValues(&self, cancel_time: Finite<f64>) -> DomRoot<AudioParam> {
+    fn CancelScheduledValues(&self, cancel_time: Finite<f64>) -> Fallible<DomRoot<AudioParam>> {
+        if *cancel_time < 0. {
+            return Err(Error::Range(format!(
+                "cancel time {} should not be negative",
+                *cancel_time
+            )));
+        }
         self.message_node(AudioNodeMessage::SetParam(
             self.param,
             UserAutomationEvent::CancelScheduledValues(*cancel_time),
         ));
-        DomRoot::from_ref(self)
+        Ok(DomRoot::from_ref(self))
     }
 
     // https://webaudio.github.io/web-audio-api/#dom-audioparam-cancelandholdattime
-    fn CancelAndHoldAtTime(&self, cancel_time: Finite<f64>) -> DomRoot<AudioParam> {
+    fn CancelAndHoldAtTime(&self, cancel_time: Finite<f64>) -> Fallible<DomRoot<AudioParam>> {
+        if *cancel_time < 0. {
+            return Err(Error::Range(format!(
+                "cancel time {} should not be negative",
+                *cancel_time
+            )));
+        }
         self.message_node(AudioNodeMessage::SetParam(
             self.param,
             UserAutomationEvent::CancelAndHoldAtTime(*cancel_time),
         ));
-        DomRoot::from_ref(self)
+        Ok(DomRoot::from_ref(self))
     }
 }
 

components/script/dom/webidls/AudioParam.webidl

@@ -18,15 +18,15 @@ interface AudioParam {
     readonly attribute float defaultValue;
     readonly attribute float minValue;
     readonly attribute float maxValue;
-    AudioParam setValueAtTime(float value, double startTime);
+    [Throws] AudioParam setValueAtTime(float value, double startTime);
-    AudioParam linearRampToValueAtTime(float value, double endTime);
+    [Throws] AudioParam linearRampToValueAtTime(float value, double endTime);
-    AudioParam exponentialRampToValueAtTime(float value, double endTime);
+    [Throws] AudioParam exponentialRampToValueAtTime(float value, double endTime);
-    AudioParam setTargetAtTime(float target,
+    [Throws] AudioParam setTargetAtTime(float target,
                                double startTime,
                                float timeConstant);
 //    AudioParam setValueCurveAtTime(sequence<float> values,
 //                                   double startTime,
 //                                   double duration);
-    AudioParam cancelScheduledValues(double cancelTime);
+    [Throws] AudioParam cancelScheduledValues(double cancelTime);
-    AudioParam cancelAndHoldAtTime(double cancelTime);
+    [Throws] AudioParam cancelAndHoldAtTime(double cancelTime);
 };

tests/wpt/metadata/webaudio/the-audio-api/the-audioparam-interface/audioparam-exceptional-values.html.ini

@@ -1,46 +1,19 @@
 [audioparam-exceptional-values.html]
-  [X gain.gain.exponentialRampToValueAtTime(-1e-100,1) did not throw an exception.]
-    expected: FAIL
-
-  [X gain.gain.exponentialRampToValueAtTime(0,1) did not throw an exception.]
-    expected: FAIL
-
-  [X gain.gain.linearRampToValueAtTime(1,-1) did not throw an exception.]
-    expected: FAIL
-
-  [X gain.gain.setTargetAtTime(1,-1,1) did not throw an exception.]
-    expected: FAIL
-
   [X gain.gain.setValueCurveAtTime([0,0,0\],1,-1) threw "TypeError" instead of EcmaScript error RangeError.]
     expected: FAIL
 
-  [< [special cases 1\] 9 out of 9 assertions were failed.]
-    expected: FAIL
-
   [X gain.gain.setValueCurveAtTime(curve, 1, -1) threw "TypeError" instead of EcmaScript error RangeError.]
     expected: FAIL
 
-  [X gain.gain.exponentialRampToValueAtTime(1,-1) did not throw an exception.]
-    expected: FAIL
-
-  [X gain.gain.setValueAtTime(1,-1) did not throw an exception.]
-    expected: FAIL
-
   [X gain.gain.setValueCurveAtTime(curve, 1, 0) threw "TypeError" instead of EcmaScript error RangeError.]
     expected: FAIL
 
-  [# AUDIT TASK RUNNER FINISHED: 2 out of 6 tasks were failed.]
+  [< [special cases 1\] 4 out of 9 assertions were failed.]
     expected: FAIL
 
   [X gain.gain.setValueCurveAtTime([0,0,0\],-1,1) threw "TypeError" instead of EcmaScript error RangeError.]
     expected: FAIL
 
-  [X gain.gain.setTargetAtTime(1,1,-1) did not throw an exception.]
+  [# AUDIT TASK RUNNER FINISHED: 1 out of 6 tasks were failed.]
-    expected: FAIL
-
-  [X gain.gain.exponentialRampToValueAtTime(1e-100,1) did not throw an exception.]
-    expected: FAIL
-
-  [< [special cases 2\] 3 out of 3 assertions were failed.]
     expected: FAIL