JavaScriptCore:

        Reviewed by Maciej & Darin.

        Fixes Bug 16868: Gmail crash
          and Bug 16871: Crash when loading apple.com/startpage

        <http://bugs.webkit.org/show_bug.cgi?id=16868>
        <rdar://problem/5686108>

        <http://bugs.webkit.org/show_bug.cgi?id=16871>
        <rdar://problem/5686670>

        Adds ActivationImp tear-off for cross-window eval() and fixes an
        existing garbage collection issue exposed by the ActivationImp tear-off
        patch (r29425) that can occur when an ExecState's m_callingExec is
        different than its m_savedExec.

        * kjs/ExecState.cpp:
        (KJS::ExecState::mark):
        * kjs/function.cpp:
        (KJS::GlobalFuncImp::callAsFunction):

LayoutTests:

        Reviewed by Maciej.

        Added a test that checks whether ActivationImp tear-off occurs before
        a cross-window eval(). Relevant to

        Bug 16868: Gmail crash

        <http://bugs.webkit.org/show_bug.cgi?id=16868>
        <rdar://problem/5686108>

        * fast/js/window-eval-tearoff-expected.txt: Added.
        * fast/js/window-eval-tearoff.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29542 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 3a8a1819262647395d6c81424d152310a0073969..26d6ef3bcdd164f9b8809c4eee13e8f87947a7a5 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,26 @@
+2008-01-16  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
+
+        Reviewed by Maciej & Darin.
+
+        Fixes Bug 16868: Gmail crash
+          and Bug 16871: Crash when loading apple.com/startpage
+
+        <http://bugs.webkit.org/show_bug.cgi?id=16868>
+        <rdar://problem/5686108>
+
+        <http://bugs.webkit.org/show_bug.cgi?id=16871>
+        <rdar://problem/5686670>
+
+        Adds ActivationImp tear-off for cross-window eval() and fixes an
+        existing garbage collection issue exposed by the ActivationImp tear-off
+        patch (r29425) that can occur when an ExecState's m_callingExec is
+        different than its m_savedExec.
+
+        * kjs/ExecState.cpp:
+        (KJS::ExecState::mark):
+        * kjs/function.cpp:
+        (KJS::GlobalFuncImp::callAsFunction):
+
 2008-01-16  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Oliver.

diff --git a/JavaScriptCore/kjs/ExecState.cpp b/JavaScriptCore/kjs/ExecState.cpp
index a2ec5dd2d0da2e245ac5c4873ecffdcedcce51a3..942da8815d3810fe5dbc83f6566cae5d44f85dbf 100644 (file)
--- a/JavaScriptCore/kjs/ExecState.cpp
+++ b/JavaScriptCore/kjs/ExecState.cpp
@@ -125,15 +125,12 @@ ExecState::~ExecState()
 
 void ExecState::mark()
 {
-    for (ExecState* exec = this; exec; exec = exec->m_callingExec)
+    for (ExecState* exec = this; exec; exec = exec->m_callingExec) {
         exec->m_scopeChain.mark();
 
-    // FIXME: It is surprising that this code is necessary, since at first
-    // glance it seems that all ActivationImps should be in a ScopeChain.
-    // However, <http://bugs.webkit.org/show_bug.cgi?id=16871> proves that is
-    // not the case.
-    if (m_activation && m_activation->isOnStack())
-        m_activation->markChildren();
+        if (exec->m_savedExec != exec->m_callingExec && exec->m_savedExec)
+            exec->m_savedExec->mark();
+    }
 }
 
 JSGlobalObject* ExecState::lexicalGlobalObject() const

diff --git a/JavaScriptCore/kjs/function.cpp b/JavaScriptCore/kjs/function.cpp
index 9ced9bbc004fc22c096eeaa3531e8dcdb92abe84..e33f8ee274acac2ac2fa3ed3e0685e2f28d59461 100644 (file)
--- a/JavaScriptCore/kjs/function.cpp
+++ b/JavaScriptCore/kjs/function.cpp
@@ -750,9 +750,7 @@ JSValue* GlobalFuncImp::callAsFunction(ExecState* exec, JSObject* thisObj, const
         bool switchGlobal = thisObj && thisObj != exec->dynamicGlobalObject() && thisObj->isGlobalObject();
 
         // enter a new execution context
-        if (!switchGlobal)
-            exec->dynamicGlobalObject()->tearOffActivation(exec);
-        
+        exec->dynamicGlobalObject()->tearOffActivation(exec);
         JSGlobalObject* globalObject = switchGlobal ? static_cast<JSGlobalObject*>(thisObj) : exec->dynamicGlobalObject();
         ExecState newExec(globalObject, evalNode.get(), exec);
           

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6ec9071baf548c1769cbecd037f55950fa9bcbb1..15eef4ee6d0855316cf3daed9d4aca121422d314 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2008-01-16  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
+
+        Reviewed by Maciej.
+
+        Added a test that checks whether ActivationImp tear-off occurs before
+        a cross-window eval(). Relevant to
+
+        Bug 16868: Gmail crash
+
+        <http://bugs.webkit.org/show_bug.cgi?id=16868>
+        <rdar://problem/5686108>
+
+        * fast/js/window-eval-tearoff-expected.txt: Added.
+        * fast/js/window-eval-tearoff.html: Added.
+
 2008-01-16  David Hyatt  <hyatt@apple.com>
 
         Update layout tests after fix for <rdar://problem/5681647>.

diff --git a/LayoutTests/fast/js/window-eval-tearoff-expected.txt b/LayoutTests/fast/js/window-eval-tearoff-expected.txt
new file mode 100644 (file)
index 0000000..c751703
--- /dev/null
+++ b/LayoutTests/fast/js/window-eval-tearoff-expected.txt
@@ -0,0 +1,2 @@
+
+Test that otherWindow.eval() performs ActivationImp tear-off: PASS

diff --git a/LayoutTests/fast/js/window-eval-tearoff.html b/LayoutTests/fast/js/window-eval-tearoff.html
new file mode 100644 (file)
index 0000000..d3d84d8
--- /dev/null
+++ b/LayoutTests/fast/js/window-eval-tearoff.html
@@ -0,0 +1,33 @@
+<body>
+<script>
+function print(message, color) 
+{
+    var paragraph = document.createElement("div");
+    paragraph.appendChild(document.createTextNode(message));
+    paragraph.style.fontFamily = "monospace";
+    if (color)
+        paragraph.style.color = color;
+    document.getElementById("console").appendChild(paragraph);
+}
+
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+<iframe id=i src='about:blank' width=10 height=10>
+</iframe>
+<div id=console></div>
+<script>
+var otherWindow = document.getElementById('i').contentWindow;
+var closure;
+
+function otherWindowClosure()
+{
+    var localVar = 1;
+    
+    return otherWindow.eval("(function () { return localVar; })");
+}
+
+closure = otherWindowClosure();
+
+print("Test that otherWindow.eval() performs ActivationImp tear-off: " + (closure() == 1 ? "PASS" : "FAIL")) ;
+</script>