Add @@ toStringTag to our iterator prototype object, as per: - https://heycam.github.io/webidl/#es-iterator-prototype-object
Created attachment 386245 [details] Patch
Comment on attachment 386245 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386245&action=review > Source/WebCore/bindings/js/JSDOMIterator.h:267 > + putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, info()->className), PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly); I guess this is the same pattern that prototypes in the JavaScript runtime follow, so I guess it’s appropriately efficient. Do we want to expose the actual values from className? No need to be indirect or hide that as an implementation detail?
(In reply to Darin Adler from comment #2) > Comment on attachment 386245 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=386245&action=review > > > Source/WebCore/bindings/js/JSDOMIterator.h:267 > > + putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, info()->className), PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly); > > I guess this is the same pattern that prototypes in the JavaScript runtime > follow, so I guess it’s appropriately efficient. > > Do we want to expose the actual values from className? No need to be > indirect or hide that as an implementation detail? Using info()->className here is important to pass the tests. info()->className looks like "URLSearchParams Iterator" and is set in our generated bindings. The spec says: """ The class string of an iterator prototype object for a given interface is the result of concatenating the identifier of the interface and the string " Iterator". """
Created attachment 386249 [details] Patch
Comment on attachment 386249 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386249&action=review > Source/WebCore/bindings/js/JSDOMIterator.h:268 > + putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, info()->className), PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly); JSC::PropertyAttribute perhaps?
Created attachment 386259 [details] Patch
Comment on attachment 386259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386259&action=review > Source/WebCore/bindings/js/JSDOMIterator.h:268 > + putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, info()->className), JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::ReadOnly); Might also have to be JSC::jsString -- not really sure.
Comment on attachment 386259 [details] Patch Clearing flags on attachment: 386259 Committed r253855: <https://trac.webkit.org/changeset/253855>
All reviewed patches have been landed. Closing bug.
<rdar://problem/58129948>
Comment on attachment 386259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386259&action=review >> Source/WebCore/bindings/js/JSDOMIterator.h:268 >> + putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, info()->className), JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::ReadOnly); > > Might also have to be JSC::jsString -- not really sure. I figured out why it does not. The type of the first argument is JSC::VM&, and so argument-dependent lookup kicks in to search the JSC namespace for the function name.
(In reply to Chris Dumez from comment #3) > (In reply to Darin Adler from comment #2) > > > Source/WebCore/bindings/js/JSDOMIterator.h:267 > > > + putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, info()->className), PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly); > > > > I guess this is the same pattern that prototypes in the JavaScript runtime > > follow, so I guess it’s appropriately efficient. > > > > Do we want to expose the actual values from className? No need to be > > indirect or hide that as an implementation detail? > > Using info()->className here is important to pass the tests. > info()->className looks like "URLSearchParams Iterator" and is set in our > generated bindings. > > The spec says: > """ > The class string of an iterator prototype object for a given interface is > the result of concatenating the identifier of the interface and the string " > Iterator". > """ Yes, I understand we need that name. What I wondered is whether it was OK to require that it be exactly equal to the className value from the info structure. The four examples I found that have a className that is different from the toStringTagSymbol value are: AsyncGeneratorFunctionPrototype: className="AsyncGenerator", toStringTag="AsyncGeneratorFunction" JSDataViewPrototype: className="DataViewPrototype", toStringTag="DataView" JSModuleNamespaceObject: className="ModuleNamespaceObject", toStringTag="Module" JSPromisePrototype: className="PromisePrototype", toStringTag="Promise" I was just wondering why this happens sometimes and why it shouldn't happen here. Maybe all four of those are just bugs.