Originally, it is hidden in web pages because it causes Facebook's comment input. When landing Object.getOwnPropertySymbols, original issue is solved, so I think it's time to enable Symbols in web pages. And I suggest exposing only effective well-known symbols. Here's all well-known symbols. Specification Name [[Description]] Value and Purpose @@hasInstance "Symbol.hasInstance" A method that determines if a constructor object recognizes an object as one of the constructor’s instances. Called by the semantics of the instanceof operator. @@isConcatSpreadable "Symbol.isConcatSpreadable" A Boolean valued property that if true indicates that an object should be flattened to its array elements by Array.prototype.concat. @@iterator "Symbol.iterator" A method that returns the default Iterator for an object. Called by the semantics of the for-of statement. @@match "Symbol.match" A regular expression method that matches the regular expression against a string. Called by the String.prototype.match method. @@replace "Symbol.replace" A regular expression method that replaces matched substrings of a string. Called by the String.prototype.replace method. @@search "Symbol.search" A regular expression method that returns the index within a string that matches the regular expression. Called by the String.prototype.search method. @@species "Symbol.species" A function valued property that is the constructor function that is used to create derived objects. @@split "Symbol.split" A regular expression method that splits a string at the indices that match the regular expression. Called by the String.prototype.split method. @@toPrimitive "Symbol.toPrimitive" A method that converts an object to a corresponding primitive value. Called by the ToPrimitive abstract operation. @@toStringTag "Symbol.toStringTag" A String valued property that is used in the creation of the default string description of an object. Accessed by the built-in method Object.prototype.toString. @@unscopables "Symbol.unscopables" An object valued property whose own property names are property names that are excluded from the with environment bindings of the associated object. In them, JSC supports the following 2 symbols. So at this time, exposing them. @@iterator @@unscopables
Created attachment 250160 [details] Patch
After https://bugs.webkit.org/show_bug.cgi?id=143404, I'm planning to land it and enable symbols in web pages.
Comment on attachment 250160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250160&action=review > Source/JavaScriptCore/ChangeLog:18 > + and makes enabling symbols by default. Or is it preferable to drop this runtime flag completely? This runtime flag will affect when adding Array.prototype.values.
Comment on attachment 250160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250160&action=review >> Source/JavaScriptCore/ChangeLog:18 >> + and makes enabling symbols by default. > > Or is it preferable to drop this runtime flag completely? > This runtime flag will affect when adding Array.prototype.values. Not sure about others, but I think we should dump it completely.
Comment on attachment 250160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250160&action=review >>> Source/JavaScriptCore/ChangeLog:18 >>> + and makes enabling symbols by default. >> >> Or is it preferable to drop this runtime flag completely? >> This runtime flag will affect when adding Array.prototype.values. > > Not sure about others, but I think we should dump it completely. I think we should keep it for a while given the previous attempt to enable this feature broke Facebook.com although I agree we should remove it eventually.
> I think we should keep it for a while given the previous attempt to enable > this feature broke Facebook.com > although I agree we should remove it eventually. +1
Are you going to land this patch?
I was planning to land it after https://bugs.webkit.org/show_bug.cgi?id=143404 is landed. But maybe Symbol.for is not so used. So I'll land it first ;)
Comment on attachment 250160 [details] Patch Clearing flags on attachment: 250160 Committed r182653: <http://trac.webkit.org/changeset/182653>
All reviewed patches have been landed. Closing bug.
JSC tests fail. However, it's very curious. "weak value == nil" is failed, but in this patch, changes in JSC shell is very few. Now in this patch, we only attached implemented well known symbols. So, before this patch, Symbol has the following well known symbols property. macro(hasInstance) \ macro(isConcatSpreadable) \ macro(iterator) \ macro(match) \ macro(replace) \ macro(search) \ macro(species) \ macro(split) \ macro(toPrimitive) \ macro(toStringTag) \ macro(unscopables) After this patch, we only attached macro(iterator) \ macro(unscopables) After investigating the cause of JSC tests failure, I've found that when adding not implemented well known symbols, JSC tests pass. macro(hasInstance) \ macro(isConcatSpreadable) \ macro(iterator) \ macro(match) \ macro(replace) \ macro(search) \ macro(species) \ macro(split) \ macro(toPrimitive) \ macro(toStringTag) \ macro(unscopables) But I think it's storage...
After investigation, I've found the test sometimes pass / fail when inserting breakpoint in lldb. And this failure is caused because the previous JSGlobalObject is not collected. I suppose that because previous JSGlobalObject's pointer is still alive in registers or C stacks, it's not released and it causes this failure.
OK, so, since it's related to conservative GC (stack/register values) issue, now tests pass.
Opened issue for this API test failure. https://bugs.webkit.org/show_bug.cgi?id=143634
Comment on attachment 250160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250160&action=review > Source/WebCore/inspector/InspectorFrontendClientLocal.cpp:124 > m_frontendPage->settings().setJavaScriptRuntimeFlags({ > - JSC::RuntimeFlags::SymbolEnabled > }); Does this function need to be called at all with the empty arguments? > Source/WebKit/mac/WebView/WebPreferencesPrivate.h:56 > - WebKitJavaScriptRuntimeFlagsSymbolEnabled = 1u << 0, > + WebKitJavaScriptRuntimeFlagsSymbolDisabled = 1u << 0, Why do we have an SPI flag for this, does any WebKit client want to disable Symbol? "Private" headers are SPI, meaning that internal Apple clients can rely on them. So, exposing more than we need has a long-term cost, and once they are exposed, they generally cannot be changed. > Source/WebKit2/UIProcess/API/C/WKPreferencesRefPrivate.h:53 > - kWKJavaScriptRuntimeFlagsSymbolEnabled = 1 << 0, > + kWKJavaScriptRuntimeFlagsSymbolDisabled = 1 << 0, Ditto.
(In reply to comment #15) > > > Source/WebKit/mac/WebView/WebPreferencesPrivate.h:56 > > - WebKitJavaScriptRuntimeFlagsSymbolEnabled = 1u << 0, > > + WebKitJavaScriptRuntimeFlagsSymbolDisabled = 1u << 0, > > Why do we have an SPI flag for this, does any WebKit client want to disable > Symbol? > > "Private" headers are SPI, meaning that internal Apple clients can rely on > them. So, exposing more than we need has a long-term cost, and once they are > exposed, they generally cannot be changed. > > > Source/WebKit2/UIProcess/API/C/WKPreferencesRefPrivate.h:53 > > - kWKJavaScriptRuntimeFlagsSymbolEnabled = 1 << 0, > > + kWKJavaScriptRuntimeFlagsSymbolDisabled = 1 << 0, > > Ditto. That's a good point. We should probably remove these runtime flags from private headers right now since we don't need them to disable the feature on websites.
Comment on attachment 250160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250160&action=review >> Source/WebCore/inspector/InspectorFrontendClientLocal.cpp:124 >> }); > > Does this function need to be called at all with the empty arguments? To search runtime flags when changing it, calling it here is nice I think.
(In reply to comment #16) > (In reply to comment #15) > > > > > Source/WebKit/mac/WebView/WebPreferencesPrivate.h:56 > > > - WebKitJavaScriptRuntimeFlagsSymbolEnabled = 1u << 0, > > > + WebKitJavaScriptRuntimeFlagsSymbolDisabled = 1u << 0, > > > > Why do we have an SPI flag for this, does any WebKit client want to disable > > Symbol? > > > > "Private" headers are SPI, meaning that internal Apple clients can rely on > > them. So, exposing more than we need has a long-term cost, and once they are > > exposed, they generally cannot be changed. > > > > > Source/WebKit2/UIProcess/API/C/WKPreferencesRefPrivate.h:53 > > > - kWKJavaScriptRuntimeFlagsSymbolEnabled = 1 << 0, > > > + kWKJavaScriptRuntimeFlagsSymbolDisabled = 1 << 0, > > > > Ditto. > > That's a good point. We should probably remove these runtime flags from > private headers right now since we don't need them to disable the feature on > websites. Is it better moving them to public headers?
No, we shouldn't expose it as SPI or API. Random app embedding WebKit shouldn't be able to change this runtime flag at all since we want to remove it soon. For binary compatibility, we can't easily remove SPI or API once added.