Bug 163916 – IDBDatabase.createObjectStore() should take a IDBObjectStoreParameters dictionary in parameter

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

[Chris Dumez]

IDBDatabase.createObjectStore() should take a IDBObjectStoreParameters dictionary in parameter:
- https://www.w3.org/TR/IndexedDB/#idl-def-IDBDatabase
- https://www.w3.org/TR/IndexedDB/#idl-def-IDBObjectStoreParameters

Align our IDL with the specification.

[Chris Dumez]

Created attachment 292663 [details]
Patch

[Chris Dumez]

Created attachment 292665 [details]
Patch

[Build Bot]

Comment on attachment 292665 [details]
Patch

Attachment 292665 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2361392

New failing tests:
animations/3d/change-transform-in-end-event.html

[Build Bot]

Created attachment 292678 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Chris Dumez]

Comment on attachment 292665 [details]
Patch

The iOS EWS failure is likely an unrelated flake.

[Darin Adler]

Comment on attachment 292665 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:216
> +    if (WTF::holds_alternative<String>(*variant)) {

The WTF:: here is not needed because of argument-dependent lookup. The argument is of type WTF::Variant and so it automatically looks for the function in the WTF namespace.

> Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:218
> +        m_string = WTFMove(WTF::get<String>(*variant));

Ditto.

> Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:221
> +        m_array = WTFMove(WTF::get<Vector<String>>(*variant));

Ditto.

> Source/WebCore/Modules/indexeddb/IDBKeyPath.h:36
> +using IDBKeyPathVariant = WTF::Variant<String, Vector<String>>;

I think we should consider putting this somewhere else. Once we use IDBKeyPathVariant everywhere, we probably won’t want an IDBKeyPath.h header any more. And I assume we will rename this type to IDBKeyPath at that time too.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4431
> -                $value = "${name}";
> +                $value = "WTFMove(${name})";

No need for the braces. Should just be WTFMove($name) like a couple lines up from here.

Also, the old code was a strange way of just assigning. Should have just been $value = $name.

[Chris Dumez]

Created attachment 292715 [details]
Patch

[Chris Dumez]

Comment on attachment 292665 [details]
Patch

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

>> Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:216
>> +    if (WTF::holds_alternative<String>(*variant)) {
> 
> The WTF:: here is not needed because of argument-dependent lookup. The argument is of type WTF::Variant and so it automatically looks for the function in the WTF namespace.

/Volumes/Data/WebKit/OpenSource/Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:216:9: error: no template named 'holds_alternative'; did you mean 'WTF::holds_alternative'?
In file included from /Volumes/Data/WebKit/OpenSource/Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:27:
In file included from /Volumes/Data/WebKit/OpenSource/Source/WebCore/Modules/indexeddb/IDBKeyPath.h:30:
/Volumes/Data/WebKit/OpenSource/WebKitBuild/Release/usr/local/include/wtf/Variant.h:1843:16: note: 'WTF::holds_alternative' declared here
constexpr bool holds_alternative(Variant<_Types...> const& __v) __NOEXCEPT{

>> Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:218
>> +        m_string = WTFMove(WTF::get<String>(*variant));
> 
> Ditto.

/Volumes/Data/WebKit/OpenSource/Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:218:28: error: use of undeclared identifier 'get'
/Volumes/Data/WebKit/OpenSource/Source/WebCore/Modules/indexeddb/IDBKeyPath.cpp:221:45: error: expected '(' for function-style cast or type construction

>> Source/WebCore/Modules/indexeddb/IDBKeyPath.h:36
>> +using IDBKeyPathVariant = WTF::Variant<String, Vector<String>>;
> 
> I think we should consider putting this somewhere else. Once we use IDBKeyPathVariant everywhere, we probably won’t want an IDBKeyPath.h header any more. And I assume we will rename this type to IDBKeyPath at that time too.

I figured this is a convenient place during the refactoring. I thought about renaming it and moving it once the refactoring is done.

>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4431
>> +                $value = "WTFMove(${name})";
> 
> No need for the braces. Should just be WTFMove($name) like a couple lines up from here.
> 
> Also, the old code was a strange way of just assigning. Should have just been $value = $name.

Ok.

[Chris Dumez]

Created attachment 292716 [details]
Patch

[Chris Dumez]

Comment on attachment 292716 [details]
Patch

Clearing flags on attachment: 292716

Committed r207806: <http://trac.webkit.org/changeset/207806>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.