Bug 75293 – Use IDL overloads in AudioContext.idl for createBuffer in the case of V8 binding

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Raymond]

Use IDL overloads in AudioContext.idl for createBuffer in the case of V8 binding

[Raymond]

Created attachment 120642 [details]
Patch

[Adam Barth]

Comment on attachment 120642 [details]
Patch

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

> Source/WebCore/bindings/js/JSAudioContextCustom.cpp:128
> +                setDOMException(exec, ec);
>                  return throwError(exec, createSyntaxError(exec, "Error decoding audio file data"));

Aren't these redundant?

> Source/WebCore/webaudio/AudioContext.cpp:330
> +fail:
> +    ec = SYNTAX_ERR;
> +    return 0;

I would have just repeated these two lines rather than use goto.

> Source/WebCore/webaudio/AudioContext.idl:54
> +        // FIXME: Remove 'else' once JSC supports overloads too.

JSC doesn't support overloads?  I thought it did.

[Raymond]

Created attachment 120713 [details]
Patch

[Raymond]

(In reply to comment #2)

Thanks for your review , patch updated according to your comments.

> 
> > Source/WebCore/webaudio/AudioContext.idl:54
> > +        // FIXME: Remove 'else' once JSC supports overloads too.
> 
> JSC doesn't support overloads?  I thought it did.

Ok. I am following other part of code in canvasRenderContext2D, hmm, so that part of code also need to be fixed. Thx.

[Raymond]

oops, need to update the commit log too

[Raymond]

Created attachment 120716 [details]
Patch

[Raymond]

Created attachment 120718 [details]
Patch

[Adam Barth]

> Ok. I am following other part of code in canvasRenderContext2D, hmm, so that part of code also need to be fixed. Thx.

Would you be willing to fix those cases as well (in a follow-up bug)?  That way folks in the future won't run into this same problem.

[Raymond]

(In reply to comment #8)

> 
> Would you be willing to fix those cases as well (in a follow-up bug)?  That way folks in the future won't run into this same problem.

OK, sure. Will try to scan through the code to find is there any more remains.

[WebKit Review Bot]

Comment on attachment 120718 [details]
Patch

Clearing flags on attachment: 120718

Committed r103832: <http://trac.webkit.org/changeset/103832>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.