Bug 76952 - IndexedDB: IDBCursor.update() should raise exception if key changed
Summary: IndexedDB: IDBCursor.update() should raise exception if key changed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joshua Bell
URL:
Keywords:
Depends on:
Blocks: 77060 77374
  Show dependency treegraph
 
Reported: 2012-01-24 15:33 PST by Joshua Bell
Modified: 2012-01-31 13:45 PST (History)
3 users (show)

See Also:


Attachments
Patch (15.67 KB, patch)
2012-01-30 15:16 PST, Joshua Bell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Bell 2012-01-24 15:33:32 PST
IDBObjectStoreBackendImpl.cpp:

During the async task execution, this is run:

       if (putMode == CursorUpdate && !keyPathKey->isEqual(key)) {
            callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "The key fetched from the keyPath does not match the key of the cursor."));
            return 0;
        }

Per spec, this should synchronously raise an exception during the put() call instead.
Comment 1 Joshua Bell 2012-01-24 15:47:35 PST
... and while there, move all of the key selection (and value updating) logic into put(), rather than the putInternal async task, to match recent discussion on public-webapps. 

The async task should be handed the final value and final key to insert with.
Comment 2 Joshua Bell 2012-01-25 11:44:02 PST
(In reply to comment #1)
> ... and while there, move all of the key selection (and value updating) logic into put(), rather than the putInternal async task, to match recent discussion on public-webapps. 
> 
> The async task should be handed the final value and final key to insert with.

Slight wrinkle - for generated keys (i.e. autoIncrement) the generation step should occur within the async task within the transaction. This may require that in put() we check to see if a value can be inserted (by inserting a dummy value?) and then in putInternal() we run the key generation step and actually insert the key.
Comment 3 Joshua Bell 2012-01-30 15:16:58 PST
Created attachment 124610 [details]
Patch
Comment 4 Joshua Bell 2012-01-30 15:25:31 PST
Filed https://bugs.webkit.org/show_bug.cgi?id=77374 for the "the generation step should occur within the async task within the transaction" note.
Comment 5 Tony Chang 2012-01-30 15:55:22 PST
The change looks sane.  If dgrogan or someone else can do an unofficial review, I can r+.
Comment 6 David Grogan 2012-01-30 18:50:30 PST
Comment on attachment 124610 [details]
Patch

LGTM
Comment 7 WebKit Review Bot 2012-01-31 13:45:43 PST
Comment on attachment 124610 [details]
Patch

Clearing flags on attachment: 124610

Committed r106387: <http://trac.webkit.org/changeset/106387>
Comment 8 WebKit Review Bot 2012-01-31 13:45:48 PST
All reviewed patches have been landed.  Closing bug.