Summary: | IndexedDB: Use ScriptValue instead of SerializedScriptValue for get/openCursor | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alec Flett <alecflett> | ||||||||
Component: | WebCore Misc. | Assignee: | Alec Flett <alecflett> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, donggwan.kim, haraken, japhet, michael, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 94023, 95385 | ||||||||||
Bug Blocks: | 88287 | ||||||||||
Attachments: |
|
Description
Alec Flett
2012-08-29 17:36:55 PDT
Created attachment 164206 [details]
Patch
haraken@ - this is the second half of bug 94023 - mind taking a look at this? Comment on attachment 164206 [details]
Patch
LGTM at a first quick look. I'm now leaving, let me take a detailed look tomorrow.
Comment on attachment 164206 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164206&action=review LGTM with some nits. > Source/WebCore/Modules/indexeddb/IDBAny.h:63 > + static PassRefPtr<IDBAny> create(const T& idbObject) Are both create(const T& idbObject) and create(PassRefPtr<T> idbObject) needed? You might want to remove create(PassRefPtr<T> idbObject) in favor of create(const T& idbObject). > Source/WebCore/Modules/indexeddb/IDBCursor.cpp:276 > + ASSERT_UNUSED(injected, injected); Given that the current code has ASSERT(valueAfterInjection), shouldn't this be ASSERT(injected)? > Source/WebCore/Modules/indexeddb/IDBCursor.cpp:277 > // FIXME: There is no way to report errors here. Move this into onSuccessWithContinuation so that we can abort the transaction there. See: https://bugs.webkit.org/show_bug.cgi?id=92278 You can move this comment to just before the ASSERT(). > Source/WebCore/Modules/indexeddb/IDBRequest.cpp:376 > + ASSERT(injected); > + if (!injected) { This code looks a bit strange: - If 'injected' can be 0, 'ASSERT(injected)' should be removed. - If 'injected' shouldn't be 0, 'if (!injected)' should be removed. Created attachment 164423 [details]
Patch for landing
Comment on attachment 164423 [details] Patch for landing Rejecting attachment 164423 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: oid WebCore::IDBRequest::onSuccess(WTF::PassRefPtr<WebCore::SerializedScriptValue>, WTF::PassRefPtr<WebCore::IDBKey>, const WebCore::IDBKeyPath&)': Source/WebCore/Modules/indexeddb/IDBRequest.cpp:374: error: unused variable 'injected' CXX(target) out/Release/obj.target/webcore_remaining/Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.o make: *** [out/Release/obj.target/webcore_remaining/Source/WebCore/Modules/indexeddb/IDBRequest.o] Error 1 make: *** Waiting for unfinished jobs.... Full output: http://queues.webkit.org/results/13860975 Created attachment 164425 [details]
Patch for landing
Comment on attachment 164425 [details] Patch for landing Clearing flags on attachment: 164425 Committed r128789: <http://trac.webkit.org/changeset/128789> All reviewed patches have been landed. Closing bug. |