Bug 154475

Summary: Builtins that should not rely on iteration do.
Product: WebKit Reporter: Mark S. Miller <erights>
Component: JavaScriptCoreAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, erights, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Open this web page in Safari and Webkit Nightly, and open the console
none
Screenshot showing Safari working and WebKit failing the badbind.html test for this bug
none
Patch
none
Benchmark results none

Mark S. Miller
Reported 2016-02-19 14:52:11 PST
Created attachment 271806 [details] Open this web page in Safari and Webkit Nightly, and open the console Now that the latest WebKit seems to mostly work again on the SES selftest page https://rawgit.com/tvcutsem/es-lab/master/src/ses/contract.html I was seeing some new failures. But this time I was able to debug (thanks!) and isolate the problem. The new problem is that the builtins used by http://wiki.ecmascript.org/doku.php?id=conventions:safe_meta_programming all of which redate ES6 now depend on iterator.next behavior in violation of the ES6 spec. You can see it yourself by opening the badbind.html in Safari and Webkit Nightly, opening the console, and seeing what's there. Works fine on Safari and on every other browser I tested. Fails on the latest WebKit nightly. Note that I'll be updating the SES selftest page to stop removing .next, because of this bug, so the SES selftest page will stop being a good test.
Attachments
Open this web page in Safari and Webkit Nightly, and open the console (1.47 KB, text/html)
2016-02-19 14:52 PST, Mark S. Miller
no flags
Screenshot showing Safari working and WebKit failing the badbind.html test for this bug (1.18 MB, image/png)
2016-02-19 14:53 PST, Mark S. Miller
no flags
Patch (2.98 KB, patch)
2016-02-22 09:41 PST, Keith Miller
no flags
Benchmark results (64.81 KB, text/plain)
2016-02-22 11:08 PST, Keith Miller
no flags
Mark S. Miller
Comment 1 2016-02-19 14:53:57 PST
Created attachment 271807 [details] Screenshot showing Safari working and WebKit failing the badbind.html test for this bug
Keith Miller
Comment 2 2016-02-19 15:09:14 PST
I think this is because of my patch (http://trac.webkit.org/changeset/196734). I didn't realize that arguments iterator was observable from outside it's scope. Working on this now.
Mark S. Miller
Comment 3 2016-02-19 15:58:36 PST
Although this specific test passes on all other browsers, note that Chrome50 Canary has a similar but different problem: https://bugs.chromium.org/p/v8/issues/detail?id=4769
Keith Miller
Comment 4 2016-02-22 09:41:37 PST
Keith Miller
Comment 5 2016-02-22 11:08:36 PST
Created attachment 271931 [details] Benchmark results
Geoffrey Garen
Comment 6 2016-02-22 11:38:07 PST
Comment on attachment 271926 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271926&action=review r=me > Source/JavaScriptCore/builtins/FunctionPrototype.js:33 > + @putByValDirect(argumentValues, i-1, arguments[i]); Spacing around - 1.
WebKit Commit Bot
Comment 7 2016-02-22 11:42:58 PST
Comment on attachment 271926 [details] Patch Clearing flags on attachment: 271926 Committed r196949: <http://trac.webkit.org/changeset/196949>
WebKit Commit Bot
Comment 8 2016-02-22 11:43:03 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.