Bug 874321 - Fix setting of expandos on Xrays for DOM bindings with named properties. r=bz.

--HG--
extra : rebase_source : 697545417d27696de05c0f22987e4cb187d226cd

dom/bindings/BindingUtils.cpp

@@ -1032,6 +1032,21 @@ XrayResolveNativeProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                                    id, desc);
 }
 
+bool
+XrayDefineProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
+                   JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                   JSPropertyDescriptor* desc, bool* defined)
+{
+  if (!js::IsProxy(obj))
+      return true;
+
+  MOZ_ASSERT(IsDOMProxy(obj), "What kind of proxy is this?");
+
+  DOMProxyHandler* handler =
+    static_cast<DOMProxyHandler*>(js::GetProxyHandler(obj));
+  return handler->defineProperty(cx, wrapper, id, desc, defined);
+}
+
 bool
 XrayEnumerateAttributes(JSContext* cx, JS::Handle<JSObject*> wrapper,
                         JS::Handle<JSObject*> obj,

dom/bindings/BindingUtils.h

@@ -1865,6 +1865,19 @@ XrayResolveNativeProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                           JS::Handle<JSObject*> obj,
                           JS::Handle<jsid> id, JSPropertyDescriptor* desc);
 
+/**
+ * Define a property on obj through an Xray wrapper.
+ *
+ * wrapper is the Xray JS object.
+ * obj is the target object of the Xray, a binding's instance object or a
+ *     interface or interface prototype object.
+ * defined will be set to true if a property was set as a result of this call.
+ */
+bool
+XrayDefineProperty(JSContext* cx, JS::Handle<JSObject*> wrapper, 
+                   JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                   JSPropertyDescriptor* desc, bool* defined);
+
 /**
  * This enumerates indexed or named properties of obj and operations, attributes
  * and constants of the interfaces for obj.

dom/bindings/Codegen.py

@@ -6116,6 +6116,29 @@ ${body}
                   'const': ' const' if self.const else '',
                   'body': body })
 
+class ClassUsingDeclaration(ClassItem):
+    """"
+    Used for importing a name from a base class into a CGClass
+
+    baseClass is the name of the base class to import the name from
+
+    name is the name to import
+
+    visibility determines the visibility of the name (public,
+    protected, private), defaults to public.
+    """
+    def __init__(self, baseClass, name, visibility='public'):
+        self.baseClass = baseClass
+        ClassItem.__init__(self, name, visibility)
+
+    def declare(self, cgClass):
+        return string.Template("""using ${baseClass}::${name};
+""").substitute({ 'baseClass': self.baseClass,
+                  'name': self.name })
+
+    def define(self, cgClass):
+        return ''
+
 class ClassConstructor(ClassItem):
     """
     Used for adding a constructor to a CGClass.
@@ -6885,8 +6908,8 @@ class CGDOMJSProxyHandler_defineProperty(ClassMethod):
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'), Argument('JS::Handle<JSObject*>', 'proxy'),
                 Argument('JS::Handle<jsid>', 'id'),
-                Argument('JSPropertyDescriptor*', 'desc')]
-        ClassMethod.__init__(self, "defineProperty", "bool", args)
+                Argument('JSPropertyDescriptor*', 'desc'), Argument('bool*', 'defined')]
+        ClassMethod.__init__(self, "defineProperty", "bool", args, virtual=True, override=True)
         self.descriptor = descriptor
     def getBody(self):
         set = ""
@@ -6897,6 +6920,7 @@ class CGDOMJSProxyHandler_defineProperty(ClassMethod):
                 raise TypeError("Can't handle creator that's different from the setter")
             set += ("int32_t index = GetArrayIndexFromId(cx, id);\n" +
                     "if (IsArrayIndex(index)) {\n" +
+                    "  *defined = true;" +
                     CGIndenter(CGProxyIndexedSetter(self.descriptor)).define() +
                     "  return true;\n" +
                     "}\n") % (self.descriptor.nativeType)
@@ -6914,8 +6938,9 @@ class CGDOMJSProxyHandler_defineProperty(ClassMethod):
                                    "  return false;\n"
                                    "}\n"
                                    "if (hasUnforgeable) {\n"
-                                   "  JSBool defined;\n"
-                                   "  return js_DefineOwnProperty(cx, ${holder}, id, *desc, &defined);\n"
+                                   "  *defined = true;" +
+                                   "  JSBool unused;\n"
+                                   "  return js_DefineOwnProperty(cx, ${holder}, id, *desc, &unused);\n"
                                    "}\n")
             set += CallOnUnforgeableHolder(self.descriptor,
                                            defineOnUnforgeable,
@@ -6927,7 +6952,8 @@ class CGDOMJSProxyHandler_defineProperty(ClassMethod):
                 raise TypeError("Can't handle creator that's different from the setter")
             # If we support indexed properties, we won't get down here for
             # indices, so we can just do our setter unconditionally here.
-            set += (CGProxyNamedSetter(self.descriptor).define() + "\n" +
+            set += ("*defined = true;\n" +
+                    CGProxyNamedSetter(self.descriptor).define() + "\n" +
                     "return true;\n")
         else:
             if self.descriptor.supportsNamedProperties():
@@ -7328,7 +7354,9 @@ class CGDOMJSProxyHandler(CGClass):
         if (descriptor.operations['IndexedSetter'] or
             descriptor.operations['NamedSetter'] or
             UseHolderForUnforgeable(descriptor)):
-            methods.append(CGDOMJSProxyHandler_defineProperty(descriptor))
+            methods.extend([CGDOMJSProxyHandler_defineProperty(descriptor),
+                            ClassUsingDeclaration("mozilla::dom::DOMProxyHandler",
+                                                  "defineProperty")])
         methods.extend([CGDOMJSProxyHandler_getOwnPropertyNames(descriptor),
                         CGDOMJSProxyHandler_hasOwn(descriptor),
                         CGDOMJSProxyHandler_get(descriptor),

dom/bindings/DOMJSProxyHandler.cpp

@@ -179,7 +179,7 @@ DOMProxyHandler::getPropertyDescriptor(JSContext* cx, JS::Handle<JSObject*> prox
 
 bool
 DOMProxyHandler::defineProperty(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
-                                JSPropertyDescriptor* desc)
+                                JSPropertyDescriptor* desc, bool* defined)
 {
   if ((desc->attrs & JSPROP_GETTER) && desc->setter == JS_StrictPropertyStub) {
     return JS_ReportErrorFlagsAndNumber(cx,

dom/bindings/DOMJSProxyHandler.h

@@ -40,7 +40,13 @@ public:
   bool getPropertyDescriptor(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
                             JSPropertyDescriptor* desc, unsigned flags) MOZ_OVERRIDE;
   bool defineProperty(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
-                      JSPropertyDescriptor* desc) MOZ_OVERRIDE;
+                      JSPropertyDescriptor* desc) MOZ_OVERRIDE
+  {
+    bool unused;
+    return defineProperty(cx, proxy, id, desc, &unused);
+  }
+  virtual bool defineProperty(JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
+                              JSPropertyDescriptor* desc, bool* defined);
   bool delete_(JSContext* cx, JS::Handle<JSObject*> proxy,
                JS::Handle<jsid> id, bool* bp) MOZ_OVERRIDE;
   bool enumerate(JSContext* cx, JS::Handle<JSObject*> proxy, JS::AutoIdVector& props) MOZ_OVERRIDE;

dom/tests/mochitest/chrome/test_sandbox_bindings.xul

@@ -244,6 +244,19 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=741267
         ok(false, "Should be able to construct my event " + e);
       }
 
+      try {
+        var elem = Components.utils.evalInSandbox('document.createElement("p")', sandbox);
+        elem.expando = 5;
+        elem.expando = 7;
+        is(elem.expando, 7, "Should be able to set expandos on Xrays for DOM bindings");
+        var doc = Components.utils.evalInSandbox('document', sandbox);
+        doc.expando = 5;
+        doc.expando = 7;
+        is(doc.expando, 7, "Should be able to set expandos on Xrays for DOM bindings with named properties");
+      } catch (e) {
+        ok(false, "Setting expandos on Xrays shouldn't throw " + e);
+      }
+
       SimpleTest.finish();
     }
 

js/xpconnect/wrappers/XrayWrapper.cpp

@@ -1210,12 +1210,8 @@ DOMXrayTraits::defineProperty(JSContext *cx, HandleObject wrapper, HandleId id,
     if (!existingDesc.obj())
         return true;
 
-    RootedObject obj(cx, getTargetObject(wrapper));
-    if (!js::IsProxy(obj))
-        return true;
-
-    *defined = true;
-    return js::GetProxyHandler(obj)->defineProperty(cx, wrapper, id, desc);
+    JS::Rooted<JSObject*> obj(cx, getTargetObject(wrapper));
+    return XrayDefineProperty(cx, wrapper, obj, id, desc, defined);
 }
 
 bool