Bug 67187 - [GTK] WebAudio DRT support
: [GTK] WebAudio DRT support
Status: RESOLVED WONTFIX
Product: WebKit
Classification: Unclassified
Component: WebKit Gtk
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Nobody
: Gtk, LayoutTestFailure
Depends on: 86914
Blocks: 61355 77638 78095
  Show dependency treegraph
 
Reported: 2011-08-30 02:29 PDT by Philippe Normand
Modified: 2013-11-07 02:57 PST (History)
13 users (show)

See Also:


Attachments
DRT API adaptations (4.91 KB, patch)
2012-01-17 01:14 PST, Philippe Normand
gyuyoung.kim: commit‑queue-
Details | Formatted Diff | Diff
DRT WebAudio support in GTK (6.98 KB, patch)
2012-01-17 01:14 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
DRT API adaptations (16.19 KB, patch)
2012-02-08 04:49 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
DRT API adaptations (16.19 KB, patch)
2012-02-15 11:51 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (1.95 MB, patch)
2012-06-19 22:40 PDT, Philippe Normand
ap: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2011-08-30 02:29:18 PDT
Since http://trac.webkit.org/changeset/91708 the LayoutTestController API changed but was updated only for Chromium.

Audio data is no longer stored b64-encoded.
Comment 1 Philippe Normand 2011-09-05 09:58:08 PDT
I have a patch, will get it to run on Mac and upload.
Comment 2 Philippe Normand 2011-11-15 04:25:29 PST
(In reply to comment #1)
> I have a patch, will get it to run on Mac and upload.

I'm stuck on this :(

The LayoutTestController receives a UInt8Array from Javascript, which is mapped to an ArrayBufferView, like in Chromium. The problem is that I can't seem to get access to toArrayBufferView() because I can't access JSArrayBufferView.h from DRT.

I could use the DOM bindings but I need access to ::baseAddress() which obviously is not exposed in the ArrayBufferView interface...
Comment 3 Philippe Normand 2012-01-17 01:14:43 PST
Created attachment 122731 [details]
DRT API adaptations
Comment 4 Philippe Normand 2012-01-17 01:14:56 PST
Created attachment 122732 [details]
DRT WebAudio support in GTK
Comment 5 Martin Robinson 2012-01-17 10:01:42 PST
Comment on attachment 122732 [details]
DRT WebAudio support in GTK

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

Looks nice apart from some style nits. What's necessary to get this working WKTR?

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:535
> +        if (gLayoutTestController->dumpAsAudio()) {
> +            dumpAudio();
> +        } else if (gLayoutTestController->dumpAsText())

No curly braces are ncessary here since the interior block is only one line.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:550
> +        if (!gLayoutTestController->dumpAsAudio()) {

Would an early return above prevent this?
Comment 6 Philippe Normand 2012-01-18 04:18:16 PST
(In reply to comment #5)
> (From update of attachment 122732 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122732&action=review
> 
> Looks nice apart from some style nits. What's necessary to get this working WKTR?
> 

No idea, I actually haven't tested WebAudio in WebKit2 yet :)
Comment 7 WebKit Review Bot 2012-02-02 06:05:04 PST
Attachment 122731 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/DumpRenderTree/LayoutTestController...." exit_code: 1

Tools/DumpRenderTree/LayoutTestController.h:293:  The parameter name "context" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/LayoutTestController.h:293:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Gyuyoung Kim 2012-02-02 06:32:27 PST
Comment on attachment 122731 [details]
DRT API adaptations

Attachment 122731 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11396821
Comment 9 Gustavo Noronha (kov) 2012-02-02 13:08:51 PST
Comment on attachment 122731 [details]
DRT API adaptations

Attachment 122731 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11393864
Comment 10 Philippe Normand 2012-02-02 13:35:58 PST
(In reply to comment #9)
> (From update of attachment 122731 [details])
> Attachment 122731 [details] did not pass gtk-ews (gtk):
> Output: http://queues.webkit.org/results/11393864

I need to merge the two patches.
A bit surprised mac-ews didn't complain though.
Comment 11 Philippe Normand 2012-02-07 08:46:53 PST
(In reply to comment #5)
> (From update of attachment 122732 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122732&action=review
> 
> 
> > Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:550
> > +        if (!gLayoutTestController->dumpAsAudio()) {
> 
> Would an early return above prevent this?

Hum but what about the if (printSeparators) test below?
Comment 12 Philippe Normand 2012-02-08 04:49:13 PST
Created attachment 126058 [details]
DRT API adaptations
Comment 13 WebKit Review Bot 2012-02-08 04:50:53 PST
Attachment 126058 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/gtk/ChangeLog', u'Source/Web..." exit_code: 1

Tools/DumpRenderTree/LayoutTestController.h:293:  The parameter name "context" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/LayoutTestController.h:293:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Philippe Normand 2012-02-15 02:31:04 PST
Chris can you please have a look at this patch?
Comment 15 Philippe Normand 2012-02-15 11:51:37 PST
Created attachment 127208 [details]
DRT API adaptations
Comment 16 Chris Rogers 2012-02-21 17:54:12 PST
Philippe, sorry it's taking me so long.  I'm not sure I'll be able to review all of the parts of this patch (the low-level GTK part and JSC bindings).  But, I'll try to have a look at details of the dump() method as soon as I can.
Comment 17 WebKit Commit Bot 2012-02-24 00:41:18 PST
Attachment 127208 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/gtk/ChangeLog', u'Source/Web..." exit_code: 1
Tools/DumpRenderTree/LayoutTestController.h:293:  The parameter name "context" adds no information, so it should be removed.  [readability/parameter_name] [5]
Tools/DumpRenderTree/LayoutTestController.h:293:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Jer Noble 2012-06-04 10:43:16 PDT
There doesn't seem to be anything GTK-specific in DumpRenderTreeSupportGtk::jsValueToAudioDataBuffer().  Could we move it somewhere more cross-platform?
Comment 19 Philippe Normand 2012-06-04 22:28:25 PDT
(In reply to comment #18)
> There doesn't seem to be anything GTK-specific in DumpRenderTreeSupportGtk::jsValueToAudioDataBuffer().  Could we move it somewhere more cross-platform?

Yes, that would be great! I think I initially put that code there because I wasn't able to get it working on mac but it seems your patch would help a lot getting there.

Sorry I don't have much time to work on this bug though :( I'll pull the patch out of the review queue for now.
Comment 20 Jer Noble 2012-06-06 15:24:50 PDT
(In reply to comment #19)
> (In reply to comment #18)
> > There doesn't seem to be anything GTK-specific in DumpRenderTreeSupportGtk::jsValueToAudioDataBuffer().  Could we move it somewhere more cross-platform?
> 
> Yes, that would be great! I think I initially put that code there because I wasn't able to get it working on mac but it seems your patch would help a lot getting there.

Unfortunately, I don't see an alternative to using the port-specific WebCoreTestSupport/ files. :(  There's just no shared location to put this function.
Comment 21 Philippe Normand 2012-06-19 22:40:49 PDT
Created attachment 148507 [details]
Patch

Won't build without patch from bug 86914 though.
Comment 22 WebKit Review Bot 2012-06-19 22:46:53 PDT
Attachment 148507 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:75:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 Alexey Proskuryakov 2013-10-19 20:47:08 PDT
Comment on attachment 148507 [details]
Patch

Typed arrays moved from WebCore to JavaScriptCore since, so this patch will need to be revised along the lines of what I just landed for Mac.

In fact, it would be ideal to make the code for writing results to stdout cross-platform, there is really nothing platform dependent here.
Comment 24 Philippe Normand 2013-11-07 02:57:56 PST
I'm not going to work on this further. The WK2 GTK port has been running the webaudio tests for a while now, this is all that matters to me :)