Bug 95501 – Use one object instead of two for closures, eliminating ScopeChainNode

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Geoffrey Garen]

Use one object instead of two for closures, eliminating ScopeChainNode

[Geoffrey Garen]

Created attachment 161570 [details]
Patch

[Geoffrey Garen]

Committed r127202: <http://trac.webkit.org/changeset/127202>

[Oliver Hunt]

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]

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]

And on Lion:

http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r127199%20(2393)/results.html
http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r127203%20(2394)/http/tests/security/inactive-document-with-empty-security-origin-pretty-diff.html

[Jessie Berlin]

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]

Temporarily Skipped the test in http://trac.webkit.org/changeset/127292.

[Geoffrey Garen]

Looking at this now.

[Geoffrey Garen]

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]

Investigation bug: <rdar://problem/12217815>

[Geoffrey Garen]

New test: <http://trac.webkit.org/changeset/127323>.