Bug 629331 - Fix handling of class getters with slotful values. r=gal, r=brendan, a=blocker
authorBlake Kaplan <mrbkap@gmail.com>
Thu, 03 Feb 2011 20:13:18 -0800
changeset 61920 d37f71235123ed544df53da2fe8dc11b3c0ed659
parent 61919 95fa72a8e8ab387ae57d177addfa9b93f1873170
child 61921 ed958cdc83961766749b7b0d0f7ae415e9b77905
push idunknown
push userunknown
push dateunknown
reviewersgal, brendan, blocker
bugs629331
milestone2.0b12pre
Bug 629331 - Fix handling of class getters with slotful values. r=gal, r=brendan, a=blocker try: -b do -p linux,win32 -u all
js/src/jsproxy.cpp
js/src/xpconnect/tests/mochitest/Makefile.in
js/src/xpconnect/tests/mochitest/test1_bug629331.html
js/src/xpconnect/tests/mochitest/test2_bug629331.html
js/src/xpconnect/tests/mochitest/test_bug629331.html
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -143,37 +143,55 @@ bool
 JSProxyHandler::set(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id, Value *vp)
 {
     JS_ASSERT(OperationInProgress(cx, proxy));
     AutoPropertyDescriptorRooter desc(cx);
     if (!getOwnPropertyDescriptor(cx, proxy, id, true, &desc))
         return false;
     /* The control-flow here differs from ::get() because of the fall-through case below. */
     if (desc.obj) {
-        if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub))
-            return CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp);
         if (desc.attrs & JSPROP_READONLY)
             return true;
+        if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub)) {
+            if (!CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp))
+                return false;
+            if (desc.attrs & JSPROP_SHARED)
+                return true;
+        }
+        if (!desc.getter)
+            desc.getter = PropertyStub;
+        if (!desc.setter)
+            desc.setter = PropertyStub;
         desc.value = *vp;
         return defineProperty(cx, receiver, id, &desc);
     }
     if (!getPropertyDescriptor(cx, proxy, id, true, &desc))
         return false;
     if (desc.obj) {
-        if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub))
-            return CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp);
         if (desc.attrs & JSPROP_READONLY)
             return true;
+        if (desc.setter && ((desc.attrs & JSPROP_SETTER) || desc.setter != PropertyStub)) {
+            if (!CallSetter(cx, receiver, id, desc.setter, desc.attrs, desc.shortid, vp))
+                return false;
+            if (desc.attrs & JSPROP_SHARED)
+                return true;
+        }
+        if (!desc.getter)
+            desc.getter = PropertyStub;
+        if (!desc.setter)
+            desc.setter = PropertyStub;
         /* fall through */
+    } else {
+        /* Pick up the class getter/setter. */
+        desc.getter = desc.setter = NULL;
     }
+
     desc.obj = receiver;
     desc.value = *vp;
     desc.attrs = JSPROP_ENUMERATE;
