Bug 75293 - Use IDL overloads in AudioContext.idl for createBuffer in the case of V8 binding
Summary: Use IDL overloads in AudioContext.idl for createBuffer in the case of V8 binding
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-27 23:51 PST by Raymond
Modified: 2011-12-29 18:29 PST (History)
5 users (show)

See Also:

Attachments
Patch (9.69 KB, patch)
2011-12-28 01:22 PST, Raymond 		no flags Details | Formatted Diff | Diff
Patch (9.95 KB, patch)
2011-12-28 19:17 PST, Raymond 		no flags Details | Formatted Diff | Diff
Patch (10.14 KB, patch)
2011-12-28 20:36 PST, Raymond 		no flags Details | Formatted Diff | Diff
Patch (9.92 KB, patch)
2011-12-28 20:41 PST, Raymond 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Raymond 2011-12-27 23:51:01 PST 
Use IDL overloads in AudioContext.idl for createBuffer in the case of V8 binding
Comment 1 Raymond 2011-12-28 01:22:29 PST 
Created attachment 120642 [details]
Patch
Comment 2 Adam Barth 2011-12-28 11:20:33 PST 
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.
Comment 3 Raymond 2011-12-28 19:17:22 PST 
Created attachment 120713 [details]
Patch
Comment 4 Raymond 2011-12-28 19:21:59 PST 
(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.
Comment 5 Raymond 2011-12-28 20:08:55 PST 
oops, need to update the commit log too
Comment 6 Raymond 2011-12-28 20:36:30 PST 
Created attachment 120716 [details]
Patch
Comment 7 Raymond 2011-12-28 20:41:02 PST 
Created attachment 120718 [details]
Patch
Comment 8 Adam Barth 2011-12-28 21:28:23 PST 
> 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.
Comment 9 Raymond 2011-12-29 16:58:31 PST 
(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.
Comment 10 WebKit Review Bot 2011-12-29 18:29:00 PST 
Comment on attachment 120718 [details]
Patch

Clearing flags on attachment: 120718

Committed r103832: <http://trac.webkit.org/changeset/103832>
Comment 11 WebKit Review Bot 2011-12-29 18:29:07 PST 
All reviewed patches have been landed.  Closing bug.