Bug 57977 – Add first basic layout test for the web audio API

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Chris Rogers]

Add first basic layout test for the web audio API

[Chris Rogers]

Created attachment 88501 [details]
Patch

[Peter Beverloo]

Quick nit: the proper syntax for comments in HTML would be:
<!-- text -->

[Chris Rogers]

Created attachment 88505 [details]
Patch

[Chris Rogers]

Peter, thanks for the quick check!  I moved the comment from the <script> section to the top of the file at the last minute and forgot to re-run my test...

new patch uploaded with the comment fix

[Chris Rogers]

obsoleting this patch -- moving to be part of:
https://bugs.webkit.org/show_bug.cgi?id=57969

[Tony Chang]

*** This bug has been marked as a duplicate of bug 57969 ***

[Kenneth Russell]

I can't easily follow the train of updates between these bugs, but at this point did you mean to have this closed as a dup of 57969? If not, please reopen this and re-request review on the patch.

[Tony Chang]

Sorry, undupping.

I didn't realize this test required changes to run-webkit-tests.

[Chris Rogers]

Created attachment 100716 [details]
Patch

[Kenneth Russell]

Comment on attachment 100716 [details]
Patch

Please rebase the patch against TOT.

[Chris Rogers]

Created attachment 100744 [details]
Patch

[WebKit Review Bot]

Comment on attachment 100744 [details]
Patch

Attachment 100744 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9016765

New failing tests:
webaudio/test-basic.html

[WebKit Review Bot]

Created attachment 100749 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Kenneth Russell]

Comment on attachment 100744 [details]
Patch

From the layout test results archive it looks like the new test failed on the Chromium Linux bot. r-'ing the patch until this is diagnosed.

[Tony Chang]

A few comments:

1) audio-testing.js should be in a subdirectory called resources (LayoutTests/webaudio/resources/audio-testing.js).
2) The stderr output in DefaultAudioDestinationNode.cpp in debug builds is kind of annoying, but I guess it's not a big deal.
3) The test is timing out in debug.  On my machine, it takes 82s to complete.  It looks like the hot part of the code is some debugging code in V8.   Here's a sample stack (for reference, the string is 940,860 characters long):

#0  0x00000000009bd953 in v8::internal::HeapObject::map_word (this=
    0x7fffea08a339) at v8/src/objects-inl.h:1222
#1  0x00000000009bd8fa in v8::internal::HeapObject::map (this=0x7fffea08a339)
    at v8/src/objects-inl.h:1212
#2  0x00000000009bc692 in v8::internal::Object::IsConsString (this=
    0x7fffea08a339) at v8/src/objects-inl.h:175
#3  0x00000000009be10e in v8::internal::ConsString::cast (object=0x7fffea08a339)
    at v8/src/objects-inl.h:2027
#4  0x0000000000b1d699 in v8::internal::ConsString::ConsStringReadBlock (this=
    0x7fffead80499, rbb=0x7fffffffb880, offset_ptr=0x7ffff7e4f698, max_chars=
    580714) at v8/src/objects.cc:5365
#5  0x0000000000b1dadf in v8::internal::String::ReadBlock (input=
    0x7fffead80499, rbb=0x7fffffffb880, offset_ptr=0x7ffff7e4f698, max_chars=
    920380) at v8/src/objects.cc:5530
#6  0x0000000000b070c8 in v8::internal::String::ReadBlock (input=
    0x7fffead80499, util_buffer=
    0x7ffff7e4f6a8 "2cV1fAu4V3pfq713B6CsdDCWJXEDjixtoIfFaCCD9WOXgMFeDYAtWYaBQFP7hP5MX4pwRpuRmj+TmoU4IqU3MR+xtylZvg0imsxBGqrbWxJN62IKQvteAkoLWfolG1nykypm6lY5iuIyR8va8FMx011fxctLaY3EkXGRvQ942LaqfGiwUX9Iqvh/f6ScfhGfQ3sFmvt1"..., capacity=1024, 
    remaining=0x7ffff7e4f688, offset_ptr=0x7ffff7e4f698)
    at v8/src/objects.cc:5702
#7  0x0000000000ac2535 in unibrow::InputBuffer<v8::internal::String, v8::internal::String*, 1024u>::FillBuffer (this=0x7ffff7e4f680) at v8/src/unicode-inl.h:202
#8  0x00000000009baa30 in unibrow::CharacterStream::GetNext (this=
    0x7ffff7e4f680) at v8/src/unicode-inl.h:132
