Upgrade Map and Set constructor interface
One concern is that, Set / Map is already shipped in Safari 8 with different interface. https://bugs.webkit.org/show_bug.cgi?id=141351#c11
Created attachment 247973 [details] rev1 prototype rev1 prototype. Tests not included.
Created attachment 248069 [details] Patch
Attachment 248069 [details] did not pass style-queue: ERROR: LayoutTests/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 248071 [details] Patch
OK, patch is ready for review. Joseph, what do you think of? Any runtime flags are needed? I'd like to hear your opinion for this breaking change from Safari 8. At least, Map interface doesn't break things because it didn't add any argument into its data. However, since Set added its argument to its data in the different style from the latest spec, it is breaking change.
(In reply to comment #6) > OK, patch is ready for review. > > Joseph, what do you think of? Any runtime flags are needed? I'd like to hear > your opinion for this breaking change from Safari 8. > > At least, Map interface doesn't break things because it didn't add any > argument into its data. > However, since Set added its argument to its data in the different style > from the latest spec, it is breaking change. I think we should just land it! We are actually aligning with other browsers with this change (Firefox/Chrome). For vendors that have shipped WebKit with the older, incompatible Set/Map constructors they may want to mention this in release notes to inform developers ("JavaScript Set/Map constructor behavior change ...") but I don't really think there is anything we should need to do here to workaround the issues of the past. In the unlikely event that this breaks something significant we can revisit this.
(In reply to comment #7) > I think we should just land it! We are actually aligning with other browsers > with this change (Firefox/Chrome). > > For vendors that have shipped WebKit with the older, incompatible Set/Map > constructors they may want to mention this in release notes to inform > developers ("JavaScript Set/Map constructor behavior change ...") but I > don't really think there is anything we should need to do here to workaround > the issues of the past. > > In the unlikely event that this breaks something significant we can revisit > this. Agreed. Thank you for your comment :D
I'll resubmit patch with a little fix about IteratorClose(value)
(In reply to comment #9) > I'll resubmit patch with a little fix about IteratorClose(value) Ah, sorry, it's no problem. current patch is correct.
Comment on attachment 248071 [details] Patch Ah, before r?, I'll add WeakMap / WeakSet upgrade too.
Created attachment 248093 [details] Patch
I've upgraded the patch with WeakMap. WeakSet is not implemented in JSC, I've opened issue 142408.
Created attachment 248094 [details] Patch
Comment on attachment 248094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=248094&action=review Added one note. > Source/JavaScriptCore/runtime/WeakMapConstructor.cpp:140 > + callData.native.function = callWeakMap; One note is that, in Call case, we need to set some function to make `typeof WeakMap === 'function'`.
Comment on attachment 248094 [details] Patch Attachment 248094 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4757494261874688 New failing tests: js/dom/basic-weakmap.html
Created attachment 248098 [details] Archive of layout-test-results from ews100 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 248099 [details] Patch
Comment on attachment 248099 [details] Patch R=me. We'll watch bugreports in nightlies to see if this is the kind of breaking change that actually breaks websites. I bet not.
Comment on attachment 248099 [details] Patch Clearing flags on attachment: 248099 Committed r181333: <http://trac.webkit.org/changeset/181333>
All reviewed patches have been landed. Closing bug.
(In reply to comment #19) > Comment on attachment 248099 [details] > Patch > > R=me. We'll watch bugreports in nightlies to see if this is the kind of > breaking change that actually breaks websites. I bet not. Landed, thank you :D