Bug 128035

Summary: IDB: Index cursors use wrong deserialization for the retrieved value
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit2Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, commit-queue, jsbell, mitz, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 124521    
Attachments:
Description Flags
Patch v1 mitz: review+

Description Brady Eidson 2014-01-31 20:21:09 PST
IDB: Index cursors use wrong deserialization for the retrieved value

The values stored in the index are encoded IDBKeys, and not encoded ScriptValues.

Fixing the (de)serialization confusion throughout the IDB mechanism will be a long, iterative process.  For now, we'll change the cursor operation callbacks to include a "value key" instead of the value buffer, and the IDB front end will choose the appropriate one to use as the result value.
Comment 1 Brady Eidson 2014-01-31 20:30:24 PST
Created attachment 222871 [details]
Patch v1
Comment 2 WebKit Commit Bot 2014-01-31 20:32:43 PST
Attachment 222871 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/indexeddb/IDBTransactionBackendOperations.cpp:157:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/indexeddb/IDBKeyData.cpp:259:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/indexeddb/IDBCursorBackendOperations.cpp:43:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/indexeddb/IDBCursorBackendOperations.cpp:66:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 4 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Brady Eidson 2014-01-31 20:39:31 PST
(In reply to comment #2)
> Attachment 222871 [details] did not pass style-queue:
> ERROR: Source/WebCore/Modules/indexeddb/IDBKeyData.cpp:259:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]

Fixed that locally.

The other 3 are the long standing (and reported!) bug of check-webkit-style not liking our C++ lambda style.
Comment 4 mitz 2014-01-31 20:53:20 PST
Comment on attachment 222871 [details]
Patch v1

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

> Source/WebCore/Modules/indexeddb/IDBCursorBackend.cpp:106
> +    m_currentValueBuffer = 0;
> +    m_currentValueKey = 0;

nullptr, not 0

> Source/WebCore/Modules/indexeddb/IDBCursorBackend.h:63
> +    SharedBuffer* valueBuffer() const { return (m_cursorType == IndexedDB::CursorType::KeyOnly) ? 0 : m_currentValueBuffer.get(); }
> +    IDBKey* valueKey() const { return (m_cursorType == IndexedDB::CursorType::KeyOnly) ? 0 : m_currentValueKey.get(); }

Ditto

> Source/WebCore/Modules/indexeddb/IDBKeyData.cpp:267
> +        String result = "<array> - { ";
> +        for (size_t i = 0; i < arrayValue.size(); ++i) {
> +            result.append(arrayValue[i].loggingString());
> +            if (i < arrayValue.size() - 1)
> +                result.append(", ");
> +        }
> +        result.append(" }");
> +        return result;

There’s probably a more efficient way to do this using StringBuilder, but this is good enough for debug-only logging.
Comment 5 Brady Eidson 2014-01-31 20:57:14 PST
http://trac.webkit.org/changeset/163234
Comment 6 Ryosuke Niwa 2014-01-31 21:12:34 PST
Release build fix landed in http://trac.webkit.org/changeset/163235.
Comment 7 Brady Eidson 2014-01-31 21:22:02 PST
(In reply to comment #6)
> Release build fix landed in http://trac.webkit.org/changeset/163235.

DOH!  Sorry about that, and thanks for the fix!