Bug 110398 – IndexedDB: Implement SharedBuffer version of put() / onSuccess()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alec Flett]

IndexedDB: Implement SharedBuffer version of put() / onSuccess()

[Alec Flett]

Created attachment 189396 [details]
Patch

[Alec Flett]

jsbell/dgrogan: quick review?

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Joshua Bell]

Comment on attachment 189396 [details]
Patch

Overall lgtm...


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

> Source/WebCore/ChangeLog:15
> +        (WebCore::IDBBackingStore::getRecord):

Add notes to these methods or delete them.

> Source/WebCore/ChangeLog:25
> +        (IDBBackingStore):

Especially delete these - they're not helpful.

> Source/WebCore/ChangeLog:42
> +        (WebCore):

Ditto.

> Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1687
> +    value.append(t, result.end() - t);

Although you're not introducing it in this patch, can you rename |t| to |valuePosition| to match the other implementation of this function, for readability?

> Source/WebCore/Modules/indexeddb/IDBRequest.cpp:286
> +void IDBRequest::onSuccess(PassRefPtr<IDBCursorBackendInterface> backend, PassRefPtr<IDBKey> key, PassRefPtr<IDBKey> primaryKey, PassRefPtr<SharedBuffer> rawValue)

Rename rawValue to valueBuffer or just buffer?

> Source/WebCore/Modules/indexeddb/IDBRequest.cpp:384
> +    return onSuccessInternal(deserializeIDBValue(requestState(), value));

Ugh... need to add ScriptValue::createString/Number/Undefined/Null, so we can get SSV out of the loop and simplify IDBAny. But that's wkbug.com/100292

> Source/WebCore/Modules/indexeddb/IDBRequest.cpp:397
> +void IDBRequest::onSuccess(PassRefPtr<IDBKey> key, PassRefPtr<IDBKey> primaryKey, PassRefPtr<SharedBuffer> rawValue)

Rename rawValue to valueBuffer or just buffer?

> Source/WebKit/chromium/tests/IDBAbortOnCorruptTest.cpp:70
> +    virtual void onSuccessWithPrefetch(const Vector<RefPtr<IDBKey> >&, const Vector<RefPtr<IDBKey> >&, const Vector<RefPtr<SharedBuffer> >&) { }

OVERRIDE here?

[Alec Flett]

Created attachment 189565 [details]
Patch

[Alec Flett]

tony@ - r? for WebCore/storage/indexeddb/ ?
fishd@ or abarth@ - r? for WebKit/chromium ?

[Alec Flett]

(also note that this will likely fail the cr-* EWS bots until https://codereview.chromium.org/12326023/ lands on the chromium side)

[Tony Chang]

Comment on attachment 189565 [details]
Patch

GTK build failures look real.

[Alec Flett]

Created attachment 189605 [details]
Patch

[Alec Flett]

Created attachment 189624 [details]
Patch

[WebKit Review Bot]

Comment on attachment 189624 [details]
Patch

Clearing flags on attachment: 189624

Committed r143694: <http://trac.webkit.org/changeset/143694>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.