Bug 1043690 - Part 1: Provide helper function for HTMLDocument and HTMLFormElement proxies to use from [[Set]]. r=efaust, a=lmandel
authorJason Orendorff <jorendorff@mozilla.com>
Tue, 29 Jul 2014 20:27:18 -0500
changeset 208257 8375886783f2
parent 208256 ac0afa7b1b25
child 208258 06542873b0dc
push id3792
push userryanvm@gmail.com
push date2014-08-07 18:27 +0000
treeherdermozilla-beta@7b29fabbf26a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust, lmandel
bugs1043690
milestone32.0
Bug 1043690 - Part 1: Provide helper function for HTMLDocument and HTMLFormElement proxies to use from [[Set]]. r=efaust, a=lmandel
js/src/jsapi-tests/moz.build
js/src/jsapi-tests/testSetPropertyIgnoringNamedGetter.cpp
js/src/jsfriendapi.h
js/src/jsproxy.cpp
--- a/js/src/jsapi-tests/moz.build
+++ b/js/src/jsapi-tests/moz.build
@@ -61,16 +61,17 @@ UNIFIED_SOURCES += [
     'testPropCache.cpp',
     'testRegExp.cpp',
     'testResolveRecursion.cpp',
     'tests.cpp',
     'testSameValue.cpp',
     'testScriptInfo.cpp',
     'testScriptObject.cpp',
     'testSetProperty.cpp',
+    'testSetPropertyIgnoringNamedGetter.cpp',
     'testSourcePolicy.cpp',
     'testStringBuffer.cpp',
     'testStructuredClone.cpp',
     'testToIntWidth.cpp',
     'testTrap.cpp',
     'testTypedArrays.cpp',
     'testUncaughtError.cpp',
     'testUTF8.cpp',
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/testSetPropertyIgnoringNamedGetter.cpp
@@ -0,0 +1,92 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=8 sts=4 et sw=4 tw=99:
+ */
+
+#include "jsfriendapi.h"
+#include "jsproxy.h"
+
+#include "jsapi-tests/tests.h"
+
+using namespace js;
+using namespace JS;
+
+class CustomProxyHandler : public DirectProxyHandler {
+  public:
+    CustomProxyHandler() : DirectProxyHandler(nullptr) {}
+
+    bool getPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
+                               MutableHandle<JSPropertyDescriptor> desc) const MOZ_OVERRIDE
+    {
+        return impl(cx, proxy, id, desc, false);
+    }
+
+    bool getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
+                                  MutableHandle<JSPropertyDescriptor> desc) const MOZ_OVERRIDE
+    {
+        return impl(cx, proxy, id, desc, true);
+    }
+
+    bool set(JSContext *cx, HandleObject proxy, HandleObject receiver,
+             HandleId id, bool strict, MutableHandleValue vp) const MOZ_OVERRIDE
+    {
+        Rooted<JSPropertyDescriptor> desc(cx);
+        if (!DirectProxyHandler::getPropertyDescriptor(cx, proxy, id, &desc))
+            return false;
+        return SetPropertyIgnoringNamedGetter(cx, this, proxy, receiver, id, &desc,
+                                              desc.object() == proxy, strict, vp);
+    }
+
+  private:
+    bool impl(JSContext *cx, HandleObject proxy, HandleId id,
+              MutableHandle<JSPropertyDescriptor> desc, bool ownOnly) const
+    {
+        if (JSID_IS_STRING(id)) {
+            bool match;
+            if (!JS_StringEqualsAscii(cx, JSID_TO_STRING(id), "phantom", &match))
+                return false;
+            if (match) {
+                desc.object().set(proxy);
+                desc.attributesRef() = JSPROP_READONLY | JSPROP_ENUMERATE;
+                desc.value().setInt32(42);
+                return true;
+            }
+        }
+
+        if (ownOnly)
+            return DirectProxyHandler::getOwnPropertyDescriptor(cx, proxy, id, desc);
+        return DirectProxyHandler::getPropertyDescriptor(cx, proxy, id, desc);
+    }
+
+};
+
+const CustomProxyHandler customProxyHandler;
+
+
+BEGIN_TEST(testSetPropertyIgnoringNamedGetter_direct)
+{
+    RootedValue protov(cx);
+    EVAL("Object.prototype", &protov);
+
+    RootedValue targetv(cx);
+    EVAL("({})", &targetv);
+
+    RootedObject proxyObj(cx, NewProxyObject(cx, &customProxyHandler, targetv,
+                                             &protov.toObject(), global, ProxyOptions()));
+    CHECK(proxyObj);
+
+    CHECK(JS_DefineProperty(cx, global, "target", targetv, 0));
+    CHECK(JS_DefineProperty(cx, global, "proxy", proxyObj, 0));
+
+    RootedValue v(cx);
+    EVAL("Object.getOwnPropertyDescriptor(proxy, 'phantom').value", &v);
+    CHECK_SAME(v, Int32Value(42));
+
+    EXEC("proxy.phantom = 123");
+    EVAL("Object.getOwnPropertyDescriptor(proxy, 'phantom').value", &v);
+    CHECK_SAME(v, Int32Value(42));
+    EVAL("target.phantom", &v);
+    CHECK_SAME(v, Int32Value(123));
+
+    return true;
+}
+END_TEST(testSetPropertyIgnoringNamedGetter_direct)
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -39,16 +39,20 @@ class JSLinearString;
 struct JSJitInfo;
 class JSErrorReport;
 
 namespace JS {
 template <class T>
 class Heap;
 } /* namespace JS */
 
