WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
127012
Use KeyedCoding as a persistent storage mechanism for blobs
https://bugs.webkit.org/show_bug.cgi?id=127012
Summary
Use KeyedCoding as a persistent storage mechanism for blobs
Brady Eidson
Reported
2014-01-14 14:32:32 PST
Use KeyedCoding as a persistent storage mechanism for blobs This is needed to support
https://bugs.webkit.org/show_bug.cgi?id=127011
and other future IDB work. Anders says he has KeyedEncoding done and KeyedDecoding partially/almost done with a patch at home. I hope to take over finishing that asap.
Attachments
Patch v1
(25.23 KB, patch)
2014-01-15 18:42 PST
,
Brady Eidson
andersca
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Brady Eidson
Comment 1
2014-01-14 14:33:42 PST
In Radar as <
rdar://problem/15818481
>
Brady Eidson
Comment 2
2014-01-15 18:42:20 PST
Created
attachment 221323
[details]
Patch v1
WebKit Commit Bot
Comment 3
2014-01-15 18:44:16 PST
Attachment 221323
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp', u'Source/WebCore/Modules/indexeddb/IDBKeyPath.h', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/platform/KeyedCoding.h', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/DatabaseProcess/IndexedDB/IDBSerialization.cpp', u'Source/WebKit2/DatabaseProcess/IndexedDB/IDBSerialization.h', u'Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp', u'Source/WebKit2/Shared/cf/KeyedDecoder.cpp', u'Source/WebKit2/Shared/cf/KeyedDecoder.h', u'Source/WebKit2/Shared/cf/KeyedEncoder.cpp', u'Source/WebKit2/Shared/cf/KeyedEncoder.h', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:286: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:301: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 2 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Anders Carlsson
Comment 4
2014-01-16 13:11:04 PST
Comment on
attachment 221323
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=221323&action=review
> Source/WebCore/platform/KeyedCoding.h:79 > + while (beginArrayElement()) {
If beginArrayElement() returns false result will be uninitialized. I'd initialize result to true and change the loop to while (true) { if (!beginArrayElement()) { result = false; break; }
> Source/WebCore/platform/KeyedCoding.h:81 > + objects.append(T()); > + result = function(*this, objects.last());
Instead of appending to the array do something like: T element; if (!function(*this, t)) { result = false; break; } object.append(std::move(element));
> Source/WebCore/platform/KeyedCoding.h:115 > + virtual PassRefPtr<SharedBuffer> createBuffer() = 0;
Let's call this finishEncoding() instead to indicate that it can only be called when you're done.
> Source/WebKit2/ChangeLog:7 > +
Extra newline.
Brady Eidson
Comment 5
2014-01-16 13:33:44 PST
(In reply to
comment #4
)
> (From update of
attachment 221323
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=221323&action=review
> > > Source/WebCore/platform/KeyedCoding.h:79 > > + while (beginArrayElement()) { > > If beginArrayElement() returns false result will be uninitialized. I'd initialize result to true and change the loop to > > while (true) { if (!beginArrayElement()) { result = false; break; } >
`result` is only about the success of the individual object function. It's okay for beginArrayElement to return false, as that just means there's no more elements in the array. So it seems the only change needed here is to initialize result to true, indicating that no individual object failed to decode.
> > Source/WebCore/platform/KeyedCoding.h:81 > > + objects.append(T()); > > + result = function(*this, objects.last()); > > Instead of appending to the array do something like: > T element; > if (!function(*this, t)) { > result = false; > break; > } > > object.append(std::move(element)); >
Looks good.
> > Source/WebCore/platform/KeyedCoding.h:115 > > + virtual PassRefPtr<SharedBuffer> createBuffer() = 0; > > Let's call this finishEncoding() instead to indicate that it can only be called when you're done.
Sounds good.
Brady Eidson
Comment 6
2014-01-16 13:41:51 PST
http://trac.webkit.org/changeset/162148
Brady Eidson
Comment 7
2014-01-16 14:02:09 PST
http://trac.webkit.org/changeset/162151
as a build fix.
Darin Adler
Comment 8
2014-01-17 08:58:21 PST
Comment on
attachment 221323
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=221323&action=review
> Source/WebKit2/Shared/cf/KeyedDecoder.h:51 > + virtual bool decodeInt64(const String& key, int64_t&); > + virtual bool decodeUInt32(const String& key, uint32_t&); > + virtual bool decodeString(const String& key, String&); > + > + virtual bool beginObject(const String& key); > + virtual void endObject(); > + > + virtual bool beginArray(const String& key); > + virtual bool beginArrayElement(); > + virtual void endArrayElement(); > + virtual void endArray();
Should add "override" to all of these.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug