[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
----------------------------------------------------------------------------
  Attachment #60928|review?                     |review-
               Flag|                            |




--- Comment #5 from Jeremy Orlow <jorlow at chromium.org>  2010-07-12 06:57:04 PST ---
(From update of attachment 60928)
WebKit/chromium/src/WebIDBObjectStoreImpl.cpp:88
 +  
remove extra newline

WebKit/chromium/src/WebIDBKeyRange.cpp:39
 +  
get rid of this extra newline

WebKit/chromium/src/WebIDBKeyRange.cpp:42
 +  #if WEBKIT_IMPLEMENTATION
This is always defined in chromium/src

WebKit/chromium/src/WebIDBKeyRange.cpp:64
 +  void WebIDBKeyRange::reset()
Do you actually need this?

Where's the WebIDBKeyRange.h?  Why isn't WebIDBKeyRange designed like WebIDBDatabaseError (and such)...i.e. a thin wrapper around a WebCore type?  We only do this stuff with Impls and proxies when chromium needs to implement something as well.

WebKit/chromium/src/WebIDBCursorImpl.cpp:32
 +  #if ENABLE(INDEXED_DATABASE)
We don't use guards in WebKit code usually...we just assume it's on.

WebKit/chromium/src/WebIDBCallbacksImpl.cpp:67
 +  void WebIDBCallbacksImpl::onSuccess(WebKit::WebIDBCursor* webKitInstance)
This should probably follow the model of database errors...


WebCore/storage/IndexedDatabaseRequest.cpp:66
 +      RefPtr<IDBAny> db = IDBAny::create();
Should we add ::create() functions for each type so we can do this inline when creating the requests rather than needing 3 lines to do it?

WebCore/storage/IDBObjectStoreRequest.h:70
 +      PassRefPtr<IDBRequest> openCursor(ScriptExecutionContext*, PassRefPtr<IDBKeyRange> range = PassRefPtr<IDBKeyRange>(), unsigned short direction = IDBCursor::NEXT);
= 0 is the same thing

WebCore/storage/IDBObjectStoreRequest.cpp:106
 +      RefPtr<IDBAny> db = IDBAny::create();
Hm...these changes shoudl probably all be done in their own cl.

WebCore/storage/IDBObjectStoreImpl.h:53
 +      void openCursor(PassRefPtr<IDBCallbacks>, PassRefPtr<IDBKeyRange> range = PassRefPtr<IDBKeyRange>(), unsigned short direction = IDBCursor::NEXT);
don't need default params

WebCore/storage/IDBObjectStore.h:55
 +      virtual void openCursor(PassRefPtr<IDBCallbacks>, PassRefPtr<IDBKeyRange> range = PassRefPtr<IDBKeyRange>(), unsigned short direction = IDBCursor::NEXT) = 0;
don't need default params

WebCore/storage/IDBKeyRange.h:61
 +  protected:
Why protected?

WebCore/storage/IDBCursorRequest.idl:37
 +          void continue(in [Optional] IDBAny key);
I believe this should be an IDBKey

WebCore/storage/IDBCursorRequest.idl:36
 +          void update(in IDBAny value);
This should probably be a serializedScriptValue

IDBAny is only a return type...and perhaps not very well named?

WebCore/storage/IDBCursorRequest.cpp:79
 +      // FIXME: Implement.
ASSERT_NOT_IMPLEMENTED();

and above

WebCore/storage/IDBCursor.h:53
 +      virtual unsigned short direction() const { return m_direction; }
virtual?  If so, the ~ should be virtual as well probably.

WebCore/storage/IDBCursor.h:37
 +  // FIXME: Make this class pure virtual.
What's the hold up?



These comments are not exhaustive...shoudl get you started tho.

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