Bug 1469217 part 1 - Remove JSPROP_PROPOP_ACCESSORS. r=anba
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 21 Jun 2018 11:05:41 +0200
changeset 479988 ca87fb0412c2d145b0eec9a8f18673fa89f4b4e9
parent 479987 f7ec87cfa1bce4db98df6e4dee6b44e42bd96252
child 479989 66fd5497203f27524e233c7dca3bed9d5eba87d7
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1469217
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1469217 part 1 - Remove JSPROP_PROPOP_ACCESSORS. r=anba
js/src/jsapi-tests/testSetProperty.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/vm/JSObject.cpp
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/src/jsapi-tests/testSetProperty.cpp
+++ b/js/src/jsapi-tests/testSetProperty.cpp
@@ -2,71 +2,16 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jsapi-tests/tests.h"
 
-BEGIN_TEST(testSetProperty_NativeGetterStubSetter)
-{
-    JS::RootedObject obj(cx, JS_NewPlainObject(cx));
-    CHECK(obj);
-
-    CHECK(JS_DefineProperty(cx, global, "globalProp", obj, JSPROP_ENUMERATE));
-
-    CHECK(JS_DefineProperty(cx, obj, "prop",
-                            JS_PROPERTYOP_GETTER(NativeGet), nullptr,
-                            JSPROP_PROPOP_ACCESSORS));
-
-    EXEC("'use strict';                                     \n"
-         "var error, passed = false;                        \n"
-         "try                                               \n"
-         "{                                                 \n"
-         "  this.globalProp.prop = 42;                      \n"
-         "  throw new Error('setting property succeeded!'); \n"
-         "}                                                 \n"
-         "catch (e)                                         \n"
-         "{                                                 \n"
-         "  error = e;                                      \n"
-         "  if (e instanceof TypeError)                     \n"
-         "    passed = true;                                \n"
-         "}                                                 \n"
-         "                                                  \n"
-         "if (!passed)                                      \n"
-         "  throw error;                                    \n");
-
-    EXEC("var error, passed = false;                        \n"
-         "try                                               \n"
-         "{                                                 \n"
-         "  this.globalProp.prop = 42;                      \n"
-         "  if (this.globalProp.prop === 17)                \n"
-         "    passed = true;                                \n"
-         "  else                                            \n"
-         "    throw new Error('bad value after set!');      \n"
-         "}                                                 \n"
-         "catch (e)                                         \n"
-         "{                                                 \n"
-         "  error = e;                                      \n"
-         "}                                                 \n"
-         "                                                  \n"
-         "if (!passed)                                      \n"
-         "  throw error;                                    \n");
-
-    return true;
-}
-static bool
-NativeGet(JSContext* cx, JS::HandleObject obj, JS::HandleId id, JS::MutableHandleValue vp)
-{
-    vp.setInt32(17);
-    return true;
-}
-END_TEST(testSetProperty_NativeGetterStubSetter)
-
 BEGIN_TEST(testSetProperty_InheritedGlobalSetter)
 {
     // This is a JSAPI test because jsapi-test globals do not have a resolve
     // hook and therefore can use the property cache in some cases where the
     // shell can't.
     MOZ_RELEASE_ASSERT(!JS_GetClass(global)->getResolve());
 
     CHECK(JS_DefineProperty(cx, global, "HOTLOOP", 8, 0));
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -2112,52 +2112,47 @@ DefineAccessorPropertyById(JSContext* cx
     if (attrs & (JSPROP_GETTER | JSPROP_SETTER))
         attrs &= ~JSPROP_READONLY;
 
     // When we use DefineProperty, we need full scriptable Function objects rather
     // than JSNatives. However, we might be pulling this property descriptor off
     // of something with JSNative property descriptors. If we are, wrap them in
     // JS Function objects.
 
-    // If !(attrs & JSPROP_PROPOP_ACCESSORS), then getter/setter are both
-    // possibly-null JSNatives (or possibly-null JSFunction* if JSPROP_GETTER or
-    // JSPROP_SETTER is appropriately set).
-    if (!(attrs & JSPROP_PROPOP_ACCESSORS)) {
-        if (getter && !(attrs & JSPROP_GETTER)) {
-            RootedAtom atom(cx, IdToFunctionName(cx, id, FunctionPrefixKind::Get));
-            if (!atom)
-                return false;
-            JSFunction* getobj = NewNativeFunction(cx, (Native) getter, 0, atom);
-            if (!getobj)
-                return false;
-
-            if (get.info)
-                getobj->setJitInfo(get.info);
-
-            getter = JS_DATA_TO_FUNC_PTR(GetterOp, getobj);
-            attrs |= JSPROP_GETTER;
-        }
-        if (setter && !(attrs & JSPROP_SETTER)) {
-            // Root just the getter, since the setter is not yet a JSObject.
-            AutoRooterGetterSetter getRoot(cx, JSPROP_GETTER, &getter, nullptr);
-            RootedAtom atom(cx, IdToFunctionName(cx, id, FunctionPrefixKind::Set));
-            if (!atom)
-                return false;
-            JSFunction* setobj = NewNativeFunction(cx, (Native) setter, 1, atom);
-            if (!setobj)
-                return false;
-
-            if (set.info)
-                setobj->setJitInfo(set.info);
-
-            setter = JS_DATA_TO_FUNC_PTR(SetterOp, setobj);
-            attrs |= JSPROP_SETTER;
-        }
-    } else {
-        attrs &= ~JSPROP_PROPOP_ACCESSORS;
+    // Getter/setter are both possibly-null JSNatives (or possibly-null
+    // JSFunction* if JSPROP_GETTER or JSPROP_SETTER is appropriately set).
+    if (getter && !(attrs & JSPROP_GETTER)) {
+        RootedAtom atom(cx, IdToFunctionName(cx, id, FunctionPrefixKind::Get));
+        if (!atom)
+            return false;
+        JSFunction* getobj = NewNativeFunction(cx, (Native) getter, 0, atom);
+        if (!getobj)
+            return false;
+
+        if (get.info)
+            getobj->setJitInfo(get.info);
+
+        getter = JS_DATA_TO_FUNC_PTR(GetterOp, getobj);
+        attrs |= JSPROP_GETTER;
+    }
+    if (setter && !(attrs & JSPROP_SETTER)) {
+        // Root just the getter, since the setter is not yet a JSObject.
+        AutoRooterGetterSetter getRoot(cx, JSPROP_GETTER, &getter, nullptr);
+        RootedAtom atom(cx, IdToFunctionName(cx, id, FunctionPrefixKind::Set));
+        if (!atom)
+            return false;
+        JSFunction* setobj = NewNativeFunction(cx, (Native) setter, 1, atom);
+        if (!setobj)
+            return false;
+
+        if (set.info)
+            setobj->setJitInfo(set.info);
+
+        setter = JS_DATA_TO_FUNC_PTR(SetterOp, setobj);
+        attrs |= JSPROP_SETTER;
     }
 
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj, id,
                           (attrs & JSPROP_GETTER)
                           ? JS_FUNC_TO_DATA_PTR(JSObject*, getter)
                           : nullptr,
@@ -2167,17 +2162,17 @@ DefineAccessorPropertyById(JSContext* cx
 
     return js::DefineAccessorProperty(cx, obj, id, getter, setter, attrs);
 }
 
 static bool
 DefineDataPropertyById(JSContext* cx, HandleObject obj, HandleId id, HandleValue value,
                        unsigned attrs)
 {
-    MOZ_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER | JSPROP_PROPOP_ACCESSORS)));
+    MOZ_ASSERT(!(attrs & (JSPROP_GETTER | JSPROP_SETTER)));
 
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj, id, value);
 
     return js::DefineDataProperty(cx, obj, id, value, attrs);
 }
 
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -471,19 +471,17 @@ static const uint8_t JSPROP_ENUMERATE = 
 
 /* not settable: assignment is no-op.  This flag is only valid when neither
    JSPROP_GETTER nor JSPROP_SETTER is set. */
 static const uint8_t JSPROP_READONLY =         0x02;
 
 /* property cannot be deleted */
 static const uint8_t JSPROP_PERMANENT =        0x04;
 
