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

[bugzilla-daemon at webkit.org]

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


Jeremy Orlow <jorlow at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fishd at chromium.org




--- Comment #12 from Jeremy Orlow <jorlow at chromium.org>  2010-07-13 13:08:01 PST ---
(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.

It's mostly just boilerplate/build files.  And the only way to land this in chunks is either to land dead code (which we're normally against as a project) or to break chromium in the process (like splitting the WebKit parts into their own patch).

I'm happy to review this as is, so I don't think he should split it up.

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

Really?  Most of the file looks like this.

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

The only way I can think of is if we added some option for the code generator to add such asserts.  This actually does seem like a good idea.

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

So that half can run in the Chrome browser process.  This is how all of IndexedDB, LocalStorage, and apparently now WebKit2 works.

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

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

This is done many places in the WebKit API last time I checked.  After thinking about it, it does seem slightly better but I'd be interested to hear what Darin thinks.

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