[webkit-reviews] review denied: [Bug 40606] Fix compilation errors in BlobBuilder with FILE_WRITER enabled : [Attachment 58743] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jun 14 23:21:27 PDT 2010
David Levin <levin at chromium.org> has denied Kinuko Yasuda
<kinuko at chromium.org>'s request for review:
Bug 40606: Fix compilation errors in BlobBuilder with FILE_WRITER enabled
https://bugs.webkit.org/show_bug.cgi?id=40606
Attachment 58743: Patch
https://bugs.webkit.org/attachment.cgi?id=58743&action=review
------- Additional Comments from David Levin <levin at chromium.org>
WebCore/ChangeLog:1
+ 2010-06-14 Kinuko Yasuda <kinuko at kinuko2-macpro.mtv.corp.google.com>
The email address needs to be fixed.
WebCore/ChangeLog:8
+ No new tests.
This should explain why no new tests were added. For example:
"No functionality change so no new tests."
WebCore/html/BlobBuilder.h:38
+ #include "PlatformString.h"
Actually why is PlatformString.h included? (Can't a forward declaration for
String suffice?)
WebCore/html/BlobBuilder.h:41
+ #include <wtf/text/CString.h>
I believe you're including this due to the destructor needing it. If you
explicitly define the destructor for BlobBuilder or even ~StringBlobItem and
put it in a cpp file, then this include shouldn't be needed.
But this header already "#include BlobItem.h" which "#include
<wtf/text/CString.h>", so it is unclear why this is needed to fix the build.
WebCore/html/BlobBuilder.h: 43
+ class ExceptionCode;
Typically Exception code is defined like this:
typedef int ExceptionCode;
and ExceptionCode.h isn't included. (Also if you add this line, it should come
after the class Blob; forward declaration.)
More information about the webkit-reviews
mailing list