Title: [171691] trunk/Source/_javascript_Core
Revision
171691
Author
mhahnenb...@apple.com
Date
2014-07-28 13:43:57 -0700 (Mon, 28 Jul 2014)

Log Message

REGRESSION: JSObjectSetPrototype() does not work on result of JSGetGlobalObject()
https://bugs.webkit.org/show_bug.cgi?id=135322

Reviewed by Oliver Hunt.

The prototype chain of the JSProxy object should match that of the JSGlobalObject. 

This is a separate but related issue with JSObjectSetPrototype which doesn't correctly 
account for JSProxies. I also audited the rest of the C API to check that we correctly 
handle JSProxies in all other situations where we expect a JSCallbackObject of some sort
and found some SPI calls (JSObject*PrivateProperty) that didn't behave correctly when 
passed a JSProxy.

I also added some new tests for these cases.

* API/JSObjectRef.cpp:
(JSObjectSetPrototype):
(JSObjectGetPrivateProperty):
(JSObjectSetPrivateProperty):
(JSObjectDeletePrivateProperty):
* API/JSWeakObjectMapRefPrivate.cpp:
* API/tests/CustomGlobalObjectClassTest.c:
(globalObjectSetPrototypeTest):
(globalObjectPrivatePropertyTest):
* API/tests/CustomGlobalObjectClassTest.h:
* API/tests/testapi.c:
(main):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/API/JSObjectRef.cpp (171690 => 171691)


--- trunk/Source/_javascript_Core/API/JSObjectRef.cpp	2014-07-28 20:41:16 UTC (rev 171690)
+++ trunk/Source/_javascript_Core/API/JSObjectRef.cpp	2014-07-28 20:43:57 UTC (rev 171691)
@@ -302,6 +302,14 @@
     JSObject* jsObject = toJS(object);
     JSValue jsValue = toJS(exec, value);
 
+    if (JSProxy* proxy = jsDynamicCast<JSProxy*>(jsObject)) {
+        if (JSGlobalObject* globalObject = jsDynamicCast<JSGlobalObject*>(proxy->target())) {
+            globalObject->resetPrototype(exec->vm(), jsValue.isObject() ? jsValue : jsNull());
+            return;
+        }
+        // Someday we might use proxies for something other than JSGlobalObjects, but today is not that day.
+        RELEASE_ASSERT_NOT_REACHED();
+    }
     jsObject->setPrototypeWithCycleCheck(exec, jsValue.isObject() ? jsValue : jsNull());
 }
 
@@ -501,6 +509,11 @@
     JSObject* jsObject = toJS(object);
     JSValue result;
     Identifier name(propertyName->identifier(&exec->vm()));
+
+    // Get wrapped object if proxied
+    if (jsObject->inherits(JSProxy::info()))
+        jsObject = jsCast<JSProxy*>(jsObject)->target();
+
     if (jsObject->inherits(JSCallbackObject<JSGlobalObject>::info()))
         result = jsCast<JSCallbackObject<JSGlobalObject>*>(jsObject)->getPrivateProperty(name);
     else if (jsObject->inherits(JSCallbackObject<JSDestructibleObject>::info()))
@@ -519,6 +532,11 @@
     JSObject* jsObject = toJS(object);
     JSValue jsValue = value ? toJS(exec, value) : JSValue();
     Identifier name(propertyName->identifier(&exec->vm()));
