Bug 49115 – Add custom bindings for AudioContext

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Chris Rogers]

Add custom bindings for AudioContext

[Chris Rogers]

Created attachment 73147 [details]
Patch

[Kenneth Russell]

Comment on attachment 73147 [details]
Patch

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

> WebCore/bindings/v8/custom/V8AudioContextCustom.cpp:43
> +        return throwError("AudioContext constructor associated frame is unavailable", V8Proxy::ReferenceError);

Why is there a difference between the JS and V8 bindings in whether these values are null-checked at run time? Please make the two bindings work identically. If this isn't supposed to happen then make it an ASSERT.

[Chris Rogers]

Created attachment 73329 [details]
Patch

[Chris Rogers]

I've added error checking in the JS custom binding as well.

[Kenneth Russell]

Comment on attachment 73329 [details]
Patch

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

> WebCore/bindings/js/JSAudioContextCustom.cpp:40
> +      return throwError(exec, createSyntaxError(exec, "AudioContext constructor associated document is unavailable"));

The V8 bindings are throwing a ReferenceError, so to be consistent you should do the same here and below.

> WebCore/bindings/js/JSAudioContextCustom.cpp:42
> +    Document* document = static_cast<Document*>(jsConstructor->scriptExecutionContext());

From looking at some of the other custom bindings, you need to check ScriptExecutionContext::isDocument() before casting to Document* to be safe. Since you're already doing the other checking you might as well do this too.

[Chris Rogers]

Created attachment 73335 [details]
Patch

[Chris Rogers]

new patch addresses all comments

[Kenneth Russell]

Comment on attachment 73335 [details]
Patch

Looks good to me. Thanks for bearing with me on the review.

[WebKit Commit Bot]

Comment on attachment 73335 [details]
Patch

Clearing flags on attachment: 73335

Committed r71619: <http://trac.webkit.org/changeset/71619>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.