[webkit-reviews] review denied: [Bug 57969] Add web audio support to DumpRenderTree (mac port) : [Attachment 88523] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Chris Rogers <crogers at google.com>'s
request for review:
Bug 57969: Add web audio support to DumpRenderTree (mac port)
https://bugs.webkit.org/show_bug.cgi?id=57969

Attachment 88523: Patch
https://bugs.webkit.org/attachment.cgi?id=88523&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88523&action=review

Sorry, I misunderstood that this required changes to NRWT/ORWT.  Can you
re-upload without the test?

> Tools/DumpRenderTree/LayoutTestController.h:271
> +    void setEncodedAudioData(std::string encodedAudioData) {
m_encodedAudioData = encodedAudioData; }

const std::string&

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:739
> +    const char* encodedAudioData =
gLayoutTestController->encodedAudioData().c_str();

I think in ObjC we put the * next to the variable name.  This file seems like a
mix of both styles.

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:741
> +    NSData *data = [NSData dataWithBytes:encodedAudioData
length:strlen(encodedAudioData)];

If encodedAudioData has null bytes in it, won't strlen give the wrong value? 
Should we use gLayoutTestController->encodedAudioData().length() instead?