Bug 106651

Summary: [Qt][WK1] Web Audio support
Product: WebKit Reporter: Allan Sandfeld Jensen <allan.jensen>
Component: Web AudioAssignee: Allan Sandfeld Jensen <allan.jensen>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, crogers, jturcotte, s.choi, vestbo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 110211    
Attachments:
Description Flags
Patch
none
Patch
none
Patch jturcotte: review+

Description Allan Sandfeld Jensen 2013-01-11 03:18:44 PST
If WebAudio is compiled in, it can currently only be used by the WebKit2 API. For WebKit1 support, we need to be able to enable it at runtime, and support audio-testing in DumpRenderTree.
Comment 1 Allan Sandfeld Jensen 2013-01-11 03:25:16 PST
Created attachment 182315 [details]
Patch
Comment 2 Allan Sandfeld Jensen 2013-01-11 04:40:27 PST
Created attachment 182321 [details]
Patch

Make WebAudio configurable from QtTestBrowser, and enable LEGACY_WEB_AUDIO which is needed for the majority of Web Audio tests.
Comment 3 Jocelyn Turcotte 2013-01-11 05:09:15 PST
Comment on attachment 182321 [details]
Patch

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

> Source/WebCore/bridge/qt/qt_runtime.cpp:509
> +            } else if (type == RTUint8Array) {
> +                WTF::Uint8Array* arr = toUint8Array(toJS(toJS(context), value));
> +                ret = QVariant(QByteArray(reinterpret_cast<const char*>(arr->data()), arr->length()));
> +                dist = 0;

Uint8ClampedArray is a child class of Uint8Array, could you merge the two cases?

> Source/WebKit/qt/Api/qwebsettings.cpp:541
> +    d->attributes.insert(QWebSettings::WebAudioEnabled, false);

Should this be enabled by default like other HTML5 features?
On one side I'm not sure if it's good to change the default value afterward.
On the other side we can't enable it by default if we only have it working on Linux.

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:931
> +        fprintf(stdout, "Content-Type: %s\n", resultContentType.toUtf8().constData());

This could be a good use of qPrintable.

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:942
> +        fprintf(stdout, "Content-Type: %s\n", resultContentType.toUtf8().constData());

ditto

> Tools/QtTestBrowser/launcherwindow.cpp:219
> -    settings->setAttribute(QWebSettings::WebGLEnabled, m_windowOptions.useWebGL);
> +    m_windowOptions.useWebGL = settings->testAttribute(QWebSettings::WebGLEnabled);

I think that this will break the -webgl command line option.
Comment 4 Allan Sandfeld Jensen 2013-01-11 05:34:19 PST
(In reply to comment #3)
> (From update of attachment 182321 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182321&action=review
> 
> > Source/WebCore/bridge/qt/qt_runtime.cpp:509
> > +            } else if (type == RTUint8Array) {
> > +                WTF::Uint8Array* arr = toUint8Array(toJS(toJS(context), value));
> > +                ret = QVariant(QByteArray(reinterpret_cast<const char*>(arr->data()), arr->length()));
> > +                dist = 0;
> 
> Uint8ClampedArray is a child class of Uint8Array, could you merge the two cases?
> 
It is possible, at least from JS -> Qt it should be safe. The other way we still want to get the clampedarray.

> > Source/WebKit/qt/Api/qwebsettings.cpp:541
> > +    d->attributes.insert(QWebSettings::WebAudioEnabled, false);
> 
> Should this be enabled by default like other HTML5 features?
> On one side I'm not sure if it's good to change the default value afterward.
> On the other side we can't enable it by default if we only have it working on Linux.
> 
True. I want to start by having it off by default until we know if it works on more than linux. Note that this patch still doesn't enable the feature at compile time.

> > Tools/QtTestBrowser/launcherwindow.cpp:219
> > -    settings->setAttribute(QWebSettings::WebGLEnabled, m_windowOptions.useWebGL);
> > +    m_windowOptions.useWebGL = settings->testAttribute(QWebSettings::WebGLEnabled);
> 
> I think that this will break the -webgl command line option.

I hadn't thought of that, but I guess it still works. WebGL is enabled by default these days.
Comment 5 Allan Sandfeld Jensen 2013-01-11 05:46:43 PST
Created attachment 182327 [details]
Patch
Comment 6 Allan Sandfeld Jensen 2013-01-11 06:39:08 PST
Committed r139437: <http://trac.webkit.org/changeset/139437>