Summary: | Use one object instead of two for closures, eliminating ScopeChainNode | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||
Component: | New Bugs | Assignee: | 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
Geoffrey Garen
2012-08-30 14:50:20 PDT
Created attachment 161570 [details]
Patch
Committed r127202: <http://trac.webkit.org/changeset/127202> 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 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 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 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. Temporarily Skipped the test in http://trac.webkit.org/changeset/127292. Looking at this now. 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. Investigation bug: <rdar://problem/12217815> New test: <http://trac.webkit.org/changeset/127323>. |