The WKTR interface needs to expose a setAudioData() method at least.
Which tests are affected by this? I couldn't find anything related in platform/wk2/Skipped.
The webaudio tests are affected by this, IIUC. It's odd they're not skipped on the mac port.
Oh, they're skipped from LayoutTests/platform/mac/Skipped, which is the generic Skipped list.
Created attachment 179455 [details] wip patch
Created attachment 179934 [details] Patch
Created attachment 179935 [details] Patch, v2. Credit for wip patch in ChangeLog
Comment on attachment 179935 [details] Patch, v2. Credit for wip patch in ChangeLog View in context: https://bugs.webkit.org/attachment.cgi?id=179935&action=review Looks good but I'd rather defer official review to a more experienced WK2 reviewer. > Tools/WebKitTestRunner/TestInvocation.cpp:312 > + printf("Content-Type: %s\n", "audio/wav"); Format string looks useless here. > Tools/WebKitTestRunner/TestInvocation.cpp:326 > + printf("#EOF\n"); > + fprintf(stderr, "#EOF\n"); Out of curiosity, why both are needed?
Comment on attachment 179935 [details] Patch, v2. Credit for wip patch in ChangeLog View in context: https://bugs.webkit.org/attachment.cgi?id=179935&action=review > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp:272 > +WKDataRef WKBundleDataFromUint8Array(WKBundleRef bundle, JSContextRef context, JSValueRef data) isnt this normally called UInt? Yeah, apparently both are used in WebCore, but UInt is what is used in /WebKit2 > Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:185 > + // Audio testing dot at end > Tools/WebKitTestRunner/TestInvocation.cpp:294 > + if (m_textOutput.length()) > + dump(m_textOutput.toString().utf8().data()); > + else if (m_audioResult) > + dumpAudio(m_audioResult.get()); You never need to dump both? assert? > Tools/WebKitTestRunner/TestInvocation.cpp:312 > + printf("Content-Type: %s\n", "audio/wav"); Why compose it?
Comment on attachment 179935 [details] Patch, v2. Credit for wip patch in ChangeLog Attachment 179935 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15418115
Comment on attachment 179935 [details] Patch, v2. Credit for wip patch in ChangeLog Attachment 179935 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15402224
Created attachment 179961 [details] Patch, v3, review comments addressed.
Comment on attachment 179961 [details] Patch, v3, review comments addressed. Attachment 179961 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15408230
Created attachment 179967 [details] Patch, v4, review comments addressed.
(In reply to comment #7) > (From update of attachment 179935 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=179935&action=review > > Looks good but I'd rather defer official review to a more experienced WK2 reviewer. Thanks. > > Tools/WebKitTestRunner/TestInvocation.cpp:312 > > + printf("Content-Type: %s\n", "audio/wav"); > > Format string looks useless here. It does, replaced with fixed string. > > Tools/WebKitTestRunner/TestInvocation.cpp:326 > > + printf("#EOF\n"); > > + fprintf(stderr, "#EOF\n"); > > Out of curiosity, why both are needed? webkitpy expects an #EOF on stderr - that's the same way as Chromium's TestShell outputs it. (In reply to comment #8) > (From update of attachment 179935 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=179935&action=review > > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp:272 > > +WKDataRef WKBundleDataFromUint8Array(WKBundleRef bundle, JSContextRef context, JSValueRef data) > > isnt this normally called UInt? Yeah, apparently both are used in WebCore, but UInt is what is used in /WebKit2 Renamed to UInt, uppercased variant. > > Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:185 > > + // Audio testing > > dot at end Done. > > Tools/WebKitTestRunner/TestInvocation.cpp:294 > > + if (m_textOutput.length()) > > + dump(m_textOutput.toString().utf8().data()); > > + else if (m_audioResult) > > + dumpAudio(m_audioResult.get()); > > You never need to dump both? assert? Unfortuntely, run-webkit-tests accepts only either text, or audio for the first element in dumped data and can only compare one. We give text priority, but we can't assert for audio to be empty at the same time.
(In reply to comment #12) > (From update of attachment 179961 [details]) > Attachment 179961 [details] did not pass qt-wk2-ews (qt): > Output: http://queues.webkit.org/results/15408230 Ossy, could you help me with this one? Seems to be that the newly used JSUint8Array.h header is not included in the step that generates forwarding headers.
Comment on attachment 179967 [details] Patch, v4, review comments addressed. Attachment 179967 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15411204
Comment on attachment 179967 [details] Patch, v4, review comments addressed. Attachment 179967 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15411267
Created attachment 180135 [details] Patch v4, mac & qt build fixes.
Comment on attachment 180135 [details] Patch v4, mac & qt build fixes. Attachment 180135 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15411593
Created attachment 180149 [details] Patch v5, hopefully last qt build fix
Comment on attachment 180149 [details] Patch v5, hopefully last qt build fix Attachment 180149 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15402719
Created attachment 180159 [details] Patch, v6, more qt build fixing. thanks ossy.
Comment on attachment 180159 [details] Patch, v6, more qt build fixing. thanks ossy. View in context: https://bugs.webkit.org/attachment.cgi?id=180159&action=review > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp:272 > +WKDataRef WKBundleDataFromUInt8Array(WKBundleRef bundle, JSContextRef context, JSValueRef data) This returns a new WKDataRef and should have Create in its name.
Comment on attachment 180159 [details] Patch, v6, more qt build fixing. thanks ossy. View in context: https://bugs.webkit.org/attachment.cgi?id=180159&action=review > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:439 > + WKDataRef audioData = WKBundleDataFromUInt8Array(InjectedBundle::shared().bundle(), context, data); Missing adoptWK().
Created attachment 180164 [details] Patch, v7, new review comments addressed.
Comment on attachment 180164 [details] Patch, v7, new review comments addressed. LGTM.
Comment on attachment 180164 [details] Patch, v7, new review comments addressed. Clearing flags on attachment: 180164 Committed r138232: <http://trac.webkit.org/changeset/138232>
All reviewed patches have been landed. Closing bug.
Weird regression: bug 105623
Comment on attachment 180164 [details] Patch, v7, new review comments addressed. View in context: https://bugs.webkit.org/attachment.cgi?id=180164&action=review > Tools/WebKitTestRunner/TestInvocation.cpp:294 > - dump(m_textOutput.toString().utf8().data()); > + if (m_textOutput.length()) > + dump(m_textOutput.toString().utf8().data()); > + else if (m_audioResult) > + dumpAudio(m_audioResult.get()); Maybe the regression is related with this change? Just an unverified hunch though.