#9  0x00000000009b0466 in v8::String::WriteUtf8 (this=0x7fffffffbf60, buffer=
    0x7fffbf481000 "UklGRmTECgBXQVZFZm10IBAAAAABAAIARKwAABCxAgAEABAAZGF0YUDECgAAAAAAARAECMIfARAEL+4XiT3CHxZLdyd2VwQvdmJiNupriT2tc3JEn3kWS6h9b1G4f3ZXxn8lXdN9dmLneWRnEHTqa2dsBHAKY61zIFjidtNLn3lVPuJ73S+ofaQg737pELh/6QD/f+bw"..., capacity=-1, 
    nchars_ref=0x0, hints=v8::String::NO_HINTS) at v8/src/api.cc:3655
#10 0x00000000009b62f3 in v8::String::Utf8Value::Utf8Value (this=
    0x7fffffffbad0, obj=...) at v8/src/api.cc:5104
#11 0x00000000010f5e08 in WebCore::convertV8ObjectToNPVariant (object=..., 
    owner=0x7fffc9788be0, result=0x7fffc588b120)
    at third_party/WebKit/Source/WebCore/bindings/v8/V8NPUtils.cpp:65
#12 0x00000000010ef1f9 in WebCore::npObjectInvokeImpl (args=..., functionId=
    WebCore::InvokeMethod)
    at third_party/WebKit/Source/WebCore/bindings/v8/V8NPObject.cpp:107
#13 0x00000000010ef468 in WebCore::npObjectMethodHandler (args=...)
    at third_party/WebKit/Source/WebCore/bindings/v8/V8NPObject.cpp:149
#14 0x00000000009e50de in v8::internal::HandleApiCallHelper<false> (args=..., 
    isolate=0x7ffff7e55000) at v8/src/builtins.cc:1105
#15 0x00000000009dfd03 in v8::internal::Builtin_Impl_HandleApiCall (args=..., 
    isolate=0x7ffff7e55000) at v8/src/builtins.cc:1122
#16 0x00000000009dfcd4 in v8::internal::Builtin_HandleApiCall (args=..., 
    isolate=0x7ffff7e55000) at v8/src/builtins.cc:1121


Mads might know why it's slow.  We might just want to skip the test in debug.

[Chris Rogers]

Tony, thanks for having a look.  Is this working OK for you in the Linux Release build?  In other words, is the reason that "cr-linux" is red because of the Debug build?

[Tony Chang]

(In reply to comment #16)
> Tony, thanks for having a look.  Is this working OK for you in the Linux Release build?  In other words, is the reason that "cr-linux" is red because of the Debug build?

Aside from timing out for me in Debug, the test passes for me (both in Release and Debug if I disable the timeout).

I'm not sure why it's red on the bots.  The ews bots run in Release.  Maybe try uploading it again seeing if it'll go green?

[Chris Rogers]

Created attachment 101229 [details]
Patch

[Chris Rogers]

uploaded new patch fixing Tony's comment (1) (moved the .js file to resources dir)

Hoping the "cr-linux" bot is happier this time...

[WebKit Review Bot]

Comment on attachment 101229 [details]
Patch

Attachment 101229 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9116460

New failing tests:
webaudio/test-basic.html

[Tony Chang]

(In reply to comment #20)
> (From update of attachment 101229 [details])
> Attachment 101229 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/9116460
> 
> New failing tests:
> webaudio/test-basic.html

Looks like the test is timing out now.  On my machine, in release, the test takes 2.9 seconds.  I'm told that the script times out at 6s.  Maybe the bots are hitting the timeout (they're probably virtual machines, so slower than our desktops).

We probably want to find a way to speed this up since 2.9s is still pretty slow for a test.

[Chris Rogers]

Ok, digging a bit deeper it looks like my idea to base64 encode in JavaScript was not such a great one.  On my machine test-basic.html takes 1762ms overall.  All but 60ms of this is spent base64 encoding!  I'm going to see what I can do in DRT to create a new JS API for passing binary data via TypedArrays to avoid this nonsense.  Then the WAV binary file data can be directly written and the tests can run *way* faster.

[Chris Rogers]

Created attachment 101938 [details]
Patch

[Chris Rogers]

Latest patch now passes WAV file data directly as binary data, which will hopefully improve the speed of these tests.

[Tony Chang]

Comment on attachment 101938 [details]
Patch

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

Much faster.  It's .35s on my machine in release and .6s in debug.

> LayoutTests/webaudio/test-basic.html:1
> +<!--

<!DOCTYPE html>

[Chris Rogers]

Committed r91780: <http://trac.webkit.org/changeset/91780>