Yeah, I'm not sure where in WebIDL the logic exists so that iterable interfaces are treated like sequences rather than records. It's probably in there somewhere....
Owner: raphael....@intel.com Status: Started (was: Available)
I'm taking a stab at this. It's a bit annoying that some of the record-related parts of the spec are still undergoing some changes, but I'll see how far I can get.
I finally have some working code that passes all of Boris' tests (which have since been added to web-platform-tests).
I had to add some local hacks to get things to pass though, as our union bindings code currently does not check for @@iterator to determine if it's passed a sequence and that basically breaks all tests.
My code still needs some cleanup, but think I can start sending some CLs.
If anyone's interested, I've been discussing this with bashi@, haraken@ and yukishiino@. https://codereview.chromium.org/2709983004/ contains a WIP version of everything that's needed to support records -- I think I covered lots of use cases in the layout test I added.
The biggest blocker is that the code in that CL generates per-record bindings files in order to convert the V8 keys and values into the right C++ objects (since we don't have a uniform way to convert strings, integers, dictionaries, unions, interfaces, DOMWindows etc etc). That isn't very good, so I've been focusing on trying to provide a more uniform interface for the V8->C++ conversion (see https://codereview.chromium.org/2725673002/ for a WIP patch in that direction).
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/5254358bc67bb4f7ef54cc68a69387d5aabd719e
commit 5254358bc67bb4f7ef54cc68a69387d5aabd719e
Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com>
Date: Thu Mar 09 15:26:17 2017
bindings: Adjust usage of CORE_EXPORT in IDLTypes and NativeValueTraits
Follow-up to https://codereview.chromium.org/2730183003:
* The types in IDLTypes.h are traits types with no member functions, so
using CORE_EXPORT does not have any effect and we can drop it.
* Export the entire NativeValueTraits specializations in
NativeValueTraitsImpl.h instead of only their nativeValue() member
functions.
In addition to avoiding some repetition in the code, doing so also solves
some linkage issues on Windows with debug component builds, where targets
such as 'webkit_unit_tests' or 'blink_media_unittests' would fail to
reference the aforementioned specializations and lead to errors like this:
FAILED: media_blink_unittests.exe media_blink_unittests.exe.pdb
E:/b/depot_tools/python276_bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64 False link.exe /nologo /OUT:./media_blink_unittests.exe /PDB:./media_blink_unittests.exe.pdb @./media_blink_unittests.exe.rsp
V8RecordTest.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static class WTF::Vector<struct std::pair<class WTF::String,int>,0,class WTF::PartitionAllocator> __cdecl blink::NativeValueTraits<struct blink::IDLRecord<struct blink::IDLString,struct blink::IDLLong> >::nativeValue(class v8::Isolate *,class v8::Local<class v8::Value>,class blink::ExceptionState &)" (__imp_?nativeValue@?$NativeValueTraits@U?$IDLRecord@UIDLString@blink@@UIDLLong@2@@blink@@@blink@@SA?AV?$Vector@U?$pair@VString@WTF@@H@std@@$0A@VPartitionAllocator@WTF@@@WTF@@PEAVIsolate@v8@@V?$Local@VValue@v8@@@6@AEAVExceptionState@2@@Z) referenced in function "void __cdecl blink::RecordTestV8Internal::setStringLongRecordMethod(class v8::FunctionCallbackInfo<class v8::Value> const &)" (?setStringLongRecordMethod@RecordTestV8Internal@blink@@YAXAEBV?$FunctionCallbackInfo@VValue@v8@@@v8@@@Z)
The exception to the rule here is IDLSequence: since it needs template
parameters of its own, it is not a full specialization of
NativeValueTraitsBase, and at least win_clang complains when it is
exported:
E:\b\c\b\win_clang\src\third_party\WebKit\Source\bindings\core\v8\NativeValueTraitsImpl.h(289,8): error: 'dllimport' attribute ignored [-Werror,-Wignored-attributes]
struct CORE_EXPORT NativeValueTraits<IDLSequence<T>>
BUG= 685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org
Review-Url: https://codereview.chromium.org/2742453002
Cr-Commit-Position: refs/heads/master@{#455750}
[modify] https://crrev.com/5254358bc67bb4f7ef54cc68a69387d5aabd719e/third_party/WebKit/Source/bindings/core/v8/IDLTypes.h
[modify] https://crrev.com/5254358bc67bb4f7ef54cc68a69387d5aabd719e/third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h
I guess we can finally close this one :)
record<K,V> support is there for most types. If things are not working as expected, I'd be glad to look at new bug reports. Big thanks to haraken, bashi and yukishiino for the help so far!
Comment 1 by raphael....@intel.com
, Jan 27 2017