[webkit-reviews] review denied: [Bug 41888] Initial bindings and plumbing for IDBCursor. : [Attachment 61392] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Marcus Bulach <bulach at chromium.org>'s
request for review:
Bug 41888: Initial bindings and plumbing for IDBCursor.
https://bugs.webkit.org/show_bug.cgi?id=41888

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
This patch is too large.  Please at least do the WebKit API changes in a
separate patch.  Also, consider breaking this patch into even smaller pieces. 
Drive-by comments below.

WebCore/GNUmakefile.am:2580
 +    WebCore/storage/IDBCursor.idl \
Bad indent.

WebCore/WebCore.vcproj/WebCore.vcproj:50055
 +	  <File
Bad indent.

WebCore/bindings/scripts/CodeGenerator.pm:480
 +	return $name . "_" if exists $cppReservedKeyWords{$name};
Yuck

WebCore/storage/IDBCursor.h:39
 +	// Keep in sync with what's in the .idl file.
Can we assert this at compile time?

WebCore/storage/IDBCursorImpl.cpp:35
 +  class IDBCursorImpl : public IDBCursor {
What's the point of hiding the Impl class?

WebCore/storage/IDBObjectStoreRequest.cpp:115
 +	return request;
request.release()

Please read http://webkit.org/coding/RefPtr.html if you haven't already.

WebKit/chromium/public/WebIDBKeyRange.h:32
 +  namespace WebCore { class IDBKeyRange; }
Please put this on multiple lines so we can add more as needed.