Changeset 31267 in webkit

Timestamp:

Mar 24, 2008 7:26:24 PM (16 years ago)

Author:

weinig@apple.com

Message:

JavaScriptCore:

2008-03-23 Sam Weinig <​sam@webkit.org>

Reviewed by Darin Adler.

Fix ​http://bugs.webkit.org/show_bug.cgi?id=18048
The "thisObject" parameter to JSEvaluateScript is not used properly

Making passing a thisObject to JSEvaluateScript actually set the thisObject of the created
ExecState.

• API/testapi.c: (main): Add tests for setting the thisObject when calling JSEvaluateScript.

• kjs/ExecState.cpp: (KJS::ExecState::ExecState): Assign the thisObject to m_thisValue and remove the comment.

WebCore:

2008-03-24 Sam Weinig <​sam@webkit.org>

Reviewed by Darin Adler.

Fix ​http://bugs.webkit.org/show_bug.cgi?id=18048
The "thisObject" parameter to JSEvaluateScript is not used properly

• bindings/js/kjs_proxy.cpp: (WebCore::KJSProxy::evaluate): No need to pass a thisObject since we want the global object to be used.
• bridge/jni/jni_jsobject.mm: (JavaJSObject::eval): To avoid any change to this function, don't pass a thisObject to keep the same behavior.

Location:

trunk

Files:

• JavaScriptCore/API/testapi.c (modified) (1 diff)
• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/kjs/ExecState.cpp (modified) (2 diffs)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/kjs_proxy.cpp (modified) (1 diff)
• WebCore/bridge/jni/jni_jsobject.mm (modified) (2 diffs)

trunk/JavaScriptCore/API/testapi.c

@@ -870 +870 @@
     v = JSObjectCallAsFunction(context, function, o, 0, NULL, NULL);
     ASSERT(JSValueIsEqual(context, v, o, NULL));
-    
+
+    functionBody = JSStringCreateWithUTF8CString("return eval(\"this\");");
+    function = JSObjectMakeFunction(context, NULL, 0, NULL, functionBody, NULL, 1, NULL);
+    JSStringRelease(functionBody);
+    v = JSObjectCallAsFunction(context, function, NULL, 0, NULL, NULL);
+    ASSERT(JSValueIsEqual(context, v, globalObject, NULL));
+    v = JSObjectCallAsFunction(context, function, o, 0, NULL, NULL);
+    ASSERT(JSValueIsEqual(context, v, o, NULL));
+
+    JSStringRef script = JSStringCreateWithUTF8CString("this;");
+    v = JSEvaluateScript(context, script, NULL, NULL, 0, NULL);
+    ASSERT(JSValueIsEqual(context, v, globalObject, NULL));
+    v = JSEvaluateScript(context, script, o, NULL, 0, NULL);
+    ASSERT(JSValueIsEqual(context, v, o, NULL));
+    JSStringRelease(script);
+
+    script = JSStringCreateWithUTF8CString("eval(this);");
+    v = JSEvaluateScript(context, script, NULL, NULL, 0, NULL);
+    ASSERT(JSValueIsEqual(context, v, globalObject, NULL));
+    v = JSEvaluateScript(context, script, o, NULL, 0, NULL);
+    ASSERT(JSValueIsEqual(context, v, o, NULL));
+    JSStringRelease(script);
+
     char* scriptUTF8 = createStringWithContentsOfFile(scriptPath);
     if (!scriptUTF8)
         printf("FAIL: Test script could not be loaded.\n");
     else {
-        JSStringRef script = JSStringCreateWithUTF8CString(scriptUTF8);
+        script = JSStringCreateWithUTF8CString(scriptUTF8);
         result = JSEvaluateScript(context, script, NULL, NULL, 1, &exception);
         if (JSValueIsUndefined(context, result))

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2008-03-23  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Fix http://bugs.webkit.org/show_bug.cgi?id=18048
+        The "thisObject" parameter to JSEvaluateScript is not used properly
+
+        Making passing a thisObject to JSEvaluateScript actually set the thisObject of the created
+        ExecState.
+
+        * API/testapi.c:
+        (main): Add tests for setting the thisObject when calling JSEvaluateScript.
+
+        * kjs/ExecState.cpp:
+        (KJS::ExecState::ExecState): Assign the thisObject to m_thisValue and remove the comment. 
+
 2008-03-22  Jesse Ruderman  <jruderman@gmail.com>
 

trunk/JavaScriptCore/kjs/ExecState.cpp

@@ -64 +64 @@
 }
 
-inline ExecState::ExecState(JSGlobalObject* globalObject, JSObject* /*thisObject*/, ProgramNode* programNode)
+inline ExecState::ExecState(JSGlobalObject* globalObject, JSObject* thisObject, ProgramNode* programNode)
     : m_globalObject(globalObject)
     , m_exception(0)
@@ -77 +77 @@
     , m_inlineScopeChainNode(0, 0)
     , m_variableObject(globalObject)
-    , m_thisValue(globalObject)
+    , m_thisValue(thisObject)
     , m_iterationDepth(0)
     , m_switchDepth(0) 
     , m_codeType(GlobalCode)
 {
-    // FIXME: This function ignores the "thisObject" parameter, which means that the API for evaluating
-    // a script with a this object that's not the same as the global object is broken, and probably
-    // has been for some time.
     ASSERT(m_scopeNode);
     m_scopeChain.push(globalObject);

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-03-24  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Fix http://bugs.webkit.org/show_bug.cgi?id=18048
+        The "thisObject" parameter to JSEvaluateScript is not used properly
+
+        * bindings/js/kjs_proxy.cpp:
+        (WebCore::KJSProxy::evaluate): No need to pass a thisObject since we want the global object to be used.
+        * bridge/jni/jni_jsobject.mm:
+        (JavaJSObject::eval): To avoid any change to this function, don't pass a thisObject to keep the same
+        behavior.
+
 2008-03-24  Brady Eidson  <beidson@apple.com>
 

trunk/WebCore/bindings/js/kjs_proxy.cpp

@@ -83 +83 @@
     m_frame->keepAlive();
 
-    // FIXME: Can this call to toJSDOMWindow be replaced by using m_globalObject.
-    JSValue* thisNode = toJSDOMWindow(m_frame);
-
     m_globalObject->startTimeoutCheck();
-    Completion comp = Interpreter::evaluate(exec, filename, baseLine, str.characters(), str.length(), thisNode);
+    Completion comp = Interpreter::evaluate(exec, filename, baseLine, str.characters(), str.length());
     m_globalObject->stopTimeoutCheck();
   

trunk/WebCore/bridge/jni/jni_jsobject.mm

@@ -311 +311 @@
     JS_LOG ("script = %s\n", JavaString(script).UTF8String());
     
-    JSObject *thisObj = const_cast<JSObject*>(_imp);
-    JSValue *result;
-    
+    JSValue* result;
+
     JSLock lock;
     
@@ -321 +320 @@
 
     rootObject->globalObject()->startTimeoutCheck();
-    Completion completion = Interpreter::evaluate(rootObject->globalObject()->globalExec(), UString(), 0, JavaString(script).ustring(),thisObj);
+    Completion completion = Interpreter::evaluate(rootObject->globalObject()->globalExec(), UString(), 0, JavaString(script).ustring());
     rootObject->globalObject()->stopTimeoutCheck();
     ComplType type = completion.complType();