+
+    // Get wrapped object if proxied
+    if (jsObject->inherits(JSProxy::info()))
+        jsObject = jsCast<JSProxy*>(jsObject)->target();
+
     if (jsObject->inherits(JSCallbackObject<JSGlobalObject>::info())) {
         jsCast<JSCallbackObject<JSGlobalObject>*>(jsObject)->setPrivateProperty(exec->vm(), name, jsValue);
         return true;
@@ -542,6 +560,11 @@
     JSLockHolder locker(exec);
     JSObject* jsObject = toJS(object);
     Identifier name(propertyName->identifier(&exec->vm()));
+
+    // Get wrapped object if proxied
+    if (jsObject->inherits(JSProxy::info()))
+        jsObject = jsCast<JSProxy*>(jsObject)->target();
+
     if (jsObject->inherits(JSCallbackObject<JSGlobalObject>::info())) {
         jsCast<JSCallbackObject<JSGlobalObject>*>(jsObject)->deletePrivateProperty(name);
         return true;

Modified: trunk/Source/_javascript_Core/API/JSWeakObjectMapRefPrivate.cpp (171690 => 171691)


--- trunk/Source/_javascript_Core/API/JSWeakObjectMapRefPrivate.cpp	2014-07-28 20:41:16 UTC (rev 171690)
+++ trunk/Source/_javascript_Core/API/JSWeakObjectMapRefPrivate.cpp	2014-07-28 20:43:57 UTC (rev 171691)
@@ -62,7 +62,9 @@
     JSObject* obj = toJS(object);
     if (!obj)
         return;
-    ASSERT(obj->inherits(JSCallbackObject<JSGlobalObject>::info()) || obj->inherits(JSCallbackObject<JSDestructibleObject>::info()));
+    ASSERT(obj->inherits(JSProxy::info())
+        || obj->inherits(JSCallbackObject<JSGlobalObject>::info()) 
+        || obj->inherits(JSCallbackObject<JSDestructibleObject>::info()));
     map->map().set(key, obj);
 }
 

Modified: trunk/Source/_javascript_Core/API/tests/CustomGlobalObjectClassTest.c (171690 => 171691)


--- trunk/Source/_javascript_Core/API/tests/CustomGlobalObjectClassTest.c	2014-07-28 20:41:16 UTC (rev 171690)
+++ trunk/Source/_javascript_Core/API/tests/CustomGlobalObjectClassTest.c	2014-07-28 20:43:57 UTC (rev 171691)
@@ -25,6 +25,7 @@
 
 #include "CustomGlobalObjectClassTest.h"
 
+#include <_javascript_Core/JSObjectRefPrivate.h>
 #include <_javascript_Core/_javascript_Core.h>
 #include <stdio.h>
 
@@ -98,3 +99,47 @@
 
     assertTrue(executedCallback, "Executed custom global object callback");
 }
+
+void globalObjectSetPrototypeTest()
+{
+    JSClassDefinition definition = kJSClassDefinitionEmpty;
+    definition.className = "Global";
+    JSClassRef global = JSClassCreate(&definition);
+    JSGlobalContextRef context = JSGlobalContextCreate(global);
+    JSObjectRef object = JSContextGetGlobalObject(context);
+
+    JSObjectRef above = JSObjectMake(context, 0, 0);
+    JSStringRef test = JSStringCreateWithUTF8CString("test");
+    JSValueRef value = JSValueMakeString(context, test);
+    JSObjectSetProperty(context, above, test, value, kJSPropertyAttributeDontEnum, 0);
+
+    JSObjectSetPrototype(context, object, above);
+    JSStringRef script = JSStringCreateWithUTF8CString("test === \"test\"");
+    JSValueRef result = JSEvaluateScript(context, script, 0, 0, 0, 0);
+
+    assertTrue(JSValueToBoolean(context, result), "test === \"test\"");
+
+    JSStringRelease(test);
+    JSStringRelease(script);
+}
+
+void globalObjectPrivatePropertyTest()
+{
+    JSClassDefinition definition = kJSClassDefinitionEmpty;
+    definition.className = "Global";
+    JSClassRef global = JSClassCreate(&definition);
+    JSGlobalContextRef context = JSGlobalContextCreate(global);
+    JSObjectRef globalObject = JSContextGetGlobalObject(context);
+
+    JSStringRef privateName = JSStringCreateWithUTF8CString("private");
+    JSValueRef privateValue = JSValueMakeString(context, privateName);
+    assertTrue(JSObjectSetPrivateProperty(context, globalObject, privateName, privateValue), "JSObjectSetPrivateProperty succeeded");
+    JSValueRef result = JSObjectGetPrivateProperty(context, globalObject, privateName);
+    assertTrue(JSValueIsStrictEqual(context, privateValue, result), "privateValue === \"private\"");
+
+    assertTrue(JSObjectDeletePrivateProperty(context, globalObject, privateName), "JSObjectDeletePrivateProperty succeeded");
+    result = JSObjectGetPrivateProperty(context, globalObject, privateName);
+    assertTrue(JSValueIsNull(context, result), "Deleted private property is indeed no longer present");
+
+    JSStringRelease(privateName);
+}

