Bug 128035 - IDB: Index cursors use wrong deserialization for the retrieved value
Summary: IDB: Index cursors use wrong deserialization for the retrieved value
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 124521
  Show dependency treegraph
 
Reported: 2014-01-31 20:21 PST by Brady Eidson
Modified: 2014-01-31 21:22 PST (History)
5 users (show)

See Also:


Attachments
Patch v1 (81.30 KB, patch)
2014-01-31 20:30 PST, Brady Eidson
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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!