From 9790bbbe74e7605aaa53918a20543599a38a8671 Mon Sep 17 00:00:00 2001
From: Blake Kaplan <mrbkap@gmail.com>
Date: Thu, 3 Feb 2011 20:13:18 -0800
Subject: [PATCH] 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                            | 38 ++++++++++++++-----
 js/src/xpconnect/tests/mochitest/Makefile.in  |  3 ++
 .../tests/mochitest/test1_bug629331.html      | 19 ++++++++++
 .../tests/mochitest/test2_bug629331.html      | 18 +++++++++
 .../tests/mochitest/test_bug629331.html       | 38 +++++++++++++++++++
 5 files changed, 106 insertions(+), 10 deletions(-)
 create mode 100644 js/src/xpconnect/tests/mochitest/test1_bug629331.html
 create mode 100644 js/src/xpconnect/tests/mochitest/test2_bug629331.html
 create mode 100644 js/src/xpconnect/tests/mochitest/test_bug629331.html

diff --git a/js/src/jsproxy.cpp b/js/src/jsproxy.cpp
index 9ea6a34ec1d..102d8684678 100644
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -148,27 +148,45 @@ JSProxyHandler::set(JSContext *cx, JSObject *proxy, JSObject *receiver, jsid id,
         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);
 }
@@ -497,8 +515,8 @@ JSScriptedProxyHandler::getPropertyDescriptor(JSContext *cx, JSObject *proxy, js
     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
@@ -510,8 +528,8 @@ JSScriptedProxyHandler::getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy,
     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
diff --git a/js/src/xpconnect/tests/mochitest/Makefile.in b/js/src/xpconnect/tests/mochitest/Makefile.in
index 9980a9b39ee..de99de66ad0 100644
--- a/js/src/xpconnect/tests/mochitest/Makefile.in
+++ b/js/src/xpconnect/tests/mochitest/Makefile.in
@@ -79,6 +79,9 @@ _TEST_FILES =	bug500931_helper.html \
 		bug589028_helper.html \
 		test_bug605167.html \
 		test_bug601299.html \
+		test_bug629331.html \
+		test1_bug629331.html \
+		test2_bug629331.html \
 		$(NULL)
 
 		#test_bug484107.html \
diff --git a/js/src/xpconnect/tests/mochitest/test1_bug629331.html b/js/src/xpconnect/tests/mochitest/test1_bug629331.html
new file mode 100644
index 00000000000..ec1d1a3b171
--- /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>
+
+
diff --git a/js/src/xpconnect/tests/mochitest/test2_bug629331.html b/js/src/xpconnect/tests/mochitest/test2_bug629331.html
new file mode 100644
index 00000000000..1bcf0373988
--- /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>
diff --git a/js/src/xpconnect/tests/mochitest/test_bug629331.html b/js/src/xpconnect/tests/mochitest/test_bug629331.html
new file mode 100644
index 00000000000..975200caf26
--- /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>