Bug 95409 – IndexedDB: Use ScriptValue instead of SerializedScriptValue for get/openCursor

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Alec Flett]

This is the second half of bug 94023, and also involves a lot of code removal that will be easier after bug 95385 lands.

[Alec Flett]

Created attachment 164206 [details]
Patch

[Alec Flett]

haraken@ - this is the second half of bug 94023 - mind taking a look at this?

[Kentaro Hara]

Comment on attachment 164206 [details]
Patch

LGTM at a first quick look. I'm now leaving, let me take a detailed look tomorrow.

[Kentaro Hara]

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.

[Alec Flett]

Created attachment 164423 [details]
Patch for landing

[WebKit Review Bot]

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

[Alec Flett]

Created attachment 164425 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 164425 [details]
Patch for landing

Clearing flags on attachment: 164425

Committed r128789: <http://trac.webkit.org/changeset/128789>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.