Bug 156367 – Modern IDB (Blob support): Support retrieving Blobs from IDB

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

Modern IDB (Blob support): Support retrieving Blobs from IDB

[Brady Eidson]

Created attachment 276353 [details]
Patch v1

[Alex Christensen]

Comment on attachment 276353 [details]
Patch v1

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

> Source/WebCore/Modules/indexeddb/server/MemoryIndexCursor.cpp:78
> +        Vector<String> dummyBlobURLs, dummyBlobFiles;

Can't you just use { } instead of these?

> Source/WebCore/Modules/indexeddb/server/MemoryObjectStoreCursor.cpp:192
> +    Vector<String> dummyBlobURLs, dummyBlobFiles;

Ditto

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1053
> +        ThreadSafeDataBuffer valueBuffer = value ? value->data() : ThreadSafeDataBuffer();

More { }

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1810
> +            getResult = { cursor->currentValue() ? *cursor->currentValue() : IDBValue(), cursor->currentPrimaryKey() };

{}

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp:106
> +    result = { m_currentKey, m_currentPrimaryKey, m_currentValue ? *m_currentValue : IDBValue() };

{}

> Source/WebCore/bindings/js/IDBBindingUtilities.cpp:457
> +        return Deprecated::ScriptValue();

{}

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:1788
>              return 0;

These should be false

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:2433
> +        for (; i < m_blobURLs.size(); ++i) {

Could this be a map, so we don't need to iterate?

> Source/WebCore/fileapi/ThreadableBlobRegistry.cpp:108
> +        queue = new MessageQueue<CrossThreadTask>;

Would NeverDestroyed work here?

[Brady Eidson]

(In reply to comment #2)
> Comment on attachment 276353 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276353&action=review
> 
> > Source/WebCore/Modules/indexeddb/server/MemoryIndexCursor.cpp:78
> > +        Vector<String> dummyBlobURLs, dummyBlobFiles;
> 
> Can't you just use { } instead of these?
> 
> > Source/WebCore/Modules/indexeddb/server/MemoryObjectStoreCursor.cpp:192
> > +    Vector<String> dummyBlobURLs, dummyBlobFiles;
> 
> Ditto

Yup.

> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1053
> > +        ThreadSafeDataBuffer valueBuffer = value ? value->data() : ThreadSafeDataBuffer();
> 
> More { }
> 
> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:1810
> > +            getResult = { cursor->currentValue() ? *cursor->currentValue() : IDBValue(), cursor->currentPrimaryKey() };
> 
> {}
> 
> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBCursor.cpp:106
> > +    result = { m_currentKey, m_currentPrimaryKey, m_currentValue ? *m_currentValue : IDBValue() };
> 
> {}

Nope. Can't use initializer lists in the ternary operator.

"Initializer list cannot be used on the right hand side of operator '?'"

> > Source/WebCore/bindings/js/IDBBindingUtilities.cpp:457
> > +        return Deprecated::ScriptValue();
> 
> {}

Yup.

> > Source/WebCore/bindings/js/SerializedScriptValue.cpp:1788
> >              return 0;
> 
> These should be false

There's lots of "integer-as-bool" in this file. Should probably be cleaned up at once.

> > Source/WebCore/bindings/js/SerializedScriptValue.cpp:2433
> > +        for (; i < m_blobURLs.size(); ++i) {
> 
> Could this be a map, so we don't need to iterate?
> 
> > Source/WebCore/fileapi/ThreadableBlobRegistry.cpp:108
> > +        queue = new MessageQueue<CrossThreadTask>;
> 
> Would NeverDestroyed work here?

A cursory glance at all of our call_once initializers suggests we do "new", "malloc", "CFCreate...", "...alloc] init..." inside without bothering with neverdestroyed.

There's definitely no static initializer problem here, so I'm not sure what the point is.

[Brady Eidson]

Created attachment 276365 [details]
Patch v2

[WebKit Commit Bot]

Comment on attachment 276365 [details]
Patch v2

Clearing flags on attachment: 276365

Committed r199524: <http://trac.webkit.org/changeset/199524>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.