Bug 95501

Summary: Use one object instead of two for closures, eliminating ScopeChainNode
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, jberlin, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 95530, 95561    
Bug Blocks:    
Attachments:
Description Flags
Patch fpizlo: review+

Geoffrey Garen
Reported 2012-08-30 14:50:20 PDT
Use one object instead of two for closures, eliminating ScopeChainNode
Attachments
Patch (230.21 KB, patch)
2012-08-30 15:14 PDT, Geoffrey Garen 		fpizlo: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2012-08-30 15:14:29 PDT
Created attachment 161570 [details] Patch
Geoffrey Garen
Comment 2 2012-08-30 15:49:34 PDT
Committed r127202: <http://trac.webkit.org/changeset/127202>
Oliver Hunt
Comment 3 2012-08-30 15:56:50 PDT
Comment on attachment 161570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161570&action=review > Source/JavaScriptCore/jit/JIT.h:63 > - class ScopeChainNode; > + class JSScope; Fix the order! http://webkitmemes.tumblr.com/post/18265177987/the-most-interesting-reviewer-in-the-world
Jessie Berlin
Comment 4 2012-08-31 09:16:23 PDT
It appears this change started causing http/tests/security/inactive-document-with-empty-security-origin.html to start timing out on Mountain Lion: http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r127199%20(382)/results.html http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r127202%20(383)/results.html
Jessie Berlin
Comment 6 2012-08-31 09:25:45 PDT
This is one of the few remaining tests that are making the Lion WK1 bots red, so please investigate this soon or I will have to consider rolling out your patch.
Jessie Berlin
Comment 7 2012-08-31 11:13:38 PDT
Temporarily Skipped the test in http://trac.webkit.org/changeset/127292.
Geoffrey Garen
Comment 8 2012-08-31 11:23:44 PDT
Looking at this now.
Geoffrey Garen
Comment 9 2012-08-31 14:42:45 PDT
Looks like the new behavior (hanging on inactive-document-with-empty-security-origin-pretty-diff.html) is correct. I'm going to: (*) Commit a new reduced test demonstrating the behavior change from my patch. (*) File a bug for investigating why the old test hangs / fails (*) Leave the old test skipped I tried to fix the old test, but it didn't have any explanation of what it was trying to test or how to verify whether it was correct, so I didn't make quick headway.
Geoffrey Garen
Comment 10 2012-08-31 14:43:13 PDT
Investigation bug: <rdar://problem/12217815>
Geoffrey Garen
Comment 11 2012-08-31 14:46:54 PDT
New test: <http://trac.webkit.org/changeset/127323>.
Note You need to log in before you can comment on or make changes to this bug.