Bug 1451017 - Remove Xray resolveNativeProperty. r=bz
authorTom Schuster <evilpies@gmail.com>
Tue, 08 May 2018 22:09:05 +0200
changeset 476116 0d24499ad4e81c211f892a3e2d025d2677b4eee8
parent 476115 38c222c1bf73be8ef89397c23c607dfe34d748ab
child 476117 18845b2e2a11fbfd27f532d01212d23a181a2860
push id9374
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:43:20 +0000
treeherdermozilla-beta@160e085dfb0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1451017
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 1451017 - Remove Xray resolveNativeProperty. r=bz
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1684,29 +1684,17 @@ HasNativeProperty(JSContext* cx, HandleO
     if (!traits->resolveOwnProperty(cx, wrapper, target, holder, id, &desc))
         return false;
     if (desc.object()) {
         *hasProp = true;
         return true;
     }
 
     // Try the holder.
-    bool found = false;
-    if (!JS_AlreadyHasOwnPropertyById(cx, holder, id, &found))
-        return false;
-    if (found) {
-        *hasProp = true;
-        return true;
-    }
-
-    // Try resolveNativeProperty.
-    if (!traits->resolveNativeProperty(cx, wrapper, holder, id, &desc))
-        return false;
-    *hasProp = !!desc.object();
-    return true;
+    return JS_AlreadyHasOwnPropertyById(cx, holder, id, hasProp);
 }
 
 } // namespace XrayUtils
 
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::preventExtensions(JSContext* cx, HandleObject wrapper,
@@ -1753,42 +1741,35 @@ XrayWrapper<Base, Traits>::getPropertyDe
     // shadows a previously-resolved non-own property that we cached on the
     // holder. This can happen with indexed properties on NodeLists, for example,
     // which are |own| value props.
     //
     // resolveOwnProperty may or may not cache what it finds on the holder,
     // depending on how ephemeral it decides the property is. This means that we have to
     // first check the result of resolveOwnProperty, and _then_, if that comes up blank,
     // check the holder for any cached native properties.
-    //
-    // Finally, we call resolveNativeProperty, which checks non-own properties,
-    // and unconditionally caches what it finds on the holder.
 
     // Check resolveOwnProperty.
     if (!Traits::singleton.resolveOwnProperty(cx, wrapper, target, holder, id, desc))
         return false;
 
     // Check the holder.
     if (!desc.object() && !JS_GetOwnPropertyDescriptorById(cx, holder, id, desc))
         return false;
     if (desc.object()) {
         desc.object().set(wrapper);
         return true;
     }
 
-    // Nothing in the cache. Call through, and cache the result.
-    if (!Traits::singleton.resolveNativeProperty(cx, wrapper, holder, id, desc))
-        return false;
-
     // We need to handle named access on the Window somewhere other than
     // Traits::resolveOwnProperty, because per spec it happens on the Global
     // Scope Polluter and thus the resulting properties are non-|own|. However,
-    // we're set up (above) to cache (on the holder) anything that comes out of
-    // resolveNativeProperty, which we don't want for something dynamic like
-    // named access. So we just handle it separately here.  Note that this is
+    // we're set up (above) to cache (on the holder),
+    // which we don't want for something dynamic like named access.
+    // So we just handle it separately here.  Note that this is
     // only relevant for CrossOriginXrayWrapper, which calls
     // getPropertyDescriptor from getOwnPropertyDescriptor.
     nsGlobalWindowInner* win = nullptr;
     if (!desc.object() &&
         JSID_IS_STRING(id) &&
         (win = AsWindow(cx, wrapper)))
     {
         nsAutoJSString name;
@@ -1801,27 +1782,18 @@ XrayWrapper<Base, Traits>::getPropertyDe
                 return xpc::Throw(cx, NS_ERROR_FAILURE);
             ExposeObjectToActiveJS(childObj);
             FillPropertyDescriptor(desc, wrapper, ObjectValue(*childObj),
                                    /* readOnly = */ true);
             return JS_WrapPropertyDescriptor(cx, desc);
         }
     }
 
-    // If we still have nothing, we're done.
-    if (!desc.object())
-        return true;
-
-    if (!JS_DefinePropertyById(cx, holder, id, desc) ||
-        !JS_GetOwnPropertyDescriptorById(cx, holder, id, desc))
-    {
-        return false;
-    }
-    MOZ_ASSERT(desc.object());
-    desc.object().set(wrapper);
+    // We found nothing, we're done.
+    MOZ_ASSERT(!desc.object());
     return true;
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::getOwnPropertyDescriptor(JSContext* cx, HandleObject wrapper, HandleId id,
                                                     JS::MutableHandle<PropertyDescriptor> desc)
                                                     const
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -56,19 +56,16 @@ public:
 
     static JSObject* getTargetObject(JSObject* wrapper) {
         JSObject* target = js::UncheckedUnwrap(wrapper, /* stopAtWindowProxy = */ false);
         if (target)
             JS::ExposeObjectToActiveJS(target);
         return target;
     }
 
