tl;dr This is going away and we are better of that way.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/handler/enumerate https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/enumerate

The enumerate trap is not pure virtual anymore, so nobody is forced to implement this trap. In the first local iteration of this patch, I accidentally removed some calls to BaseProxyHandler::enumerate that were overwriting a call to the DirectProxyHandler version.

This is basically a partial backout of Bug 783829. There is no good reason to support new-style iterators anymore in this context.

Comment on attachment 8716902 [details] [diff] [review] Make the enumerate trap non-standard Review of attachment 8716902 [details] [diff] [review]: ----------------------------------------------------------------- r=me with question below addressed. ::: js/public/Proxy.h @@ +58,5 @@ > * (CPOWs, implemented in js/ipc) > * > * ### Proxies and internal methods > * > + * ES 2015 specifies 13 internal methods. The runtime semantics of just nit: Should say ES2016. We are removing it in this year's edition. ::: js/src/proxy/DeadObjectProxy.h @@ +37,5 @@ > virtual bool construct(JSContext* cx, HandleObject proxy, const CallArgs& args) const override; > > /* SpiderMonkey extensions. */ > // BaseProxyHandler::getPropertyDescriptor will throw by calling getOwnPropertyDescriptor. > + // BaseProxyHandler::enumerate will throw by calling ownKeys. Didn't we just remove BPH::enumerate?

Comment on attachment 8716994 [details] [diff] [review] Remove support for new style iterators with for..in Review of attachment 8716994 [details] [diff] [review]: ----------------------------------------------------------------- I'm kinda surprised to see this. Do we not do new iterators internally?

(In reply to Eric Faust [:efaust] from comment #8) > Comment on attachment 8716902 [details] [diff] [review] > Make the enumerate trap non-standard > > Review of attachment 8716902 [details] [diff] [review]: > ----------------------------------------------------------------- > ::: js/src/proxy/DeadObjectProxy.h > @@ +37,5 @@ > > virtual bool construct(JSContext* cx, HandleObject proxy, const CallArgs& args) const override; > > > > /* SpiderMonkey extensions. */ > > // BaseProxyHandler::getPropertyDescriptor will throw by calling getOwnPropertyDescriptor. > > + // BaseProxyHandler::enumerate will throw by calling ownKeys. > > Didn't we just remove BPH::enumerate? Actually not yet. I moved BPH::enumerate to the non-standard section of BPH. It is also not pure-virtual anymore, so nobody is required to implement enumerate. However this made me realize that we can probably completely remove BPH::enumerate, but this will break the enumerate (called iterate in this case) trap for ScriptedIndirectProxy. For that matter Wrappers are the only other special case, because they usually inherit the DirectProxy::enumerate trap, which calls GetIterator(target).

(In reply to Eric Faust [:efaust] from comment #9) > Comment on attachment 8716994 [details] [diff] [review] > Remove support for new style iterators with for..in > > Review of attachment 8716994 [details] [diff] [review]: > ----------------------------------------------------------------- > > I'm kinda surprised to see this. Do we not do new iterators internally? Okay. So internally, most of the time we end up with NativeIteratorNext, so we never actually take a "standard" path. With the changes in ES2016 you can't create any custom iterator for for..in. However we have our very old non-standard extensions Iterator and __iterator__. Iterator still uses the old iteration protocol. __iterator__ is of course user defined, but I don't see why we should support the new iteration protocol there.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd209a5854c0 https://hg.mozilla.org/integration/mozilla-inbound/rev/da0d807517ca https://hg.mozilla.org/integration/mozilla-inbound/rev/874423b7e907

https://hg.mozilla.org/mozilla-central/rev/fd209a5854c0 https://hg.mozilla.org/mozilla-central/rev/da0d807517ca https://hg.mozilla.org/mozilla-central/rev/874423b7e907

Comment on attachment 8716903 [details] [diff] [review] Remove the still disabled Reflect.enumerate code Review of attachment 8716903 [details] [diff] [review]: ----------------------------------------------------------------- Stealing, since Jason is out of the office this week. APPROVED.

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/proxy-s-enumerate-handler-has-been-removed/

https://developer.mozilla.org/en-US/Firefox/Releases/47 https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/handler/enumerate https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/enumerate