Summary: | [GTK] WebAudio DRT support | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Philippe Normand <pnormand> | ||||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||||
Severity: | Normal | CC: | commit-queue, crogers, eric.carlson, feature-media-reviews, gustavo, jer.noble, philn, pnormand, rakuco, s.choi, webkit.review.bot, xan.lopez, zan | ||||||||||||
Priority: | P2 | Keywords: | Gtk, LayoutTestFailure | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 86914 | ||||||||||||||
Bug Blocks: | 61355, 77638, 78095 | ||||||||||||||
Attachments: |
|
Description
Philippe Normand
2011-08-30 02:29:18 PDT
I have a patch, will get it to run on Mac and upload. (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... Created attachment 122731 [details]
DRT API adaptations
Created attachment 122732 [details]
DRT WebAudio support in GTK
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? (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 :) 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 on attachment 122731 [details] DRT API adaptations Attachment 122731 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11396821 Comment on attachment 122731 [details] DRT API adaptations Attachment 122731 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11393864 (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. (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? Created attachment 126058 [details]
DRT API adaptations
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.
Chris can you please have a look at this patch? Created attachment 127208 [details]
DRT API adaptations
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. 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.
There doesn't seem to be anything GTK-specific in DumpRenderTreeSupportGtk::jsValueToAudioDataBuffer(). Could we move it somewhere more cross-platform? (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. (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. Created attachment 148507 [details] Patch Won't build without patch from bug 86914 though. 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 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.
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 :) |