Bug 1043690, part 1 - Provide helper function for HTMLDocument and HTMLFormElement proxies to use from [[Set]]. r=efaust

js/src/jsapi-tests/moz.build

@@ -66,6 +66,7 @@ UNIFIED_SOURCES += [
     'testScriptInfo.cpp',
     'testScriptObject.cpp',
     'testSetProperty.cpp',
+    'testSetPropertyIgnoringNamedGetter.cpp',
     'testSourcePolicy.cpp',
     'testStringBuffer.cpp',
     'testStructuredClone.cpp',

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)

js/src/jsfriendapi.h

@@ -46,6 +46,10 @@ 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);
 
@@ -2460,6 +2464,33 @@ CheckDefineProperty(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::Ha
                     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);
+
 JS_FRIEND_API(void)
 ReportErrorWithId(JSContext *cx, const char *msg, JS::HandleId id);
 

js/src/jsproxy.cpp

@@ -162,11 +162,32 @@ BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
 {
     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 (desc.object()) {
+    if (descIsOwn) {
+        JS_ASSERT(desc.object());
+
         // Check for read-only properties.
         if (desc.isReadonly())
             return strict ? Throw(cx, id, JSMSG_READ_ONLY) : true;
@@ -178,7 +199,7 @@ BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
         } 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;
@@ -189,10 +210,8 @@ BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
                 desc.setGetter(JS_PropertyStub);
         }
         desc.value().set(vp.get());
-        return defineProperty(cx, receiver, id, &desc);
+        return handler->defineProperty(cx, receiver, id, desc);
     }
-    if (!getPropertyDescriptor(cx, proxy, id, &desc))
-        return false;
     if (desc.object()) {
         // Check for read-only properties.
         if (desc.isReadonly())
@@ -205,7 +224,7 @@ BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
         } 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;
@@ -216,7 +235,7 @@ BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
                 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);
@@ -224,7 +243,7 @@ BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
     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