[Webkit-unassigned] [Bug 41888] Initial bindings and plumbing for IDBCursor.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jul 30 04:37:05 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=41888
Jeremy Orlow <jorlow at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #62935|review? |review-
Flag| |
--- Comment #23 from Jeremy Orlow <jorlow at chromium.org> 2010-07-30 04:37:04 PST ---
(From update of attachment 62935)
Please redo the names in this patch to match the latest flavor of the week.
LayoutTests/storage/indexeddb/script-tests/open-cursor.js:29
+ function openCursor(objectStore)
Maybe put above dataAddedSuccess so everything's in revert chronological order?
LayoutTests/storage/indexeddb/script-tests/open-cursor.js:16
+ function dataAddedSuccess()
Not needed...just have the success call open cursor directly or pull that logic into this.
There's no inheritance of IDLs in the latest spec. So please merge the 2 files.
WebCore/GNUmakefile.am:2854
+ WebCore/storage/IDBCursor.cpp \
spacing issue
WebCore/storage/IDBCursor.h:43
+ // Keep in sync with what's in the .idl file.
Remove comment and use your automated mechanism.
WebCore/storage/IDBCursor.h:41
+ class IDBCursor : public RefCounted<IDBCursor> {
The interfaces we've been doing threadsafe ref counted since they could be used on a worker or the main thread once we add it to workers. (If I were doing it again, I would have waited until we actually needed it to be threadsafe to do this, but it's better to be consistent at this point I think.)
WebCore/storage/IDBCursorImpl.cpp:29
+ #include "IDBAny.h"
put inside guard...here an elsewhere...from now on
WebCore/storage/IDBCursorImpl.cpp:68
+ long long IDBCursorImpl::count() const
Could just be inline...but doesn't matter much either way.
WebCore/storage/IDBCursorImpl.cpp:74
+ {
ASSERT_NOT_REACHED()
+ FIXME
WebCore/storage/IDBCursorImpl.h:57
+ unsigned short m_direction;
This should be an enum, right?
WebCore/storage/IDBCursorImpl.h:60
+ int m_count;
long long?
WebCore/storage/IDBCursorImpl.h:43
+ IDBCursorImpl(PassRefPtr<IDBObjectStoreImpl>, PassRefPtr<IDBKeyRange>, unsigned short direction, PassRefPtr<IDBKey>, PassRefPtr<SerializedScriptValue>, int count);
make private, add ::create method.
WebCore/storage/IDBCursorRequest.cpp:62
+ return m_cursor->count();
Could be inline...but I don't care much.
WebCore/storage/IDBObjectStoreImpl.cpp:139
+ int count = value ? 1 : 0;
we should move count all together since we're not doing sync cursors.
WebCore/storage/IDBObjectStoreImpl.cpp:141
+ value = SerializedScriptValue::create();
Aren't we supposed to return a null cursor in this case?
WebCore/storage/IDBObjectStoreImpl.cpp:138
+ RefPtr<SerializedScriptValue> value = m_tree->get(key.get());
This isn't right...what if the value doesn't exist but we can find something higher/lower (depending on cursor type) than that? m_tree will need some new methods I guess.
WebCore/storage/IDBObjectStoreImpl.cpp:137
+ RefPtr<IDBKey> key = range->left();
This doesn't handle direction.
WebCore/storage/IDBObjectStoreRequest.cpp:66
+ return request.release();
Maybe do in another patch? (And look at the other files too?)
WebKit/chromium/public/WebIDBCursor.h:44
+ WEBKIT_ASSERT_NOT_REACHED();
Should be easy to implement in this CL.
WebKit/chromium/public/WebIDBKeyRange.h:43
+ WEBKIT_API WebIDBKeyRange(const WebIDBKeyRange& keyRange) { m_private = keyRange.m_private; }
This won't work. You need to make an assign method that does this and just call assign.
Similarly ~ should be inline and call reset().
All methods that aren't inline and aren't virtual should have a WEBKIT_API prefix.
WebKit/chromium/src/IDBCursorProxy.h:56
+ PassOwnPtr<WebKit::WebIDBCursor> m_idbCursor;
RefPtr
--
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