[webkit-reviews] review denied: [Bug 57969] Add web audio support to DumpRenderTree (mac port) : [Attachment 88523] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 6 15:14:58 PDT 2011
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?
More information about the webkit-reviews
mailing list