Bug 68382 – Add settings to control the availability of the Web Audio API to WebKit and WebKit2.

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

[Jer Noble]

Add settings to control the availability of the Web Audio API to WebKit and WebKit2.

[Jer Noble]

Created attachment 107908 [details]
Patch

[Darin Adler]

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?

[Jer Noble]

(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.

[Jer Noble]

Created attachment 108009 [details]
Updated patch to address Darin's comment about the many lines of code dedicated to null-checks.

[Jer Noble]

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.

[Jer Noble]

Created attachment 108015 [details]
Patch

More LOC than the previous updated patch, but now with 100% fewer compile errors.

[Darin Adler]

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.

[Jer Noble]

(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!

[Jer Noble]

Committed r95678: <http://trac.webkit.org/changeset/95678>

[Adam Roben (:aroben)]

What about WebKit1 on Windows?

[Jer Noble]

(In reply to comment #10)
> What about WebKit1 on Windows?

Chris Rogers has already added a WebKit1 API for controlling this setting.