Summary: | Add settings to control the availability of the Web Audio API to WebKit and WebKit2. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aroben, darin | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Jer Noble
2011-09-19 13:19:40 PDT
Created attachment 107908 [details]
Patch
Comment on attachment 107908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107908&action=review > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:616 > #if ENABLE(WEB_AUDIO) > JSValue JSDOMWindow::webkitAudioContext(ExecState* exec) const > { > + Frame* frame = impl()->frame(); > + if (!frame) > + return jsUndefined(); > + Settings* settings = frame->settings(); > + if (!settings || !settings->webAudioEnabled()) > + return jsUndefined(); > return getDOMConstructor<JSAudioContextConstructor>(exec, this); > } > #endif I wish there was a tighter way to implement this so the null checks didn’t take so many lines of code. Does the webkitAudioContext property still show up if you iterate properties? (In reply to comment #2) > (From update of attachment 107908 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=107908&action=review > > > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:616 > > #if ENABLE(WEB_AUDIO) > > JSValue JSDOMWindow::webkitAudioContext(ExecState* exec) const > > { > > + Frame* frame = impl()->frame(); > > + if (!frame) > > + return jsUndefined(); > > + Settings* settings = frame->settings(); > > + if (!settings || !settings->webAudioEnabled()) > > + return jsUndefined(); > > return getDOMConstructor<JSAudioContextConstructor>(exec, this); > > } > > #endif > > I wish there was a tighter way to implement this so the null checks didn’t take so many lines of code. I do as well. This same pattern is used a couple of other places in this file; perhaps it could be pulled out into a file-static helper function? I'll see what I can do. > Does the webkitAudioContext property still show up if you iterate properties? Just checked, and yes it still does. I used the same pattern as we're using for the WebGL setting, and the WebGL properties still appear on Window as well. Created attachment 108009 [details]
Updated patch to address Darin's comment about the many lines of code dedicated to null-checks.
Comment on attachment 108009 [details]
Updated patch to address Darin's comment about the many lines of code dedicated to null-checks.
Whoops, this will never work. Uploading a new patch soon.
Created attachment 108015 [details]
Patch
More LOC than the previous updated patch, but now with 100% fewer compile errors.
Comment on attachment 108015 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108015&action=review I think this version of runtime-enabled properties is not good. The feature will still seem to be there for people who use best practices such as: if (webkitAudioContext in window) I think that needs to be resolved. I believe V8 currently has a better version of runtime-enabled features. > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:610 > + Frame* frame = impl()->frame(); > + Settings* settings = frame ? frame->settings() : 0; We could make a helper function to get settings and so remove one line of code from all of these. (In reply to comment #7) > (From update of attachment 108015 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=108015&action=review > > I think this version of runtime-enabled properties is not good. The feature will still seem to be there for people who use best practices such as: > > if (webkitAudioContext in window) > > I think that needs to be resolved. I believe V8 currently has a better version of runtime-enabled features. Indeed. I think the only way to do this is to manually handle each runtime-enabled property in JSDOMWindow::getOwnPropertySlot(). But I think Sam might have an idea about this; I'll talk to him. > > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:610 > > + Frame* frame = impl()->frame(); > > + Settings* settings = frame ? frame->settings() : 0; > > We could make a helper function to get settings and so remove one line of code from all of these. Will do. Thanks! Committed r95678: <http://trac.webkit.org/changeset/95678> What about WebKit1 on Windows? (In reply to comment #10) > What about WebKit1 on Windows? Chris Rogers has already added a WebKit1 API for controlling this setting. |