Modified: trunk/Source/_javascript_Core/API/tests/CustomGlobalObjectClassTest.h (171690 => 171691)


--- trunk/Source/_javascript_Core/API/tests/CustomGlobalObjectClassTest.h	2014-07-28 20:41:16 UTC (rev 171690)
+++ trunk/Source/_javascript_Core/API/tests/CustomGlobalObjectClassTest.h	2014-07-28 20:43:57 UTC (rev 171691)
@@ -27,5 +27,7 @@
 #define CustomGlobalObjectClassTest_h
 
 void customGlobalObjectClassTest(void);
+void globalObjectSetPrototypeTest(void);
+void globalObjectPrivatePropertyTest(void);
 
 #endif // CustomGlobalObjectClassTest_h

Modified: trunk/Source/_javascript_Core/API/tests/testapi.c (171690 => 171691)


--- trunk/Source/_javascript_Core/API/tests/testapi.c	2014-07-28 20:41:16 UTC (rev 171690)
+++ trunk/Source/_javascript_Core/API/tests/testapi.c	2014-07-28 20:43:57 UTC (rev 171691)
@@ -2079,6 +2079,8 @@
         printf("PASS: global context name behaves as expected.\n");
 
     customGlobalObjectClassTest();
+    globalObjectSetPrototypeTest();
+    globalObjectPrivatePropertyTest();
 
     if (failed) {
         printf("FAIL: Some tests failed.\n");

Modified: trunk/Source/_javascript_Core/ChangeLog (171690 => 171691)


--- trunk/Source/_javascript_Core/ChangeLog	2014-07-28 20:41:16 UTC (rev 171690)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-07-28 20:43:57 UTC (rev 171691)
@@ -1,3 +1,33 @@
+2014-07-28  Mark Hahnenberg  <mhahnenb...@apple.com>
+
+        REGRESSION: JSObjectSetPrototype() does not work on result of JSGetGlobalObject()
+        https://bugs.webkit.org/show_bug.cgi?id=135322
+
+        Reviewed by Oliver Hunt.
+
+        The prototype chain of the JSProxy object should match that of the JSGlobalObject. 
+
+        This is a separate but related issue with JSObjectSetPrototype which doesn't correctly 
+        account for JSProxies. I also audited the rest of the C API to check that we correctly 
+        handle JSProxies in all other situations where we expect a JSCallbackObject of some sort
+        and found some SPI calls (JSObject*PrivateProperty) that didn't behave correctly when 
+        passed a JSProxy.
+
+        I also added some new tests for these cases.
+
+        * API/JSObjectRef.cpp:
+        (JSObjectSetPrototype):
+        (JSObjectGetPrivateProperty):
+        (JSObjectSetPrivateProperty):
+        (JSObjectDeletePrivateProperty):
+        * API/JSWeakObjectMapRefPrivate.cpp:
+        * API/tests/CustomGlobalObjectClassTest.c:
+        (globalObjectSetPrototypeTest):
+        (globalObjectPrivatePropertyTest):
+        * API/tests/CustomGlobalObjectClassTest.h:
+        * API/tests/testapi.c:
+        (main):
+
 2014-07-28  Filip Pizlo  <fpi...@apple.com>
 
         Make sure that we don't use non-speculative BooleanToNumber for a speculative Branch
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to