-    virtual bool resolveNativeProperty(JSContext* cx, JS::HandleObject wrapper,
-                                       JS::HandleObject holder, JS::HandleId id,
-                                       JS::MutableHandle<JS::PropertyDescriptor> desc) = 0;
     // NB: resolveOwnProperty may decide whether or not to cache what it finds
     // on the holder. If the result is not cached, the lookup will happen afresh
     // for each access, which is the right thing for things like dynamic NodeList
     // properties.
     virtual bool resolveOwnProperty(JSContext* cx, JS::HandleObject wrapper,
                                     JS::HandleObject target, JS::HandleObject holder,
                                     JS::HandleId id, JS::MutableHandle<JS::PropertyDescriptor> desc);
 
@@ -140,31 +137,16 @@ private:
 
 class DOMXrayTraits : public XrayTraits
 {
 public:
     constexpr DOMXrayTraits() = default;
 
     static const XrayType Type = XrayForDOMObject;
 
-    virtual bool resolveNativeProperty(JSContext* cx, JS::HandleObject wrapper,
-                                       JS::HandleObject holder, JS::HandleId id,
-                                       JS::MutableHandle<JS::PropertyDescriptor> desc) override
-    {
-        // Xrays for DOM binding objects have a prototype chain that consists of
-        // Xrays for the prototypes of the DOM binding object (ignoring changes
-        // in the prototype chain made by script, plugins or XBL). All properties for
-        // these Xrays are really own properties, either of the instance object or
-        // of the prototypes.
-        // FIXME https://bugzilla.mozilla.org/show_bug.cgi?id=1072482
-        //       This should really be:
-        // MOZ_CRASH("resolveNativeProperty hook should never be called with HasPrototype = 1");
-        //       but we can't do that yet because XrayUtils::HasNativeProperty calls this.
-        return true;
-    }
     virtual bool resolveOwnProperty(JSContext* cx, JS::HandleObject wrapper, JS::HandleObject target,
                                     JS::HandleObject holder, JS::HandleId id,
                                     JS::MutableHandle<JS::PropertyDescriptor> desc) override;
 
     bool delete_(JSContext* cx, JS::HandleObject wrapper, JS::HandleId id, JS::ObjectOpResult& result);
 
     bool defineProperty(JSContext* cx, JS::HandleObject wrapper, JS::HandleId id,
                         JS::Handle<JS::PropertyDescriptor> desc,
@@ -192,23 +174,16 @@ protected:
                                            JS::HandleObject target) const override;
 };
 
 class JSXrayTraits : public XrayTraits
 {
 public:
     static const XrayType Type = XrayForJSObject;
 
-    virtual bool resolveNativeProperty(JSContext* cx, JS::HandleObject wrapper,
-                                       JS::HandleObject holder, JS::HandleId id,
-                                       JS::MutableHandle<JS::PropertyDescriptor> desc) override
-    {
-        MOZ_CRASH("resolveNativeProperty hook should never be called with HasPrototype = 1");
-    }
-
     virtual bool resolveOwnProperty(JSContext* cx, JS::HandleObject wrapper, JS::HandleObject target,
                                     JS::HandleObject holder, JS::HandleId id,
                                     JS::MutableHandle<JS::PropertyDescriptor> desc) override;
 
     bool delete_(JSContext* cx, JS::HandleObject wrapper, JS::HandleId id, JS::ObjectOpResult& result);
 
     bool defineProperty(JSContext* cx, JS::HandleObject wrapper, JS::HandleId id,
                         JS::Handle<JS::PropertyDescriptor> desc,
@@ -306,23 +281,16 @@ public:
 // These traits are used when the target is not Xrayable and we therefore want
 // to make it opaque modulo the usual Xray machinery (like expandos and
 // .wrappedJSObject).
 class OpaqueXrayTraits : public XrayTraits
 {
 public:
     static const XrayType Type = XrayForOpaqueObject;
 
-    virtual bool resolveNativeProperty(JSContext* cx, JS::HandleObject wrapper,
-                                       JS::HandleObject holder, JS::HandleId id,
-                                       JS::MutableHandle<JS::PropertyDescriptor> desc) override
-    {
-        MOZ_CRASH("resolveNativeProperty hook should never be called with HasPrototype = 1");
-    }
-
     virtual bool resolveOwnProperty(JSContext* cx, JS::HandleObject wrapper, JS::HandleObject target,
                                     JS::HandleObject holder, JS::HandleId id,
                                     JS::MutableHandle<JS::PropertyDescriptor> desc) override;
 
     bool defineProperty(JSContext* cx, JS::HandleObject wrapper, JS::HandleId id,
                         JS::Handle<JS::PropertyDescriptor> desc,
                         JS::Handle<JS::PropertyDescriptor> existingDesc,
                         JS::ObjectOpResult& result, bool* defined)