-    desc.getter = NULL;
-    desc.setter = NULL;
     desc.shortid = 0;
     return defineProperty(cx, receiver, id, &desc);
 }
 
 bool
 JSProxyHandler::keys(JSContext *cx, JSObject *proxy, AutoIdVector &props)
 {
     JS_ASSERT(OperationInProgress(cx, proxy));
@@ -492,31 +510,31 @@ bool
 JSScriptedProxyHandler::getPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, bool set,
                                               PropertyDescriptor *desc)
 {
     JSObject *handler = GetProxyHandlerObject(cx, proxy);
     AutoValueRooter tvr(cx);
     return GetFundamentalTrap(cx, handler, ATOM(getPropertyDescriptor), tvr.addr()) &&
            Trap1(cx, handler, tvr.value(), id, tvr.addr()) &&
            ((tvr.value().isUndefined() && IndicatePropertyNotFound(cx, desc)) ||
-            ReturnedValueMustNotBePrimitive(cx, proxy, ATOM(getPropertyDescriptor), tvr.value()) &&
-            ParsePropertyDescriptorObject(cx, proxy, id, tvr.value(), desc));
+            (ReturnedValueMustNotBePrimitive(cx, proxy, ATOM(getPropertyDescriptor), tvr.value()) &&
+             ParsePropertyDescriptorObject(cx, proxy, id, tvr.value(), desc)));
 }
 
 bool
 JSScriptedProxyHandler::getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, bool set,
                                                  PropertyDescriptor *desc)
 {
     JSObject *handler = GetProxyHandlerObject(cx, proxy);
     AutoValueRooter tvr(cx);
     return GetFundamentalTrap(cx, handler, ATOM(getOwnPropertyDescriptor), tvr.addr()) &&
            Trap1(cx, handler, tvr.value(), id, tvr.addr()) &&
            ((tvr.value().isUndefined() && IndicatePropertyNotFound(cx, desc)) ||
-            ReturnedValueMustNotBePrimitive(cx, proxy, ATOM(getPropertyDescriptor), tvr.value()) &&
-            ParsePropertyDescriptorObject(cx, proxy, id, tvr.value(), desc));
+            (ReturnedValueMustNotBePrimitive(cx, proxy, ATOM(getPropertyDescriptor), tvr.value()) &&
+             ParsePropertyDescriptorObject(cx, proxy, id, tvr.value(), desc)));
 }
 
 bool
 JSScriptedProxyHandler::defineProperty(JSContext *cx, JSObject *proxy, jsid id,
                                        PropertyDescriptor *desc)
 {
     JSObject *handler = GetProxyHandlerObject(cx, proxy);
     AutoValueRooter tvr(cx);
--- a/js/src/xpconnect/tests/mochitest/Makefile.in
+++ b/js/src/xpconnect/tests/mochitest/Makefile.in
@@ -74,14 +74,17 @@ include $(topsrcdir)/config/rules.mk
 		test_bug564330.html \
 		test_frameWrapping.html \
 		test_bug585745.html \
 		test_bug589028.html \
 		test_bug628410.html \
 		bug589028_helper.html \
 		test_bug605167.html \
 		test_bug601299.html \
+		test_bug629331.html \
+		test1_bug629331.html \
+		test2_bug629331.html \
 		$(NULL)
 
 		#test_bug484107.html \
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/js/src/xpconnect/tests/mochitest/test1_bug629331.html
@@ -0,0 +1,19 @@
+<body>
+<iframe src="about:blank" id="ifr"></iframe>
+<script>
+/** Test for Bug 629331 **/
+function finish() {
+    parent.postMessage(JSON.stringify({fun: "finish"}), "*");
+}
+
+function is(a, b, description) {
+    parent.postMessage(JSON.stringify({ fun: "is", a: a, b: b, description: description }), "*");
+}
+
+document.domain = "example.org";
+var i = 0;
+is(i, 0, 'i meets starting conditions');
+document.getElementById('ifr').src = 'http://test2.example.org/tests/js/src/xpconnect/tests/mochitest/test2_bug629331.html';
+</script>
+
+
new file mode 100644
--- /dev/null
+++ b/js/src/xpconnect/tests/mochitest/test2_bug629331.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+  <head>
+  </head>
+  <body>
+    <script>
+      document.domain = "example.org";
+
+      for (var j = 1; j <= 9; j++) {
+        parent.i = j;
+        var locali = parent.i;
+        parent.is(locali, j, 'step ' + j + ' worked');
+      }
+
+      parent.finish();
+    </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/js/src/xpconnect/tests/mochitest/test_bug629331.html
@@ -0,0 +1,38 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=629331
+-->
+<head>
+  <title>Test for Bug 629331</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=629331">Mozilla Bug 629331</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+SimpleTest.waitForExplicitFinish();
+
+function handler(event) {
+    var obj = JSON.parse(event.data);
+    if (obj.fun == "finish") {
+        SimpleTest.finish();
+    } else {
+        is(obj.a, obj.b, obj.description);
+    }
+}
+
+window.addEventListener('message', handler, false);
+
+</script>
+<iframe src="http://test1.example.org/tests/js/src/xpconnect/tests/mochitest/test1_bug629331.html">
+</iframe>
+</pre>
+</body>
+</html>