Bug 90001 - IndexedDB: Hook up render-side key ASSERTing for get()
Summary: IndexedDB: Hook up render-side key ASSERTing for get()
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: Alec Flett
URL:
Keywords:
Depends on: 89377
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-26 11:46 PDT by Alec Flett
Modified: 2012-06-28 15:57 PDT (History)
7 users (show)

See Also:


Attachments
Patch (12.54 KB, patch)
2012-06-26 14:02 PDT, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (12.55 KB, patch)
2012-06-26 14:07 PDT, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (45.24 KB, patch)
2012-06-27 12:33 PDT, Alec Flett
no flags Details | Formatted Diff | Diff
Patch (45.24 KB, patch)
2012-06-27 12:37 PDT, Alec Flett
no flags Details | Formatted Diff | Diff
Patch for landing (13.66 KB, patch)
2012-06-27 14:55 PDT, Alec Flett
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alec Flett 2012-06-26 11:46:09 PDT
IndexedDB: Hook up render-side key ASSERTing for get()
Comment 1 Alec Flett 2012-06-26 14:02:58 PDT
Created attachment 149603 [details]
Patch
Comment 2 WebKit Review Bot 2012-06-26 14:05:18 PDT
Attachment 149603 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alec Flett 2012-06-26 14:06:32 PDT
jsbell@ - mind a once-over? this is dependent on (and the patch is based on top of) the patch in https://bugs.webkit.org/show_bug.cgi?id=89377 - this won't land cleanly until that bug is fixed.
Comment 4 Alec Flett 2012-06-26 14:07:34 PDT
Created attachment 149604 [details]
Patch
Comment 5 Joshua Bell 2012-06-26 15:20:34 PDT
Comment on attachment 149604 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBCursor.cpp:273
> +        // FIXME: Assert until we can actually inject the right value.

This FIXME just describes the current state; change it to describe the desired change.

> Source/WebCore/Modules/indexeddb/IDBCursor.cpp:277
> +            if (!keyPath.isNull()) {

This check is redundant.

> Source/WebCore/Modules/indexeddb/IDBCursor.cpp:278
> +                RefPtr<IDBKey> expectedKey =

Unnecessary line break.
Comment 6 Alec Flett 2012-06-27 12:33:21 PDT
Created attachment 149784 [details]
Patch
Comment 7 Alec Flett 2012-06-27 12:37:46 PDT
Created attachment 149786 [details]
Patch
Comment 8 Alec Flett 2012-06-27 12:39:57 PDT
Hey tony@ - another one here, continuing some refactoring work with some chromium plumbing. r? cq?
Comment 9 Tony Chang 2012-06-27 13:53:18 PDT
Comment on attachment 149786 [details]
Patch

Might be nice to have a clean ews run before landing.

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

> Source/WebCore/ChangeLog:28
> +        (WebCore):
> +
> +2012-06-26  Joshua Bell  <jsbell@chromium.org>
> +

Looks like a merge conflict?

> Source/WebCore/inspector/InspectorIndexedDBAgent.cpp:115
>      virtual void onSuccess(PassRefPtr<IDBTransactionBackendInterface>) { }
>      virtual void onSuccess(PassRefPtr<SerializedScriptValue>) { }
> +    virtual void onSuccess(PassRefPtr<SerializedScriptValue>, PassRefPtr<IDBKey>, const IDBKeyPath&) { }

Nit: Should these all have OVERRIDE?
Comment 10 Alec Flett 2012-06-27 14:43:32 PDT
hmm... somehow my git repository got stuck on a day-old chrome version, which means my webkit diff was broken. new patch coming up (almost identical, but with nits addressed, changelog fixed)
Comment 11 Alec Flett 2012-06-27 14:55:34 PDT
Created attachment 149801 [details]
Patch for landing
Comment 12 Alec Flett 2012-06-28 15:14:27 PDT
jsbell@ - cq? (you reviewed this earlier...)
Comment 13 WebKit Review Bot 2012-06-28 15:57:23 PDT
Comment on attachment 149801 [details]
Patch for landing

Clearing flags on attachment: 149801

Committed r121477: <http://trac.webkit.org/changeset/121477>
Comment 14 WebKit Review Bot 2012-06-28 15:57:30 PDT
All reviewed patches have been landed.  Closing bug.