Bug 90114 - IndexedDB: Keep direction on IDBCursor to avoid calls to back end
Summary: IndexedDB: Keep direction on IDBCursor to avoid calls to back end
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joshua Bell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-27 17:03 PDT by Joshua Bell
Modified: 2012-06-29 20:09 PDT (History)
4 users (show)

See Also:

Attachments
Patch (20.33 KB, patch)
2012-06-27 17:06 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Patch (20.51 KB, patch)
2012-06-29 11:05 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Patch for landing (20.50 KB, patch)
2012-06-29 13:25 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Bell 2012-06-27 17:03:16 PDT 
IndexedDB: Keep direction on IDBCursor to avoid calls to back end
Comment 1 Joshua Bell 2012-06-27 17:06:57 PDT 
Created attachment 149825 [details]
Patch
Comment 2 Joshua Bell 2012-06-27 17:10:43 PDT 
alecflett@ - can you take a look?

I think this is the last of the simple ones. IDBTransactionBackendInterface::mode() will be difficult to remove as IDBObjectStoreBackendInterface is passed a IDBTransactionBackendInterface (for example). We'll have to solve the "XXXImpl objects deserve XXXImpl pointers" problem to clean that up.
Comment 3 Joshua Bell 2012-06-27 17:14:07 PDT 
alecflett@ - also, I'm not sure why IDBCursor::stringToDirection() was an unsigned int rather than an enum value. Changing it didn't seem problematic (other than having to pick a default in the error case), and was necessary for making IDBRequest::setCursorTypeAndDirection() accept the enum w/o a cast.

Speaking of, the name setCursorTypeAndDirection() is wordy. setCursorInfo() ? setCursorDetails() ?
Comment 4 Alec Flett 2012-06-29 10:24:23 PDT 
LGTM with extra const'ness on member variables
Comment 5 Joshua Bell 2012-06-29 11:05:25 PDT 
Created attachment 150218 [details]
Patch
Comment 6 Joshua Bell 2012-06-29 11:06:02 PDT 
(In reply to comment #4)
> LGTM with extra const'ness on member variables

Added to IDBCursor::m_direction, also (based on offline discussion) went with setCursorDetails()
Comment 7 Joshua Bell 2012-06-29 11:06:21 PDT 
tony@ - r?
Comment 8 Tony Chang 2012-06-29 11:28:10 PDT 
Comment on attachment 150218 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150218&action=review

> Source/WebCore/Modules/indexeddb/IDBCursor.h:62
> +    static IDBCursor::Direction stringToDirection(const String& modeString, ExceptionCode&);

Nit: I don't think you need the IDBCursor:: prefix here.
Comment 9 Joshua Bell 2012-06-29 13:24:35 PDT 
(In reply to comment #8)
> > +    static IDBCursor::Direction stringToDirection(const String& modeString, ExceptionCode&);
> 
> Nit: I don't think you need the IDBCursor:: prefix here.

Apparently it is needed, otherwise it's trying to resolve WebCore::Direction - the return type part of the function declaration is outside the IDBCursor scope, or some such.
Comment 10 Joshua Bell 2012-06-29 13:25:08 PDT 
Created attachment 150244 [details]
Patch for landing
Comment 11 WebKit Review Bot 2012-06-29 20:09:49 PDT 
Comment on attachment 150244 [details]
Patch for landing

Clearing flags on attachment: 150244

Committed r121612: <http://trac.webkit.org/changeset/121612>
Comment 12 WebKit Review Bot 2012-06-29 20:09:53 PDT 
All reviewed patches have been landed.  Closing bug.