Bug 67187 - [GTK] WebAudio DRT support
: [GTK] WebAudio DRT support
Status: RESOLVED WONTFIX
: WebKit
WebKit Gtk
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
: Gtk, LayoutTestFailure
: 86914
: 61355 77638 78095
  Show dependency treegraph
 
Reported: 2011-08-30 02:29 PST by
Modified: 2013-11-07 02:57 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-08-30 02:29:18 PST
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 From 2011-09-05 09:58:08 PST -------
I have a patch, will get it to run on Mac and upload.
------- Comment #2 From 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 From 2012-01-17 01:14:43 PST -------
Created an attachment (id=122731) [details]
DRT API adaptations
------- Comment #4 From 2012-01-17 01:14:56 PST -------
Created an attachment (id=122732) [details]
DRT WebAudio support in GTK
------- Comment #5 From 2012-01-17 10:01:42 PST -------
(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?

> 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 From 2012-01-18 04:18:16 PST -------
(In reply to comment #5)
> (From update of attachment 122732 [details] [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 From 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 From 2012-02-02 06:32:27 PST -------
(From update of attachment 122731 [details])
Attachment 122731 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11396821
------- Comment #9 From 2012-02-02 13:08:51 PST -------
(From update of attachment 122731 [details])
Attachment 122731 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11393864
------- Comment #10 From 2012-02-02 13:35:58 PST -------
(In reply to comment #9)
> (From update of attachment 122731 [details] [details])
> Attachment 122731 [details] [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 From 2012-02-07 08:46:53 PST -------
(In reply to comment #5)
> (From update of attachment 122732 [details] [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 From 2012-02-08 04:49:13 PST -------
Created an attachment (id=126058) [details]
DRT API adaptations
------- Comment #13 From 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 From 2012-02-15 02:31:04 PST -------
Chris can you please have a look at this patch?
------- Comment #15 From 2012-02-15 11:51:37 PST -------
Created an attachment (id=127208) [details]
DRT API adaptations
------- Comment #16 From 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 From 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 From 2012-06-04 10:43:16 PST -------
There doesn't seem to be anything GTK-specific in DumpRenderTreeSupportGtk::jsValueToAudioDataBuffer().  Could we move it somewhere more cross-platform?
------- Comment #19 From 2012-06-04 22:28:25 PST -------
(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 From 2012-06-06 15:24:50 PST -------
(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 From 2012-06-19 22:40:49 PST -------
Created an attachment (id=148507) [details]
Patch

Won't build without patch from bug 86914 though.
------- Comment #22 From 2012-06-19 22:46:53 PST -------
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 From 2013-10-19 20:47:08 PST -------
(From update of attachment 148507 [details])
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 From 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 :)