1256688 - Change implementation of BaseProxyHandler::has() to follow ES6 [[HasProperty]] specification

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tom S. (please needinfo tschuster)]

Same idea as bug 1083211. The biggest advantage here is the removal of the getPropertyDescriptor call. Sadly Sandbox and Xray still require the old getPropertyDescriptor based semantics.

Comment 1

[Tom S. (please needinfo tschuster)]

Attachment: Change BPH::has to follow [[HasProperty]] for ordinary objects

This also removes the DOMProxyHandler override, because it is basically a copy of the new implementation expressed differently.

Comment 2

[Tom S. (please needinfo tschuster)]

Attachment: Continue using getPropertyDescriptor for has in XrayWrapper

This is just what we did in Bug 1242072 for ::get. It is the simplest solution for having this continue to work before somebody does a deeper fix for Xrays.

Comment 3

[Tom S. (please needinfo tschuster)]

Attachment: Continue using getPropertyDescriptor for has in Sandbox

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 8731596 [details] [diff] [review]
Continue using getPropertyDescriptor for has in XrayWrapper

Review of attachment 8731596 [details] [diff] [review]:
-----------------------------------------------------------------

:-(.

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 8731599 [details] [diff] [review]
Continue using getPropertyDescriptor for has in Sandbox

Review of attachment 8731599 [details] [diff] [review]:
-----------------------------------------------------------------

Would calling JS_HasPropertyById on wrappedObject(proxy) be equivalent?

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 8731594 [details] [diff] [review]
Change BPH::has to follow [[HasProperty]] for ordinary objects

Review of attachment 8731594 [details] [diff] [review]:
-----------------------------------------------------------------

Yes please.

Comment 7

[Tom S. (please needinfo tschuster)]

(In reply to Peter Van der Beken [:peterv] from comment #5)
> Comment on attachment 8731599 [details] [diff] [review]
> Continue using getPropertyDescriptor for has in Sandbox
> 
> Review of attachment 8731599 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Would calling JS_HasPropertyById on wrappedObject(proxy) be equivalent?

I think so in this case. I am working on patches to remove getPropertyDescriptor completely, so I will figure this out soon anyway.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7db58032977a
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f5ac8a1b956
https://hg.mozilla.org/integration/mozilla-inbound/rev/94f67b8f0583

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7db58032977a
https://hg.mozilla.org/mozilla-central/rev/3f5ac8a1b956
https://hg.mozilla.org/mozilla-central/rev/94f67b8f0583