[Webkit-unassigned] [Bug 41888] Initial bindings and plumbing for IDBCursor.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 14 03:32:01 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=41888





--- Comment #14 from Marcus Bulach <bulach at chromium.org>  2010-07-14 03:32:01 PST ---
thanks, Adam! replies inline:

(In reply to comment #11)
> (From update of attachment 61392 [details])
> 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.
as mentioned above, this is not yet finished. please ignore any project files for the time being.

> 
> WebCore/bindings/scripts/CodeGenerator.pm:480
>  +      return $name . "_" if exists $cppReservedKeyWords{$name};
> Yuck
Yuck indeed! :)
This is being implemented as a separate patch, I added you to
https://bugs.webkit.org/show_bug.cgi?id=41613
Happy to hear any suggestion.

> 
> WebCore/storage/IDBCursor.h:39
>  +      // Keep in sync with what's in the .idl file.
> Can we assert this at compile time?
Good idea! We could probably improve the code generator to assert this.
I'll add a separate patch for it.

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

> 
> WebCore/storage/IDBObjectStoreRequest.cpp:115
>  +      return request;
> request.release()
ugh. done, in several places in the file.

> 
> Please read http://webkit.org/coding/RefPtr.html if you haven't already.
thanks for the refresher.

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

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list