Bug 102461 – Exception should be thrown when start or close of AudioBufferSourceNode is called at the wrong time.

• Component: Web Audio
• Status: NEW

[Li Yin]

Spec:https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#AudioSourceNode
start may only be called one time and must be called before stop is called or an exception will be thrown.
stop must only be called one time and only after a call to start or stop, or an exception will be thrown.

[Li Yin]

If stop only can be called one time, it should be impossible that stop is called after stop. So maybe this is a defect of W3C Spec.

[Li Yin]

"stop must only be called one time and only after a call to start or stop, or an exception will be thrown."
should be 
stop must only be called one time and only after a call to start, or an exception will be thrown.

[Li Yin]

Created attachment 174661 [details]
Patch

[Li Yin]

(In reply to comment #1)
> If stop only can be called one time, it should be impossible that stop is called after stop. So maybe this is a defect of W3C Spec.

And after audio rendering is complete, the call of stop will throw exception as well.

So maybe it will be more reasonable if we describe it like this:
start can be called only when playbackState is UNSCHEDULED_STATE, or INVALID_STATE_ERR exception will be thrown.
stop can be called only when playbackState is SCHEDULED_STATE or PLAYING_STATE, or NVALID_STATE_ERR exception will be thrown.

Hi Chris,
  Do you think it is reasonable? If yes, I will file a bug in W3C for the more discussion.

[Li Yin]

Hi Chirs,
  Do you think it is reasonable?

[Praveen Jadhav]

*** Bug 109674 has been marked as a duplicate of this bug. ***

[Li Yin]

Hi Praveen Jadhav,
According to the newest spec, the exception should be thrown, you can see from https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#AudioBufferSourceNode

[Praveen Jadhav]

(In reply to comment #7)
> Hi Praveen Jadhav,
> According to the newest spec, the exception should be thrown, you can see from https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#AudioBufferSourceNode

Well, I too was of same opinion earlier. But if you see the discussion we had in Bug 109674, "raises(DOMException)" is being phased out from Web IDL spec.

The exception mechanism is changed to use "DOMString" rather than "raises(DOMException)".

After going through your patch, I realised that it is identical to what I had proposed in bug 109674. Hence, I have made my bug as DUPLICATE.

[Li Yin]

I don't understand what you pointed, could you please add the detailed information?
Do you mean all the Web IDL spec won't raise DOMException or only for web audio spec?

[Praveen Jadhav]

(In reply to comment #9)
> I don't understand what you pointed, could you please add the detailed information?

For more details, you may refer to the discussion between Chris Rogers and Kentaro Hara in Bug 105058 and in Bug 105977.

> Do you mean all the Web IDL spec won't raise DOMException or only for web audio spec?

"raises(DOMException)" will be spec from Web IDL and not only from WebAudio(as per my understanding while discussing in Bug 109674).

Chris, any comments here?

[Praveen Jadhav]

(In reply to comment #10)
> (In reply to comment #9)
> > I don't understand what you pointed, could you please add the detailed information?
> 
> For more details, you may refer to the discussion between Chris Rogers and Kentaro Hara in Bug 105058 and in Bug 105977.
> 
> > Do you mean all the Web IDL spec won't raise DOMException or only for web audio spec?
> 
> "raises(DOMException)" will be spec from Web IDL and not only from WebAudio(as per my understanding while discussing in Bug 109674).
> 
> Chris, any comments here?

I meant "spec out" from Web IDL.

[Kentaro Hara]

Comment on attachment 174661 [details]
Patch

_The implementation looks good. Once the spec issue is resolved, I'm happy to r+ it.

[Kentaro Hara]

Comment on attachment 174661 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174661&action=review

> LayoutTests/webaudio/audiobuffersource-exception.html:11
> +<div id="description"></div>
> +<div id="console"></div>
> +

Nit: These are not necessary.

> LayoutTests/webaudio/audiobuffersource-exception.html:34
> +    if (window.testRunner) {
> +        testRunner.dumpAsText();
> +        testRunner.waitUntilDone();
> +    }
> +    

Nit: I think these are not necessary.

[Li Yin]

Created attachment 190413 [details]
Patch

[Li Yin]

(In reply to comment #12)
> (From update of attachment 174661 [details])
> _The implementation looks good. Once the spec issue is resolved, I'm happy to r+ it.

Thanks Kentaro's view. The new patch fixed some nits.

[Kentaro Hara]

crogers: WDYT about the patch from the perspective of the spec?

[Brent Fulgham]

Comment on attachment 190413 [details]
Patch

This patch doesn't apply against the current trunk sources. If you are interested in getting this patch integrated, please revise to match our current sources and upload a new patch. I apologize for the inconvenience.