+namespace js {
+class JS_FRIEND_API(BaseProxyHandler);
+} /* namespace js */
+
 extern JS_FRIEND_API(void)
 JS_SetGrayGCRootsTracer(JSRuntime *rt, JSTraceDataOp traceOp, void *data);
 
 extern JS_FRIEND_API(JSString *)
 JS_GetAnonymousString(JSRuntime *rt);
 
 extern JS_FRIEND_API(JSObject *)
 JS_FindCompilationScope(JSContext *cx, JS::HandleObject obj);
@@ -2179,16 +2183,43 @@ DefaultValue(JSContext *cx, JS::HandleOb
  * a sort of extension point, but there is no hook in js::Class,
  * js::ProxyHandler, or the JSAPI with precisely the right semantics for it.
  */
 extern JS_FRIEND_API(bool)
 CheckDefineProperty(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::HandleValue value,
                     unsigned attrs,
                     JSPropertyOp getter = nullptr, JSStrictPropertyOp setter = nullptr);
 
+/*
+ * Helper function for HTMLDocument and HTMLFormElement.
+ *
+ * These are the only two interfaces that have [OverrideBuiltins], a named
+ * getter, and no named setter. They're implemented as proxies with a custom
+ * getOwnPropertyDescriptor() method. Unfortunately, overriding
+ * getOwnPropertyDescriptor() automatically affects the behavior of set(),
+ * which normally is just common sense but is *not* desired for these two
+ * interfaces.
+ *
+ * The fix is for these two interfaces to override set() to ignore the
+ * getOwnPropertyDescriptor() override.
+ *
+ * SetPropertyIgnoringNamedGetter is exposed to make it easier to override
+ * set() in this way.  It carries out all the steps of BaseProxyHandler::set()
+ * except the initial getOwnPropertyDescriptor()/getPropertyDescriptor() calls.
+ * The caller must supply those results as the 'desc' and 'descIsOwn'
+ * parameters.
+ *
+ * Implemented in jsproxy.cpp.
+ */
+JS_FRIEND_API(bool)
+SetPropertyIgnoringNamedGetter(JSContext *cx, const BaseProxyHandler *handler,
+                               JS::HandleObject proxy, JS::HandleObject receiver,
+                               JS::HandleId id, JS::MutableHandle<JSPropertyDescriptor> desc,
+                               bool descIsOwn, bool strict, JS::MutableHandleValue vp);
+
 } /* namespace js */
 
 extern JS_FRIEND_API(bool)
 js_DefineOwnProperty(JSContext *cx, JSObject *objArg, jsid idArg,
                      JS::Handle<JSPropertyDescriptor> descriptor, bool *bp);
 
 extern JS_FRIEND_API(bool)
 js_ReportIsNotFunction(JSContext *cx, JS::HandleValue v);
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -153,79 +153,98 @@ BaseProxyHandler::get(JSContext *cx, Han
 }
 
 bool
 BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
                       HandleId id, bool strict, MutableHandleValue vp)
 {
     assertEnteredPolicy(cx, proxy, id, SET);
 
+    // Find an own or inherited property. The code here is strange for maximum
+    // backward compatibility with earlier code written before ES6 and before
+    // SetPropertyIgnoringNamedGetter.
     Rooted<PropertyDescriptor> desc(cx);
     if (!getOwnPropertyDescriptor(cx, proxy, id, &desc))
         return false;
+    bool descIsOwn = desc.object() != nullptr;
+    if (!descIsOwn) {
+        if (!getPropertyDescriptor(cx, proxy, id, &desc))
+            return false;
+    }
+
+    return SetPropertyIgnoringNamedGetter(cx, this, proxy, receiver, id, &desc, descIsOwn, strict,
+                                          vp);
+}
+
+bool
+js::SetPropertyIgnoringNamedGetter(JSContext *cx, const BaseProxyHandler *handler,
+                                   HandleObject proxy, HandleObject receiver,
+                                   HandleId id, MutableHandle<PropertyDescriptor> desc,
+                                   bool descIsOwn, bool strict, MutableHandleValue vp)
+{
     /* The control-flow here differs from ::get() because of the fall-through case below. */
+    if (descIsOwn) {
+        JS_ASSERT(desc.object());
+
+        // Check for read-only properties.
+        if (desc.isReadonly())
+            return strict ? Throw(cx, id, JSMSG_CANT_REDEFINE_PROP) : true;
+        if (!desc.setter()) {
+            // Be wary of the odd explicit undefined setter case possible through
+            // Object.defineProperty.
+            if (!desc.hasSetterObject())
+                desc.setSetter(JS_StrictPropertyStub);
+        } else if (desc.hasSetterObject() || desc.setter() != JS_StrictPropertyStub) {
+            if (!CallSetter(cx, receiver, id, desc.setter(), desc.attributes(), strict, vp))
+                return false;
+            if (!proxy->is<ProxyObject>() || proxy->as<ProxyObject>().handler() != handler)
+                return true;
+            if (desc.isShared())
+                return true;
+        }
+        if (!desc.getter()) {
+            // Same as above for the null setter case.
+            if (!desc.hasGetterObject())
+                desc.setGetter(JS_PropertyStub);
+        }
+        desc.value().set(vp.get());
+        return handler->defineProperty(cx, receiver, id, desc);
+    }
     if (desc.object()) {
         // Check for read-only properties.
         if (desc.isReadonly())
             return strict ? Throw(cx, id, JSMSG_CANT_REDEFINE_PROP) : true;
         if (!desc.setter()) {
             // Be wary of the odd explicit undefined setter case possible through
             // Object.defineProperty.
             if (!desc.hasSetterObject())
                 desc.setSetter(JS_StrictPropertyStub);
         } else if (desc.hasSetterObject() || desc.setter() != JS_StrictPropertyStub) {
             if (!CallSetter(cx, receiver, id, desc.setter(), desc.attributes(), strict, vp))
                 return false;
-            if (!proxy->is<ProxyObject>() || proxy->as<ProxyObject>().handler() != this)
+            if (!proxy->is<ProxyObject>() || proxy->as<ProxyObject>().handler() != handler)
                 return true;
             if (desc.isShared())
                 return true;
         }
         if (!desc.getter()) {
             // Same as above for the null setter case.
             if (!desc.hasGetterObject())
                 desc.setGetter(JS_PropertyStub);
         }
         desc.value().set(vp.get());
-        return defineProperty(cx, receiver, id, &desc);
-    }
-    if (!getPropertyDescriptor(cx, proxy, id, &desc))
-        return false;
-    if (desc.object()) {
-        // Check for read-only properties.
-        if (desc.isReadonly())
-            return strict ? Throw(cx, id, JSMSG_CANT_REDEFINE_PROP) : true;
-        if (!desc.setter()) {
-            // Be wary of the odd explicit undefined setter case possible through
-            // Object.defineProperty.
-            if (!desc.hasSetterObject())
-                desc.setSetter(JS_StrictPropertyStub);
-        } else if (desc.hasSetterObject() || desc.setter() != JS_StrictPropertyStub) {
-            if (!CallSetter(cx, receiver, id, desc.setter(), desc.attributes(), strict, vp))
-                return false;
-            if (!proxy->is<ProxyObject>() || proxy->as<ProxyObject>().handler() != this)
-                return true;
-            if (desc.isShared())
-                return true;
-        }
-        if (!desc.getter()) {
-            // Same as above for the null setter case.
-            if (!desc.hasGetterObject())
-                desc.setGetter(JS_PropertyStub);
-        }
-        desc.value().set(vp.get());
-        return defineProperty(cx, receiver, id, &desc);
+        return handler->defineProperty(cx, receiver, id, desc);
     }
 
     desc.object().set(receiver);
     desc.value().set(vp.get());
     desc.setAttributes(JSPROP_ENUMERATE);
     desc.setGetter(nullptr);
     desc.setSetter(nullptr); // Pick up the class getter/setter.
-    return defineProperty(cx, receiver, id, &desc);
+    return handler->defineProperty(cx, receiver, id, desc);
 }
 
 bool
 BaseProxyHandler::keys(JSContext *cx, HandleObject proxy, AutoIdVector &props)
 {
     assertEnteredPolicy(cx, proxy, JSID_VOID, ENUMERATE);
     JS_ASSERT(props.length() == 0);