js_DefineOwnProperty should really be used to implement DefineOwnProperty, using it for replaceable properties fails for proxies (like DOM binding proxies or Xrays) because we end up in some code that expects the PropertyDescriptor to have been created from a JS object that was passed in. The generated code for replaceable properties creates its own PropertyDescriptor, not passed in from JS. JS_DefineProperty works equally well here.

Comment on attachment 824029 [details] [diff] [review] v1 r=me, though I wonder how this came up.... Where do we have replaceable properties on a proxy, exactly? Note that the Window binding is NOT a proxy. Was the issue xrays?

Yes, we're hitting this with Xrays. But at one point I accidentally made Window a proxy too, and hit it there.

Given bug 933513 we'll need to backport this to Aurora 27 too, right?

either this patch or the patch on bug 932315 caused a large tp5o shutdown regression on windows xp: https://groups.google.com/forum/#!topic/mozilla.dev.tree-management/r9iehVDynmM

(In reply to Joel Maher (:jmaher) from comment #5) > either this patch or the patch on bug 932315 caused a large tp5o shutdown > regression on windows xp: Backouts on try seem to implicate bug 932315, which is pretty weird. But backing out just this patch didn't change the numbers significantly: Both backed out: https://tbpl.mozilla.org/?tree=Try&rev=5a846b15ad15 Just this backed out: https://tbpl.mozilla.org/?tree=Try&rev=ccc02d51f76b Nothing backed out: https://tbpl.mozilla.org/?tree=Try&rev=9571d819903a

Comment on attachment 824029 [details] [diff] [review] v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 932322 User impact if declined: broken addons and websites Testing completed (on m-c, etc.): landed on m-c Risk to taking this patch (and alternatives if risky): low risk String or IDL/UUID changes made by this patch: none

(In reply to Peter Van der Beken [:peterv] from comment #8) > Comment on attachment 824029 [details] [diff] [review] > Bug caused by (feature/regressing bug #): bug 932322 Er, that was meant to be bug 918345.