-/* Passed to JS_Define(UC)Property* and JS_DefineElement if getters/setters are
-   JSGetterOp/JSSetterOp */
-static const uint8_t JSPROP_PROPOP_ACCESSORS = 0x08;
+/* (0x08 is unused) */
 
 /* property holds getter function */
 static const uint8_t JSPROP_GETTER =           0x10;
 
 /* property holds setter function */
 static const uint8_t JSPROP_SETTER =           0x20;
 
 /* internal JS engine use only */
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -2801,18 +2801,16 @@ js::DefineProperty(JSContext* cx, Handle
     return NativeDefineProperty(cx, obj.as<NativeObject>(), id, desc, result);
 }
 
 bool
 js::DefineAccessorProperty(JSContext* cx, HandleObject obj, HandleId id,
                            JSGetterOp getter, JSSetterOp setter, unsigned attrs,
                            ObjectOpResult& result)
 {
-    MOZ_ASSERT(!(attrs & JSPROP_PROPOP_ACCESSORS));
-
     Rooted<PropertyDescriptor> desc(cx);
     desc.initFields(nullptr, UndefinedHandleValue, attrs, getter, setter);
     if (DefinePropertyOp op = obj->getOpsDefineProperty()) {
         MOZ_ASSERT(!cx->helperThread());
         return op(cx, obj, id, desc, result);
     }
     return NativeDefineProperty(cx, obj.as<NativeObject>(), id, desc, result);
 }
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -442,18 +442,17 @@ TryResolvePropertyFromSpecs(JSContext* c
                                                  JSSetterOp));
             }
             desc.setAttributes(flags);
             if (!JS_DefinePropertyById(cx, holder, id,
                                        JS_PROPERTYOP_GETTER(desc.getter()),
                                        JS_PROPERTYOP_SETTER(desc.setter()),
                                        // This particular descriptor, unlike most,
                                        // actually stores JSNatives directly,
-                                       // since we just set it up.  Do NOT pass
-                                       // JSPROP_PROPOP_ACCESSORS here!
+                                       // since we just set it up.
                                        desc.attributes()))
             {
                 return false;
             }
         } else {
             RootedValue v(cx);
             if (!psMatch->getValue(cx, &v))
                 return false;