Bug 95501 - Use one object instead of two for closures, eliminating ScopeChainNode
Summary: Use one object instead of two for closures, eliminating ScopeChainNode
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
Depends on: 95530 95561
  Show dependency treegraph
Reported: 2012-08-30 14:50 PDT by Geoffrey Garen
Modified: 2012-08-31 14:46 PDT (History)
3 users (show)

See Also:

Patch (230.21 KB, patch)
2012-08-30 15:14 PDT, Geoffrey Garen
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2012-08-30 14:50:20 PDT
Use one object instead of two for closures, eliminating ScopeChainNode
Comment 1 Geoffrey Garen 2012-08-30 15:14:29 PDT
Created attachment 161570 [details]
Comment 2 Geoffrey Garen 2012-08-30 15:49:34 PDT
Committed r127202: <http://trac.webkit.org/changeset/127202>
Comment 3 Oliver Hunt 2012-08-30 15:56:50 PDT
Comment on attachment 161570 [details]

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!

Comment 4 Jessie Berlin 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:

Comment 6 Jessie Berlin 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.
Comment 7 Jessie Berlin 2012-08-31 11:13:38 PDT
Temporarily Skipped the test in http://trac.webkit.org/changeset/127292.
Comment 8 Geoffrey Garen 2012-08-31 11:23:44 PDT
Looking at this now.
Comment 9 Geoffrey Garen 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.
Comment 10 Geoffrey Garen 2012-08-31 14:43:13 PDT
Investigation bug: <rdar://problem/12217815>
Comment 11 Geoffrey Garen 2012-08-31 14:46:54 PDT
New test: <http://trac.webkit.org/changeset/127323>.