Bug 1021289 part 4. Implement support for WebIDL deleters over Xrays. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 04 Nov 2016 12:41:26 -0400
changeset 363892 77ea5bc98768c61514f8a5b4fb34c7ffaf9875b8
parent 363891 1127566afc68dd760b476f398f6dc70f99d1e45a
child 363893 795788e43f132c210f7f81001ba6e3eb524c7d13
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1021289
milestone52.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 1021289 part 4. Implement support for WebIDL deleters over Xrays. r=peterv
dom/base/WindowNamedPropertiesHandler.cpp
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/Configuration.py
dom/bindings/DOMJSClass.h
dom/bindings/test/test_dom_xrays.html
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/dom/base/WindowNamedPropertiesHandler.cpp
+++ b/dom/base/WindowNamedPropertiesHandler.cpp
@@ -266,16 +266,17 @@ EnumerateWindowNamedProperties(JSContext
 {
   JSAutoCompartment ac(aCx, aObj);
   return js::GetProxyHandler(aObj)->ownPropertyKeys(aCx, aObj, aProps);
 }
 
 const NativePropertyHooks sWindowNamedPropertiesNativePropertyHooks[] = { {
   ResolveWindowNamedProperty,
   EnumerateWindowNamedProperties,
+  nullptr,
   { nullptr, nullptr },
   prototypes::id::_ID_Count,
   constructors::id::_ID_Count,
   nullptr
 } };
 
 static const DOMIfaceAndProtoJSClass WindowNamedPropertiesClass = {
   PROXY_CLASS_DEF("WindowProperties",
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -1902,16 +1902,31 @@ XrayGetExpandoClass(JSContext* cx, JS::H
   if (!IsInstance(type)) {
     // Non-instances don't need any special expando classes.
     return &DefaultXrayExpandoObjectClass;
   }
 
   return nativePropertyHooks->mXrayExpandoClass;
 }
 
+bool
+XrayDeleteNamedProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
+                        JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                        JS::ObjectOpResult& opresult)
+{
+  DOMObjectType type;
+  const NativePropertyHooks* nativePropertyHooks =
+    GetNativePropertyHooks(cx, obj, type);
+  if (!IsInstance(type) || !nativePropertyHooks->mDeleteNamedProperty) {
+    return true;
+  }
+  return nativePropertyHooks->mDeleteNamedProperty(cx, wrapper, obj, id,
+                                                   opresult);
+}
+
 JSObject*
 GetCachedSlotStorageObjectSlow(JSContext* cx, JS::Handle<JSObject*> obj,
                                bool* isXray)
 {
   if (!xpc::WrapperFactory::IsXrayWrapper(obj)) {
     JSObject* retval = js::UncheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
     MOZ_ASSERT(IsDOMObject(retval));
     *isXray = false;
@@ -1922,16 +1937,17 @@ GetCachedSlotStorageObjectSlow(JSContext
   return xpc::EnsureXrayExpandoObject(cx, obj);;
 }
 
 DEFINE_XRAY_EXPANDO_CLASS(, DefaultXrayExpandoObjectClass, 0);
 
 NativePropertyHooks sEmptyNativePropertyHooks = {
   nullptr,
   nullptr,
+  nullptr,
   {
     nullptr,
     nullptr
   },
   prototypes::id::_ID_Count,
   constructors::id::_ID_Count,
   nullptr
 };
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -2396,16 +2396,29 @@ XrayGetNativeProto(JSContext* cx, JS::Ha
 
 /**
  * Get the Xray expando class to use for the given DOM object.
  */
 const JSClass*
 XrayGetExpandoClass(JSContext* cx, JS::Handle<JSObject*> obj);
 
 /**
+ * Delete a named property, if any.  Return value is false if exception thrown,
+ * true otherwise.  The caller should not do any more work after calling this
+ * function, because it has no way whether a deletion was performed and hence
+ * opresult already has state set on it.  If callers ever need to change that,
+ * add a "bool* found" argument and change the generated DeleteNamedProperty to
+ * use it instead of a local variable.
+ */
+bool
+XrayDeleteNamedProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
+                        JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                        JS::ObjectOpResult& opresult);
+
+/**
  * Get the object which should be used to cache the return value of a property
  * getter in the case of a [Cached] or [StoreInSlot] property.  `obj` is the
  * `this` value for our property getter that we're working with.
  *
  * This function can return null on failure to allocate the object, throwing on
  * the JSContext in the process.
  *
  * The isXray outparam will be set to true if obj is an Xray and false
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -330,19 +330,22 @@ class CGNativePropertyHooks(CGThing):
             // would require a run-time load for proper initialization, which would
             // then induce static constructors.  Lots of static constructors.
             extern const NativePropertyHooks sNativePropertyHooks[];
             """)
 
     def define(self):
         if not self.descriptor.wantsXrays:
             return ""
+        deleteNamedProperty = "nullptr"
         if self.descriptor.concrete and self.descriptor.proxy:
             resolveOwnProperty = "ResolveOwnProperty"
             enumerateOwnProperties = "EnumerateOwnProperties"
+            if self.descriptor.needsXrayNamedDeleterHook():
+                deleteNamedProperty = "DeleteNamedProperty"
         elif self.descriptor.needsXrayResolveHooks():
             resolveOwnProperty = "ResolveOwnPropertyViaResolve"
             enumerateOwnProperties = "EnumerateOwnPropertiesViaGetOwnPropertyNames"
         else:
             resolveOwnProperty = "nullptr"
             enumerateOwnProperties = "nullptr"
         if self.properties.hasNonChromeOnly():
             regular = "sNativeProperties.Upcast()"
@@ -371,25 +374,27 @@ class CGNativePropertyHooks(CGThing):
         else:
             expandoClass = "&DefaultXrayExpandoObjectClass"
 
         return fill(
             """
             const NativePropertyHooks sNativePropertyHooks[] = { {
               ${resolveOwnProperty},
               ${enumerateOwnProperties},
+              ${deleteNamedProperty},
               { ${regular}, ${chrome} },
               ${prototypeID},
               ${constructorID},
               ${parentHooks},
               ${expandoClass}
             } };
             """,
             resolveOwnProperty=resolveOwnProperty,
             enumerateOwnProperties=enumerateOwnProperties,
+            deleteNamedProperty=deleteNamedProperty,
             regular=regular,
             chrome=chrome,
             prototypeID=prototypeID,
             constructorID=constructorID,
             parentHooks=parentHooks,
             expandoClass=expandoClass)
 
 
@@ -1794,16 +1799,17 @@ class CGNamedConstructors(CGThing):
                 "{ \"%s\", { %s, &sNamedConstructorNativePropertyHooks }, %i },\n" %
                 (n.identifier.name, NamedConstructorName(n), methodLength(n)))
 
         return fill(
             """
             const NativePropertyHooks sNamedConstructorNativePropertyHooks = {
                 nullptr,
                 nullptr,
+                nullptr,
                 { nullptr, nullptr },
                 prototypes::id::${name},
                 ${constructorID},
                 nullptr
             };
 
             static const NamedConstructor namedConstructors[] = {
               $*{namedConstructors}
@@ -11407,16 +11413,44 @@ def getDeleterBody(descriptor, type, fou
             foundDecl=foundDecl,
             presenceChecker=presenceCheckerClass(descriptor, foundVar=foundVar).define(),
             foundVar=foundVar)
     else:
         body = None
     return body
 
 
+class CGDeleteNamedProperty(CGAbstractStaticMethod):
+    def __init__(self, descriptor):
+        args = [Argument('JSContext*', 'cx'),
+                Argument('JS::Handle<JSObject*>', 'xray'),
+                Argument('JS::Handle<JSObject*>', 'proxy'),
+                Argument('JS::Handle<jsid>', 'id'),
+                Argument('JS::ObjectOpResult&', 'opresult')]
+        CGAbstractStaticMethod.__init__(self, descriptor, "DeleteNamedProperty",
+                                        "bool", args)
+
+    def definition_body(self):
+        return fill(
+            """
+            MOZ_ASSERT(xpc::WrapperFactory::IsXrayWrapper(xray));
+            MOZ_ASSERT(js::IsProxy(proxy));
+            MOZ_ASSERT(!xpc::WrapperFactory::IsXrayWrapper(proxy));
+            JSAutoCompartment ac(cx, proxy);
+            bool deleteSucceeded;
+            bool found = false;
+            $*{namedBody}
+            if (found) {
+              return deleteSucceeded ? opresult.succeed() : opresult.failCantDelete();
+            }
+            return true;
+            """,
+            namedBody=getDeleterBody(self.descriptor, "Named", foundVar="found"))
+
+
 class CGDOMJSProxyHandler_delete(ClassMethod):
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'),
                 Argument('JS::Handle<JSObject*>', 'proxy'),
                 Argument('JS::Handle<jsid>', 'id'),
                 Argument('JS::ObjectOpResult&', 'opresult')]
         ClassMethod.__init__(self, "delete_", "bool", args,
                              virtual=True, override=True, const=True)
@@ -12238,31 +12272,16 @@ class CGDescriptor(CGThing):
         if (descriptor.interface.hasInterfaceObject() and
             NeedsGeneratedHasInstance(descriptor)):
             cgThings.append(CGHasInstanceHook(descriptor))
 
         properties = PropertyArrays(descriptor)
         cgThings.append(CGGeneric(define=str(properties)))
         cgThings.append(CGNativeProperties(descriptor, properties))
 
-        # Set up our Xray callbacks as needed.
-        if descriptor.wantsXrays:
-            if descriptor.concrete and descriptor.proxy:
-                cgThings.append(CGResolveOwnProperty(descriptor))
-                cgThings.append(CGEnumerateOwnProperties(descriptor))
-            elif descriptor.needsXrayResolveHooks():
-                cgThings.append(CGResolveOwnPropertyViaResolve(descriptor))
-                cgThings.append(CGEnumerateOwnPropertiesViaGetOwnPropertyNames(descriptor))
-            if descriptor.wantsXrayExpandoClass:
-                cgThings.append(CGXrayExpandoJSClass(descriptor))
-
-        # Now that we have our ResolveOwnProperty/EnumerateOwnProperties stuff
-        # done, set up our NativePropertyHooks.
-        cgThings.append(CGNativePropertyHooks(descriptor, properties))
-
         if descriptor.interface.hasInterfaceObject():
             cgThings.append(CGClassConstructor(descriptor,
                                                descriptor.interface.ctor()))
             cgThings.append(CGInterfaceObjectJSClass(descriptor, properties))
             cgThings.append(CGNamedConstructors(descriptor))
 
         cgThings.append(CGLegacyCallHook(descriptor))
         if descriptor.interface.getExtendedAttribute("NeedResolve"):
@@ -12324,16 +12343,34 @@ class CGDescriptor(CGThing):
                 cgThings.append(CGWrapGlobalMethod(descriptor, properties))
             elif descriptor.wrapperCache:
                 cgThings.append(CGWrapWithCacheMethod(descriptor, properties))
                 cgThings.append(CGWrapMethod(descriptor))
             else:
                 cgThings.append(CGWrapNonWrapperCacheMethod(descriptor,
                                                             properties))
 
+        # Set up our Xray callbacks as needed.  This needs to come
+        # after we have our DOMProxyHandler defined.
+        if descriptor.wantsXrays:
+            if descriptor.concrete and descriptor.proxy:
+                cgThings.append(CGResolveOwnProperty(descriptor))
+                cgThings.append(CGEnumerateOwnProperties(descriptor))
+                if descriptor.needsXrayNamedDeleterHook():
+                    cgThings.append(CGDeleteNamedProperty(descriptor))
+            elif descriptor.needsXrayResolveHooks():
+                cgThings.append(CGResolveOwnPropertyViaResolve(descriptor))
+                cgThings.append(CGEnumerateOwnPropertiesViaGetOwnPropertyNames(descriptor))
+            if descriptor.wantsXrayExpandoClass:
+                cgThings.append(CGXrayExpandoJSClass(descriptor))
+
+        # Now that we have our ResolveOwnProperty/EnumerateOwnProperties stuff
+        # done, set up our NativePropertyHooks.
+        cgThings.append(CGNativePropertyHooks(descriptor, properties))
+
         # If we're not wrappercached, we don't know how to clear our
         # cached values, since we can't get at the JSObject.
         if descriptor.wrapperCache:
             cgThings.extend(CGClearCachedValueMethod(descriptor, m) for
                             m in clearableCachedAttrs(descriptor))
 
         haveUnscopables = (len(unscopableNames) != 0 and
                            descriptor.interface.hasInterfacePrototypeObject())
--- a/dom/bindings/Configuration.py
+++ b/dom/bindings/Configuration.py
@@ -648,16 +648,18 @@ class Descriptor(DescriptorProvider):
         those, because we don't want to instantiate plug-ins simply
         due to chrome touching them and that's all those hooks do on
         those elements.  So we special-case those here.
         """
         return (self.interface.getExtendedAttribute("NeedResolve") and
                 self.interface.identifier.name not in ["HTMLObjectElement",
                                                        "HTMLEmbedElement",
                                                        "HTMLAppletElement"])
+    def needsXrayNamedDeleterHook(self):
+        return self.operations["NamedDeleter"] is not None
 
     def needsSpecialGenericOps(self):
         """
         Returns true if this descriptor requires generic ops other than
         GenericBindingMethod/GenericBindingGetter/GenericBindingSetter.
 
         In practice we need to do this if our this value might be an XPConnect
         object or if we need to coerce null/undefined to the global.
--- a/dom/bindings/DOMJSClass.h
+++ b/dom/bindings/DOMJSClass.h
@@ -69,16 +69,21 @@ typedef bool
                        JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
                        JS::MutableHandle<JS::PropertyDescriptor> desc);
 
 typedef bool
 (* EnumerateOwnProperties)(JSContext* cx, JS::Handle<JSObject*> wrapper,
                            JS::Handle<JSObject*> obj,
                            JS::AutoIdVector& props);
 
+typedef bool
+(* DeleteNamedProperty)(JSContext* cx, JS::Handle<JSObject*> wrapper,
+                        JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                        JS::ObjectOpResult& opresult);
+
 // Returns true if the given global is of a type whose bit is set in
 // aNonExposedGlobals.
 bool
 IsNonExposedGlobal(JSContext* aCx, JSObject* aGlobal,
                    uint32_t aNonExposedGlobals);
 
 struct ConstantSpec
 {
@@ -271,16 +276,25 @@ struct NativePropertiesHolder
 struct NativePropertyHooks
 {
   // The hook to call for resolving indexed or named properties. May be null if
   // there can't be any.
   ResolveOwnProperty mResolveOwnProperty;
   // The hook to call for enumerating indexed or named properties. May be null
   // if there can't be any.
   EnumerateOwnProperties mEnumerateOwnProperties;
+  // The hook to call to delete a named property.  May be null if there are no
+  // named properties or no named property deleter.  On success (true return)
+  // the "found" argument will be set to true if there was in fact such a named
+  // property and false otherwise.  If it's set to false, the caller is expected
+  // to proceed with whatever deletion behavior it would have if there were no
+  // named properties involved at all (i.e. if the hook were null).  If it's set
+  // to true, it will indicate via opresult whether the delete actually
+  // succeeded.
+  DeleteNamedProperty mDeleteNamedProperty;
 
   // The property arrays for this interface.
   NativePropertiesHolder mNativeProperties;
 
   // This will be set to the ID of the interface prototype object for the
   // interface, if it has one. If it doesn't have one it will be set to
   // prototypes::id::_ID_Count.
   prototypes::ID mPrototypeID;
--- a/dom/bindings/test/test_dom_xrays.html
+++ b/dom/bindings/test/test_dom_xrays.html
@@ -190,16 +190,54 @@ function test()
   is(dataTransfer.types.contains, dataTransfer.types.includes,
      "Should have contains() set up as an alias to includes()");
   // Waive Xrays on the dataTransfer itself, since the .types we get is
   // different over Xrays vs not.
   is(dataTransfer.wrappedJSObject.types.contains, undefined,
      "Underlying object should not have contains() set up as an alias to " +
      "includes()");
 
+  // Check that deleters work correctly in the [OverrideBuiltins] case.
+  var elem = win.document.documentElement;
+  var dataset = elem.dataset;
+  is(dataset.foo, undefined, "Should not have a 'foo' property");
+  ok(!('foo' in dataset), "Really should not have a 'foo' property");
+  is(elem.getAttribute("data-foo"), null,
+     "Should not have a 'data-foo' attribute");
+  ok(!elem.hasAttribute("data-foo"),
+     "Really should not have a 'data-foo' attribute");
+  dataset.foo = "bar";
+  is(dataset.foo, "bar", "Should now have a 'foo' property");
+  ok('foo' in dataset, "Really should have a 'foo' property");
+  is(elem.getAttribute("data-foo"), "bar",
+     "Should have a 'data-foo' attribute");
+  ok(elem.hasAttribute("data-foo"),
+     "Really should have a 'data-foo' attribute");
+  delete dataset.foo;
+  is(dataset.foo, undefined, "Should not have a 'foo' property again");
+  ok(!('foo' in dataset), "Really should not have a 'foo' property again");
+  is(elem.getAttribute("data-foo"), null,
+     "Should not have a 'data-foo' attribute again");
+  ok(!elem.hasAttribute("data-foo"),
+     "Really should not have a 'data-foo' attribute again");
+
+  // Check that deleters work correctly in the non-[OverrideBuiltins] case.
+  var storage = win.sessionStorage;
+  is(storage.foo, undefined, "Should not have a 'foo' property");
+  ok(!('foo' in storage), "Really should not have a 'foo' property");
+  is(storage.getItem("foo"), null, "Should not have an item named 'foo'");
+  storage.foo = "bar";
+  is(storage.foo, "bar", "Should have a 'foo' property");
+  ok('foo' in storage, "Really should have a 'foo' property");
+  is(storage.getItem("foo"), "bar", "Should have an item named 'foo'");
+  delete storage.foo
+  is(storage.foo, undefined, "Should not have a 'foo' property again");
+  ok(!('foo' in storage), "Really should not have a 'foo' property again");
+  is(storage.getItem("foo"), null, "Should not have an item named 'foo' again");
+
   SimpleTest.finish();
 }
 
 SimpleTest.waitForExplicitFinish();
 addLoadEvent(test);
 
 </script>
 </pre>
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1686,16 +1686,24 @@ DOMXrayTraits::resolveOwnProperty(JSCont
     if (!desc.object() || !cacheOnHolder)
         return true;
 
     return JS_DefinePropertyById(cx, holder, id, desc) &&
            JS_GetOwnPropertyDescriptorById(cx, holder, id, desc);
 }
 
 bool
+DOMXrayTraits::delete_(JSContext* cx, JS::HandleObject wrapper,
+                       JS::HandleId id, JS::ObjectOpResult& result)
+{
+    RootedObject target(cx, getTargetObject(wrapper));
+    return XrayDeleteNamedProperty(cx, wrapper, target, id, result);
+}
+
+bool
 DOMXrayTraits::defineProperty(JSContext* cx, HandleObject wrapper, HandleId id,
                               Handle<PropertyDescriptor> desc,
                               Handle<PropertyDescriptor> existingDesc,
                               JS::ObjectOpResult& result, bool* defined)
 {
     // Check for an indexed property on a Window.  If that's happening, do
     // nothing but claim we defined it so it won't get added as an expando.
     if (IsWindow(cx, wrapper)) {
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -191,16 +191,19 @@ public:
         //       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, const js::Wrapper& jsWrapper, JS::HandleObject wrapper,
                                     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,
                         JS::Handle<JS::PropertyDescriptor> existingDesc,
                         JS::ObjectOpResult& result, bool* defined);
     virtual bool enumerateNames(JSContext* cx, JS::HandleObject wrapper, unsigned flags,
                                 JS::AutoIdVector& props);
     static bool call(JSContext* cx, JS::HandleObject wrapper,
                      const JS::CallArgs& args, const js::Wrapper& baseInstance);