Bug 92414 - [Chromium] IndexedDB: API stubs to simplify WebIDBCursor API
Summary: [Chromium] IndexedDB: API stubs to simplify WebIDBCursor API
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: 92278
  Show dependency treegraph
 
Reported: 2012-07-26 12:49 PDT by Joshua Bell
Modified: 2012-08-01 16:16 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.97 KB, patch)
2012-07-26 12:52 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (3.00 KB, patch)
2012-07-26 15:04 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch for landing (3.03 KB, patch)
2012-08-01 13:20 PDT, 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-07-26 12:49:26 PDT
[Chromium] IndexedDB: API stubs to simplify WebIDBCursor API
Comment 1 Joshua Bell 2012-07-26 12:52:29 PDT
Created attachment 154724 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-26 12:55:25 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Joshua Bell 2012-07-26 12:59:35 PDT
Note that I will NOT be landing this until alecflett@'s in-progress index changes have landed, as they touch similar code.

Full landing sequence is:

(1) WebKit prep: Add new API stubs http://webkit.org/b/92414 (this patch)
(2) Chromium changes: dispatch to old and new API https://chromiumcodereview.appspot.com/10830028
(3) WebKit changes: use the new API http://webkit.org/b/92278
(4) Chromium cleanup: delete old API usage
(5) WebKit changes: delete old API

Wheee.

Questions for reviewers:

* Should I name the two IDBKey arguments? (key and primaryKey)
* Instead of two new WebIDBCallback methods, onSuccess(cursor, key, primaryKey, value) - callback from openCursor(), and onSuccess(key, primaryKey, value) - callback from continue(), I could use the same method for both; passing along the cursor as a callback from continue() would be redundant but not harmful.

dgrogan@, alecflett@ - please take a look
Comment 4 Joshua Bell 2012-07-26 15:04:04 PDT
Created attachment 154759 [details]
Patch
Comment 5 David Grogan 2012-07-26 15:12:13 PDT
LGTM

(In reply to comment #3)
> * Instead of two new WebIDBCallback methods, onSuccess(cursor, key, primaryKey, value) - callback from openCursor(), and onSuccess(key, primaryKey, value) - callback from continue(), I could use the same method for both; passing along the cursor as a callback from continue() would be redundant but not harmful.

I'd keep them separate but *shrug*.
Comment 6 Alec Flett 2012-07-26 16:51:40 PDT
+1 on separate - its hard enough to keep the various onSuccess's apart as it is :)
Comment 7 Joshua Bell 2012-07-30 16:27:14 PDT
abarth@, fishd@ or others - r?

Note the "full landing sequence" comment for context.
Comment 8 Adam Barth 2012-07-30 16:34:10 PDT
> * Should I name the two IDBKey arguments? (key and primaryKey)

Yeah, generally if you have two arguments of the same type it's a good idea to give them names so folks can tell them apart.
Comment 9 Alec Flett 2012-07-31 10:53:03 PDT
FWIW, the bulk of my changes that should affect this have landed - I'm happy to have this bug sequence start to land and I'll sort through any leftover conflicts in bug 91125
Comment 10 WebKit Review Bot 2012-08-01 12:43:36 PDT
Comment on attachment 154759 [details]
Patch

Rejecting attachment 154759 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WTF/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/13410492
Comment 11 Joshua Bell 2012-08-01 13:20:22 PDT
Created attachment 155869 [details]
Patch for landing
Comment 12 WebKit Review Bot 2012-08-01 16:16:06 PDT
Comment on attachment 155869 [details]
Patch for landing

Clearing flags on attachment: 155869

Committed r124382: <http://trac.webkit.org/changeset/124382>
Comment 13 WebKit Review Bot 2012-08-01 16:16:11 PDT
All reviewed patches have been landed.  Closing bug.